Add context manager for unpinning #57
Open
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 commit adds support for nested use of the UsePrimaryDB context manager. If a new instance of the context manager was created at each level we could use instance variables to store the pinning state but the default instance is generally used directly. Instead, we store a stack of pinning values locally in the thread and pop each value as we unwind the stack.
This commit adds a UseSecondaryDB context manager which unpins within a block.
bardiharborow
commented
Jun 28, 2022
|
|
||
| def __exit__(self, type, value, tb): | ||
| if not _locals.old.pop(): | ||
| previous_state = _locals.old.pop() | ||
| if previous_state: |
There was a problem hiding this comment.
ℹ️ This introduces a subtle behaviour change. Previously, manual use of unpin_this_thread() would leak out of the context block, for example when:
- a thread is pinned
- enters the context block (
_locals.old = True) - is manually unpinned (with
unpin_this_thread()) - exits the context block (
not _locals.old == Falseso no action is taken) - the thread will be unpinned when it reaches here
This new code will always restore the previous pinning state when exiting a context block. In my view this is more predictable, but I'm open to your opinion on this.
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.
Building on #56, this PR adds a
UseSecondaryDBcontext manager which unpins within a block.(You can just review the second commit, apparently I can't stack PRs from a fork. I will rebase if/when the first PR merges.)