-
Notifications
You must be signed in to change notification settings - Fork 0
Allow persistant configuration #15
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
Conversation
8e1c0b9
to
8153672
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #15 +/- ##
==========================================
+ Coverage 96.87% 97.09% +0.21%
==========================================
Files 5 5
Lines 384 413 +29
Branches 51 58 +7
==========================================
+ Hits 372 401 +29
Misses 5 5
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/montagu_deploy/cli.py
Outdated
montagu start [<path>] [--extra=PATH] [--option=OPTION]... [--pull] | ||
montagu status [<path>] | ||
montagu stop [<path>] [--volumes] [--network] [--kill] [--force] | ||
[--extra=PATH] [--option=OPTION]... | ||
montagu renew-certificate <path> [--option=OPTION]... [--] [ARGS...] | ||
montagu renew-certificate [<path>] [--option=OPTION]... [--] [ARGS...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you're still allowing non-configure commands to provide an explicit path? Would you not prefer to do the packit-style approach where separate configuration is mandatory? That seems safer..? Since currently I think you could still get yourself into a pickle by doing
montagu configure prod
montagu start uat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I'll look at trying to prevent that a bit - I think from memory it causes some issues with the tests if we're too strict!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this now with the same syntax as used in packit-deploy, where we can pass montagu start --name uat
which is more deliberate. This mostly avoids lots of refactoring of tests...
@@ -10,6 +11,18 @@ | |||
from src.montagu_deploy.config import MontaguConfig | |||
|
|||
|
|||
@contextmanager | |||
def transient_working_directory(path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a minute to understand this! Nice pattern.
Simpler, but the same idea, as mrc-ide/packit-deploy#27 but I just used the existing docopt setup because I wanted less of a fight. I am happy to port over to click if we think that's worth the effort. Once this is in I will update the montagu-config docs
Merge after #16, contains those commits