Prevent feature flags from leaking between clients #355
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
tomlongridge
approved these changes
Aug 29, 2023
Previously you had to manage the delegate being None if one didn't already exist in the current context. This made the API a bit awkward as every use of it required the same check & creation of a new delegate Now the delegate is created in ContextLocalState directly so users don't need to worry about it possibly returning None
imjoehaines
force-pushed
the
fix-feature-flag-client-leaks
branch
from
abdddad
to
4bb9ad5
Compare
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.
Goal
Currently feature flags are stored in the same place across all Client instances, meaning every client shares the same list of feature flags (though they are stored in a ContextVar/thread local so don't leak between async contexts/threads)
This PR introduces a new
ContextLocalStateAPI that allows storing things separately per-client and per-context. This works by storing aWeakKeyDictionaryin aContextVarwithClientinstances as keys and a dict as values. TheWeakKeyDictionaryensures that that when Clients are garbage collected their contextual state is collected tooIn the future this can be extended to cover other contextual data (breadcrumbs, sessions etc…) but that would require a lot of refactoring and could be considered a BC break so will be implemented in a future PR for the next major version