-
Notifications
You must be signed in to change notification settings - Fork 123
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
Improve poetry adjustments in CI workflows #1272
Labels
Comments
I'd suggest using (will update lock file for changes in pyproject.toml, but not other changes) |
you can also use poetry add to update pyproject.toml (no need to run poetry remove) e.g. |
Thanks @carlcsaposs-canonical ! |
amandahla
pushed a commit
to amandahla/operator
that referenced
this issue
Jun 26, 2024
When running `poetry lock` in the CI workloads, only update the ops dependency, not all dependencies. We want to know whether the proposed changes to ops would break the charm's tests, not whether updating all dependencies, including the proposed ops changes, would do so. This is one of the suggestions in canonical#1272. Using `poetry add --lock` seems slightly cleaner than the `sed` system we currently use, but I wasn't able to easily figure out how to do that with a `ops = { path = "ci/path/for/ops/branch" }` type specifier. It seems like this approach is at least an improvement on the existing one, even if not perfect. (It also will unblock other PRs, given that the tests fail because we're re-locking in entirety). Fixes canonical#1272.
tonyandrewmeyer
added a commit
to tonyandrewmeyer/operator
that referenced
this issue
Jun 26, 2024
When running `poetry lock` in the CI workloads, only update the ops dependency, not all dependencies. We want to know whether the proposed changes to ops would break the charm's tests, not whether updating all dependencies, including the proposed ops changes, would do so. This is one of the suggestions in canonical#1272. Using `poetry add --lock` seems slightly cleaner than the `sed` system we currently use, but I wasn't able to easily figure out how to do that with a `ops = { path = "ci/path/for/ops/branch" }` type specifier. It seems like this approach is at least an improvement on the existing one, even if not perfect. (It also will unblock other PRs, given that the tests fail because we're re-locking in entirety). Fixes canonical#1272.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Our workflows that test against charms currently patches in the appropriate version of ops:
With poetry, this has a side effect that the
poetry lock
will regenerate the entire lockfile, which could bump dependencies other thanops
, which is not the intention for this workflow (but is probably a good idea for the charm itself).@dimaqq suggested that if we do
poetry remove ops
andpoetry add x
that should only make the ops change, which would better match what we want. If that doesn't work, perhaps we can adjust the workflow in some other way to avoid this side effect.The text was updated successfully, but these errors were encountered: