Skip to content

rust(feat): adding tdms imports#553

Open
Brandon-Shippy wants to merge 9 commits intomainfrom
rust/adding-tdms-imports
Open

rust(feat): adding tdms imports#553
Brandon-Shippy wants to merge 9 commits intomainfrom
rust/adding-tdms-imports

Conversation

@Brandon-Shippy
Copy link
Copy Markdown
Contributor

What was changed

  • Added TDMS Import functionality to Rust CLI
  • Added tdms crate
  • For --preview added some client-side parsing that matches that of detect_config() on the backend, so we still find the timestamp properties or timestamp channel to ensure parity between what is shown in preview and what will be found when passing it into the backend
  • Note that we are parsing the tdms file client-side specifically for --preview, we are still sending the raw bytes to the backend to be parsed there. This is because of limitation with the tdms crate and that it does not have functionality for all tdms data types. In the preview if we hit a data type that the tdms crate does not know, we output an error alongside it to tell the user that even if its not able to be previewed here it could still be properly imported into sift.

Validation

  • Added a suite of unit tests
  • Validated imports with manual tests, testing with error prone tdms files and proper tdms files.

@Brandon-Shippy Brandon-Shippy force-pushed the rust/adding-tdms-imports branch from d245f73 to 0d86e2e Compare May 6, 2026 22:12
@Brandon-Shippy
Copy link
Copy Markdown
Contributor Author

In the future I will do gradual commits, this is on me should not have done one big commit

@Brandon-Shippy Brandon-Shippy requested a review from wei-qlu May 6, 2026 23:32
@Brandon-Shippy Brandon-Shippy changed the title Rust(feat): adding tdms imports rust(feat): adding tdms imports May 7, 2026
@Brandon-Shippy Brandon-Shippy requested a review from tsift May 7, 2026 18:31
Comment thread rust/crates/sift_cli/src/cli/mod.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/tdms/detect_tdms_config.rs Outdated
.context("error creating data import for tdms")?
.into_inner();

file.rewind()?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the purpose of rewind here?

Copy link
Copy Markdown
Contributor Author

@Brandon-Shippy Brandon-Shippy May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call out, just an artifact from an earlier implementation I tried. I removed it

0c9e6bf

file.rewind()?;
let compressed_data = gzip_file(file)?;

let rest_client = create_rest_client(&ctx).context("failed to create rest client for tdms")?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The REST client and creating/sending the POST below are also good candidates to put in some utility or helper place since that will likely end up being common across all import file types.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will likely need to do another ticket to make all other imports use this util, I am not sure if it would fit under this PR

}

#[test]
fn test_build_tdms_config() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: instead of one large test, split into many small ones where the name helps indicate exactly what is being tested. Makes it easier to maintain/update tests and no add duplicate test coverage down the road.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Brandon-Shippy Brandon-Shippy requested review from solidiquis and tsift May 7, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants