-
Notifications
You must be signed in to change notification settings - Fork 0
Uptake the changes to publish responses and add get_condition and get_measurement from id #73
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
Conversation
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 updates the data store client APIs to align with changes in ni-apis. The publish methods now return IDs instead of full objects, and two new getter methods (get_measurement() and get_condition()) are introduced to retrieve objects by ID.
- Simplified publish API responses to return string IDs directly
- Added
get_measurement()andget_condition()methods for retrieving objects by ID - Updated all tests to use the new two-step pattern: publish for ID, then get object when needed
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/ni/datastore/data/_data_store_client.py |
Updated publish methods to return string IDs instead of objects; added new get_measurement() and get_condition() methods |
tests/unit/data/test_publish_measurement.py |
Updated to expect string IDs from publish responses; removed unused import |
tests/unit/data/test_publish_condition.py |
Updated to expect string IDs from publish responses; removed unused import |
tests/acceptance/test_query_measurements.py |
Updated to work with string IDs instead of objects |
tests/acceptance/test_query_conditions.py |
Updated to work with string IDs instead of objects |
tests/acceptance/test_publish_with_metadata.py |
Updated to work with string IDs instead of objects |
tests/acceptance/test_publish_measurement_batch_and_read_data.py |
Updated to use new pattern: get ID from publish, retrieve object with get_measurement() |
tests/acceptance/test_publish_measurement_and_read_data.py |
Updated to use new pattern: get ID from publish, retrieve object with get_measurement() |
tests/acceptance/test_publish_condition_batch_and_read_data.py |
Updated to use new pattern: get ID from publish, retrieve object with get_condition() |
tests/acceptance/test_publish_condition_and_read_data.py |
Updated to use new pattern: get ID from publish, retrieve object with get_condition() |
pyproject.toml |
Updated ni-measurements-data-v1-client dependency from 0.2.0.dev2 to 0.2.0.dev4 |
poetry.lock |
Updated lock file with new dependency versions and hashes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What does this Pull Request accomplish?
Pulls in the latest changes to the .proto APIs, specifically from this commit in ni-apis.
Why should this Pull Request be merged?
Performance gains are expected when publishing. Will test this out once this package and the datastore-service are updated.
What testing has been done?
PR and updating unit tests that called
publish_measurement(_batch)andpublish_conditon(_batch)to callget_measurementandget_conditionafterwards.