rust(feat): adding tdms imports#553
Conversation
d245f73 to
0d86e2e
Compare
|
In the future I will do gradual commits, this is on me should not have done one big commit |
| .context("error creating data import for tdms")? | ||
| .into_inner(); | ||
|
|
||
| file.rewind()?; |
There was a problem hiding this comment.
Whats the purpose of rewind here?
There was a problem hiding this comment.
good call out, just an artifact from an earlier implementation I tried. I removed it
| file.rewind()?; | ||
| let compressed_data = gzip_file(file)?; | ||
|
|
||
| let rest_client = create_rest_client(&ctx).context("failed to create rest client for tdms")?; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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.
What was changed
tdms cratedetect_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--preview, we are still sending the raw bytes to the backend to be parsed there. This is because of limitation with thetdms crateand that it does not have functionality for all tdms data types. In the preview if we hit a data type that thetdms cratedoes 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