-
Notifications
You must be signed in to change notification settings - Fork 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
unify parse_storage_constraint or client.Constraints during deploy #1079
unify parse_storage_constraint or client.Constraints during deploy #1079
Conversation
1 similar comment
f931900
to
183979a
Compare
0d08092
to
3c4bded
Compare
…ng deploy Signed-off-by: Adam Dyess <[email protected]>
3c4bded
to
3eb4cd3
Compare
@cderici do you think this is a PR that could be reviewed? |
This PR needs to be rebased since #1186 has been merged. |
@dimaqq any chance this PR can get a look now? |
I've asked on Matrix for a review. Meanwhile, please run pre-commit on your change. |
I had a cursory look, the changes are in the It's used only twice, in Model's Model's higher level function already coerces this argument, but maybe not correctly? that is it allows dicts but not strings. I feel that this change needs to be refactored at the very least. |
⚡ pre-commit run --files $(echo juju/client/_[cd]*.py)
check for added large files..............................................Passed
check python ast.........................................................Passed
check for case conflicts.................................................Passed
check that executables have shebangs.................(no files to check)Skipped
check that scripts with shebangs are executable..........................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
check json...........................................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
check toml...........................................(no files to check)Skipped
mixed line ending........................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
detect private key.......................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
codespell................................................................Passed |
I wonder if this was actually already fixed by @james-garner-canonical in #1105 in which case we could close this PR and the associated issue #1075 |
Hi @addyess , it's plausible that #1105 did indeed fix #1075. It sounds like #1105 was fixing an error introduced in #1053, which was indeed released in 3.5.2.0 -- unfortunately #1105 wasn't linked to #1075 or your PR. I'll have to take a deeper look on Monday. Hopefully we can do a 3.5.2.2 release to get this fix to you. |
Should now be fully resolved in Thanks for your help with this, @adyess Release coming soon! |
Description
Fixes #1075 by collapsing both storage constraint deviations in
model._deploy
QA Steps
All CI tests need to pass.
Notes & Discussion
Perhaps it's not the best change, feel free to scrap and adjust better.