Skip to content

Conversation

@dixonjoel
Copy link
Collaborator

@dixonjoel dixonjoel commented Sep 30, 2025

What does this Pull Request accomplish?

Makes the sessionmanagement.client depend on sessionmanagement.proto 0.1.0-dev2 or greater. Prior, it was depending on 0.1.0-dev0 or greater.

Why should this Pull Request be merged?

As of #129, sessionmanagement.client needs sessionmanagement.proto to have an annotations module. See this line in _annotations.py:
from ni.measurementlink.sessionmanagement.v1.annotations import ...

However, this was only added to sessionmanagement.proto in #134 which corresponds with sessionmanagement.proto 0.1.0-dev2. The dependency from sessionmanagement.client was never updated to require this version.

Therefore, there are scenarios where a user is updating their sessionmanagement.client dependency, but they won't get a new sessionmanagement.proto version as required.

What testing has been done?

Local 'poetry lock' and 'poetry install' of the sessionmanagement.client file to ensure it got the right version. PR checks.

@dixonjoel dixonjoel requested a review from bkeryan as a code owner September 30, 2025 15:45
Copilot AI review requested due to automatic review settings September 30, 2025 15:45
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 minimum required version of the sessionmanagement.proto dependency from 0.1.0-dev0 to 0.1.0-dev2 to ensure compatibility with the annotations module that was introduced in version 0.1.0-dev2.

  • Updated dependency version constraint for ni-measurementlink-sessionmanagement-v1-proto

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link
Contributor

Test Results

  120 files  ±0    120 suites  ±0   3m 20s ⏱️ +6s
  240 tests ±0    238 ✅ ±0   2 💤 ±0  0 ❌ ±0 
2 400 runs  ±0  2 370 ✅ ±0  30 💤 ±0  0 ❌ ±0 

Results for commit b5e3ed1. ± Comparison against base commit c045a15.

@dixonjoel
Copy link
Collaborator Author

@bkeryan Sorry I didn't fix this earlier or mention it yesterday. If we agree and this gets submitted, if we decide to, I'll create another release of sessionmanagement.client after that. I'm not sure it's too important. Normally, if you depend on the latest version of sessionmanagement.client and you do 'poetry lock' initially, you'll get the latest version of both. However, if you were to depend on sessionmanagement.client 0.1.0-dev0 and then change to depend on 0.1.0-dev1, it will not update to sessionmanagement.proto 0.1.0-dev1 since 0.1.0-dev0 still satisfies the requirement.

@dixonjoel dixonjoel merged commit 59c5c88 into main Sep 30, 2025
347 checks passed
@dixonjoel dixonjoel deleted the users/jdixon/fix-sessionmanagement-proto-dep branch September 30, 2025 19:28
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