Skip to content
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

generator: Add dependency on service #944

Merged
merged 3 commits into from
Sep 26, 2024

Conversation

bkeryan
Copy link
Collaborator

@bkeryan bkeryan commented Sep 25, 2024

What does this Pull Request accomplish?

Update ni-measurement-plugin-sdk-generator to depend on ni-measurement-plugin-sdk-service

Update the generator's Poetry dev dependencies to install service in develop mode, in order to avoid saving the service source in the cached venv.

Why should this Pull Request be merged?

It was relying on Poetry dev dependencies. No one tried installing the generator in isolation.

What testing has been done?

cd packages/generator
pipx install .
mkdir ../examples/client
cd ../examples/client
ni-measurement-plugin-client-generator -a

Copy link

github-actions bot commented Sep 25, 2024

Test Results

    40 files  ±0      40 suites  ±0   53m 32s ⏱️ +49s
   695 tests ±0     695 ✅ ±0      0 💤 ±0  0 ❌ ±0 
16 890 runs  ±0  15 820 ✅ ±0  1 070 💤 ±0  0 ❌ ±0 

Results for commit 145f03d. ± Comparison against base commit 90d3ab7.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jayaseelan-james jayaseelan-james left a comment

Choose a reason for hiding this comment

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

Should we consider adding the "develop=true" attribute to the NIMS package specified under the dev.dependencies?

@bkeryan
Copy link
Collaborator Author

bkeryan commented Sep 26, 2024

Should we consider adding the "develop=true" attribute to the NIMS package specified under the dev.dependencies?

Good point. I think that will help. Poetry will install a .pth link file into lib/site-packages instead of installing the whole package.

We were previously doing it this way, but we stopped because it didn't catch erroneous code that accessed files that only exist in Git and not in the distribution package. I guess we'll have to be more careful about that in the future, or reorganize the repo (again) to use the "src" package layout instead of the "flat" package layout.

@bkeryan bkeryan merged commit cccabe8 into main Sep 26, 2024
17 checks passed
@bkeryan bkeryan deleted the users/bkeryan/fix-generator-dependency branch September 26, 2024 05:59
bkeryan added a commit that referenced this pull request Sep 26, 2024
* generator: Add dependency on service

* generator: Update poetry.lock

* generator: Use develop mode to avoid caching service source in the venv

(cherry picked from commit cccabe8)
bkeryan added a commit that referenced this pull request Sep 26, 2024
… (#947)

generator: Add dependency on service (#944)

* generator: Add dependency on service

* generator: Update poetry.lock

* generator: Use develop mode to avoid caching service source in the venv

(cherry picked from commit cccabe8)
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.

ni-measurement-plugin-sdk-generator is missing dependency on ni-measurement-plugin-sdk-service
2 participants