-
Notifications
You must be signed in to change notification settings - Fork 18
Add ability for python measurement services to have multiple user interface files #102
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
@bkeryan I did not yet update all the examples with their .lock files because I understood we have to publish a release, then update the .lock files to reference the new release. However, did we say that we were going to add the .lock files to the .gitignore and not distribute them? If that's the case, it seems like we won't have to wait to update the examples to use multiple ui-file-paths and we can (not sure it's a good idea or a separate PR is better anyway) do it all in this one change? Key point being we don't have to wait for a new published release of ni-measurement-service to change the examples. |
Yes, I've been meaning to get back to that. Ignore the lock files for this PR. |
ni_measurement_generator/ni_measurement_generator/templates/measurement.py.mako
Outdated
Show resolved
Hide resolved
we usually do `typing.List[str]`
--
Matthew Shafer
Senior Software Engineer | Segmenting Driver Ownership | Segmenting Installers NIC | ni.com<http://ni.com/>
From: Joel Dixon ***@***.***>
Sent: Monday, October 17, 2022 11:18 AM
To: ni/measurement-services-python ***@***.***>
Cc: Matthew Shafer ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [ni/measurement-services-python] Add ability for python measurement services to have multiple user interface files (PR #102)
@dixonjoel commented on this pull request.
________________________________
In ni_measurement_service/measurement/info.py<https://urldefense.com/v3/__https:/github.com/ni/measurement-services-python/pull/102*discussion_r997265932__;Iw!!FbZ0ZwI3Qg!oI-EmtGlkRWVRvo7dxoA305kBFhoRjOLHg7MWQJGeEE5OMwfSuf8Tp1eHBhXKy5uA3LHshUFQblbcN9o-BLGdBc08A$>:
"""
display_name: str
version: str
measurement_type: str
product_type: str
- ui_file_path: str
+ ui_file_paths: list
Static analysis didn't pass. I found this in python/mypy#7907<https://urldefense.com/v3/__https:/github.com/python/mypy/issues/7907__;!!FbZ0ZwI3Qg!oI-EmtGlkRWVRvo7dxoA305kBFhoRjOLHg7MWQJGeEE5OMwfSuf8Tp1eHBhXKy5uA3LHshUFQblbcN9o-BJ2T1rjng$>. If I include: from __future__ import annotations at the top, I can use ui_file_paths: list[str].
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/ni/measurement-services-python/pull/102*discussion_r997265932__;Iw!!FbZ0ZwI3Qg!oI-EmtGlkRWVRvo7dxoA305kBFhoRjOLHg7MWQJGeEE5OMwfSuf8Tp1eHBhXKy5uA3LHshUFQblbcN9o-BLGdBc08A$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AFUMV2IJJFKM4PMBTUV3T4LWDV34VANCNFSM6AAAAAARF7LNWI__;!!FbZ0ZwI3Qg!oI-EmtGlkRWVRvo7dxoA305kBFhoRjOLHg7MWQJGeEE5OMwfSuf8Tp1eHBhXKy5uA3LHshUFQblbcN9o-BJu-u71wQ$>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
INTERNAL - NI CONFIDENTIAL
|
…ix static analysis errors.
What does this Pull Request accomplish?
Metadata was changed recently to allow a service to specify multiple UI files. This updates the templates and the grpc service to allow multiple UI file paths. Also updating the sample measurement to specify and include two .measui files which are slightly different to demonstrate this and also to expand the test so it's asserting both .measui files exist.
Why should this Pull Request be merged?
Add ability for multiple .measui files to the Python repo.
What testing has been done?
Automated tests plus running the sample measurement with the feature toggle on for switching between multiple .measui files and running with that.