-
Notifications
You must be signed in to change notification settings - Fork 0
Broaden Data Store Client API Support #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Broaden Data Store Client API Support #13
Conversation
…dditional DataStoreService RPCs. Updating Publish and Read methods to perform type conversion for more types.
… doesn't require the client to know what type of data to expect. Review feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for some resolutions on whether we wrap timestamp types. Generally looks good and I'm happy we're getting this much API surface reviewed at this point.
mjohanse-emr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I'm out tomorrow, I'm going to approve this just so my "Changes requested" isn't blocking this PR. This review doesn't mean you can submit, please get approvals from others.
If it's still open on Monday, I'd be happy to take another look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR broadens the Python API for the Data Store by providing a unified Client interface that consolidates functionality from lower-level clients. It expands support for various data types beyond boolean and double analog waveform types, implements support for steps, test results, and metadata operations, and enhances the read API with flexible type handling.
- Added support for publishing/reading multiple data types including scalars, vectors, waveforms, spectrums, and digital waveforms
- Implemented comprehensive metadata store operations for UUTs, operators, test descriptions, hardware/software items, etc.
- Enhanced timestamp handling for waveforms with validation and automatic t0 detection
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pyproject.toml | Added hightime dependency for improved timestamp handling |
| src/ni/datastore/client.py | Major expansion of Client class with unified API, extensive data type support, and metadata operations |
| tests/unit/test_ni_datastore_client.py | Updated tests for new API and added timestamp validation test cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
After discussion with @csjall and @dixonjoel, we've decided to go ahead and push this through so that we can more easily start collaborating on iterative improvements and additions. |
What does this Pull Request accomplish?
This PR provides an initial pass at a broader Python API for the Data Store. This includes handling the responsibilities of the lower-level
DataStoreClient,MetadataStoreClient, andMonikerClientthrough a unifiedClientinterface.The PR broadens the existing
Publishsupport for a number of data types beyond thebooland double analog waveform types that were previous supported. It also continues to iterate on theReadAPI to allow for flexibility in either returning a strongly type-hinted value or a genericobjectwhen the type isn't known beforehand.Furthermore, it implements support for operations related to
Steps,TestResultss, and various types of metadata owned by theMetadataStoreClient.Why should this Pull Request be merged?
We believe these changes move us closer to having a fully featured high-level Python API for the Data Store present as part of the
datastore-pythonrepo. We plan to continue iterating on this as needed.What testing has been done?
Publishtimestamps for waveforms.