Skip to content

Conversation

@dixonjoel
Copy link
Collaborator

@dixonjoel dixonjoel commented Nov 18, 2025

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) and publish_conditon(_batch) to call get_measurement and get_condition afterwards.

@dixonjoel dixonjoel requested a review from csjall as a code owner November 18, 2025 20:30
Copilot AI review requested due to automatic review settings November 18, 2025 20:30
Copy link
Contributor

Copilot AI left a 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() and get_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.

@dixonjoel dixonjoel merged commit 378d425 into main Nov 19, 2025
26 checks passed
@dixonjoel dixonjoel deleted the users/jdixon/uptake-publish-response-changes branch November 19, 2025 14:54
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