Locking usability improvements #1470
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This also adds a description parameter to provide human-friendly context to plugins.
This only expand the coverage of the finally/unlock to the state initialization. If that initialization fails, we have no reason to hold on to the lock.
This will help us detect programming errors where we requested read-only locking but still attempt to write. Before locking, such a distinction was unnecessary, so it's important that we enforce this, to promote out-of-sync state bugs to proper errors.
Because we're now disallowing change when we don't request locks for writing, we can avoid state upload churn as well.
Even a non-exclusive lock can needlessly block exclusive operations like deploy, particularly when it's an idle interactive session.
Such upgrades are still necessary and are permissible as long as the db isn't uploaded (taken care of in earlier commit) and the upgrade does not touch anything outside the db, which is currently the case, and is documented inline.
roberth
force-pushed
the
locking-usability
branch
from
1d13724
to
1d236ce
Compare
It only matters for construction. After construction, we can erase the type by upcasting, keeping the types simple and improving mypy precision.
roberth
force-pushed
the
locking-usability
branch
from
1d236ce
to
ec14c2a
Compare
roberth
force-pushed
the
locking-usability
branch
from
ec14c2a
to
365bf3c
Compare
> 'store_true' and 'store_false' - These are special cases of > 'store_const' used for storing the values True and False > respectively. In addition, they create default values of > False and True respectively. https://docs.python.org/3/library/argparse.html#argparse.ArgumentParser.add_argument
roberth
force-pushed
the
locking-usability
branch
from
1ab5569
to
c2d3e6b
Compare
This comment has been minimized.
This comment has been minimized.
|
cc @adisbladis, would you mind reviewing this? |
|
This is sorely needed and has been up for review for nearly 3 months. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With remote state and locking, concurrent use of
nixopscommands has become impossible. This is bad when, for example, you want tonixops sshwhile anixops deployis running.This PR solves a couple of locking related problems by
nixops sshunlock before the session starts, so read-write operations can resume while presumably-interactive sessions existnixops ssh --nowwhich uses whatever state it can get but does not lock at allMore details are in the commit messages.