Skip to content

Conversation

@mjohanse-emr
Copy link
Collaborator

@mjohanse-emr mjohanse-emr commented Aug 26, 2025

What does this Pull Request accomplish?

  • Adds a new namespace package ni.measurementlink.discovery.v1.client that provides the client interface to ni.measurementlink.discovery.v1.proto.
  • I copied these files from the measurement-plugin-python repo and made some small edits to fix mypy and unit test errors.
  • I copied the discovery client unit tests into this new package.
  • I got rid of the deprecated methods in the DiscoveryClient class and updated the unit tests to only use the non-deprecated versions. This includes deleting one unit test that was testing the deprecation warning.
  • To avoid a circular dependency, I included the ServiceInfo definition from measurement-plugin-python's measurement client folder in this new discovery client package.

Why should this Pull Request be merged?

Implements AB#3239055

What testing has been done?

Unit tests, mypy, pyright, styleguide.

Michael Johansen added 2 commits August 26, 2025 15:02
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2025

Test Results

   66 files  + 20     66 suites  +20   1m 48s ⏱️ + 1m 18s
  180 tests + 36    180 ✅ + 36   0 💤 ± 0  0 ❌ ±0 
1 784 runs  +360  1 774 ✅ +350  10 💤 +10  0 ❌ ±0 

Results for commit df74bb3. ± Comparison against base commit e6ab1b6.

♻️ This comment has been updated with latest results.

Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Copy link
Collaborator Author

@mjohanse-emr mjohanse-emr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm leaving this review so there are comments on some of the changes I made after copying the files from measurement-plugin-python. Hopefully the comments are useful.

If any of these changes should not be made, please let me know.

@mjohanse-emr mjohanse-emr marked this pull request as ready for review August 26, 2025 20:53
@mjohanse-emr
Copy link
Collaborator Author

This package needs to be added to the workflow files to get the checks to run on it. I'm going to wait until @csjall 's PR goes in and I'll add it to the json file.

Copy link
Collaborator

@jfriedri-ni jfriedri-ni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to update the workflows, too, but it might be simpler to wait for @csjall's #94 to complete first.

Michael Johansen added 5 commits August 27, 2025 08:53
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
@mjohanse-emr mjohanse-emr requested a review from bkeryan August 27, 2025 18:09
Michael Johansen added 3 commits August 27, 2025 13:44
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
Michael Johansen added 2 commits August 27, 2025 15:09
Signed-off-by: Michael Johansen <michael.johansen@ni.com>
…the run_unit_tests workflow.

Signed-off-by: Michael Johansen <michael.johansen@ni.com>
@mjohanse-emr mjohanse-emr merged commit d42e063 into main Aug 27, 2025
61 checks passed
@mjohanse-emr mjohanse-emr deleted the users/mjohanse/discovery_client branch August 27, 2025 20:41
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.

5 participants