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

script_defs: open state files and deployments with a context manager #1263

Merged
merged 43 commits into from Mar 26, 2020

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Mar 24, 2020

Use context managers to open and manage statefiles and deployments. This lets us do some fancier things with those state files. For example, storing the state other places.

This change is almost entirely whitespace. Check it out without whitespace in the diff: https://github.com/NixOS/nixops/pull/1263/files?w=1

Also consider looking commit-by-commit, which makes it easier, I think, to review.

Here is a diff adding state backends using this: legacy, in-memory, and s3: https://github.com/grahamc/nixops/compare/context-manager...grahamc:context-manager-states-rebase?expand=1

@grahamc grahamc force-pushed the context-manager branch 4 times, most recently from 2841369 to 0c7c2a6 Compare March 24, 2020 21:17
nixops/script_defs.py Outdated Show resolved Hide resolved
state = nixops.statefile.StateFile(args.state_file)
try:
yield state
finally:
Copy link
Member Author

Choose a reason for hiding this comment

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

This used to catch the exception and no longer tries to.

"ssh://{}".format(m.get_ssh_name()),
args.storepath,
],
m.logger,
Copy link
Member

Choose a reason for hiding this comment

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

👍

@domenkozar domenkozar merged commit f2804bc into NixOS:master Mar 26, 2020
@grahamc grahamc deleted the context-manager branch March 26, 2020 17:20
@grahamc grahamc added this to the 2.0 milestone Apr 20, 2020
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.

None yet

3 participants