-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update measurement service to get version from .serviceconfig #827
Conversation
Test Results 30 files ± 0 30 suites ±0 42m 57s ⏱️ - 6m 6s Results for commit 3bb7a41. ± Comparison against base commit 7d1a5fe. This pull request removes 7 and adds 18 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
packages/service/ni_measurement_plugin_sdk_service/measurement/service.py
Outdated
Show resolved
Hide resolved
@nick-beer |
@jayaseelan-james - Good catch! Yes, this PR has been updated to include the necessary changes to the generator. |
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.
I'd like to see the changes with multiple versions.
@dixonjoel - can you explain what you mean by this? I don't follow. |
@nick-beer "Sorry - I reset you without seeing this message. Thanks for pointing this out - I think it makes sense for ServiceInfo to have versions rather than version. I'll make the update." Maybe I misunderstood this. You resolved this conversation. What changed? |
@nick-beer Oh, this update? Sorry I missed this. |
@dixonjoel - yeah, that's the one that updated |
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.
Approved with suggestion
What does this Pull Request accomplish?
Add support for reading a measurement's version from its
.serviceconfig
file, as well as passing that version through discovery service'sRegisterService
API.To do this:
version
andui_file_paths
parameters to theMeasurementService
constructor optional.serviceconfig
file AND the two values are different. If the values are different, raise an errorversion
property toServiceInfo
ServiceInfo.version
through theRegisterServiceRequest
inside theDiscoveryClient
.Why should this Pull Request be merged?
We're working to add version support to our plugins. The intention is for the version of a plugin to be stored in its
.serviceconfig
file and passed to discovery service when it registers itself.What testing has been done?
A couple new tests have been written, and a couple existing tests have been updated to account for the presence of a version in the service config.