From 7ab331098eab6d37cdfe3afd022392398dc499b3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 25 Aug 2023 11:10:36 +0100 Subject: [PATCH] Simplify ContextLocalState API 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 --- bugsnag/client.py | 26 ++++-------- bugsnag/context.py | 27 +++++-------- tests/test_context.py | 93 ++++++++++++++++++++++++------------------- 3 files changed, 70 insertions(+), 76 deletions(-) diff --git a/bugsnag/client.py b/bugsnag/client.py index 1fef169c..5797fd79 100644 --- a/bugsnag/client.py +++ b/bugsnag/client.py @@ -14,7 +14,7 @@ ) from bugsnag.configuration import Configuration, RequestConfiguration from bugsnag.event import Event -from bugsnag.feature_flags import FeatureFlag, FeatureFlagDelegate +from bugsnag.feature_flags import FeatureFlag from bugsnag.handlers import BugsnagHandler from bugsnag.sessiontracker import SessionTracker from bugsnag.utils import to_rfc3339 @@ -86,7 +86,7 @@ def notify(self, exception: BaseException, asynchronous=None, **options): self.configuration, RequestConfiguration.get_instance(), **options, - feature_flag_delegate=self._feature_flag_delegate + feature_flag_delegate=self._context.feature_flag_delegate ) self._leave_breadcrumb_for_event(event) @@ -107,7 +107,7 @@ def notify_exc_info(self, exc_type, exc_value, traceback, self.configuration, RequestConfiguration.get_instance(), **options, - feature_flag_delegate=self._feature_flag_delegate + feature_flag_delegate=self._context.feature_flag_delegate ) self._leave_breadcrumb_for_event(event) @@ -226,35 +226,25 @@ def log_handler( ) -> BugsnagHandler: return BugsnagHandler(client=self, extra_fields=extra_fields) - @property - def _feature_flag_delegate(self) -> FeatureFlagDelegate: - feature_flag_delegate = self._context.feature_flag_delegate - - if feature_flag_delegate is None: - feature_flag_delegate = FeatureFlagDelegate() - self._context.feature_flag_delegate = feature_flag_delegate - - return feature_flag_delegate - @property def feature_flags(self) -> List[FeatureFlag]: - return self._feature_flag_delegate.to_list() + return self._context.feature_flag_delegate.to_list() def add_feature_flag( self, name: Union[str, bytes], variant: Union[None, str, bytes] = None ) -> None: - self._feature_flag_delegate.add(name, variant) + self._context.feature_flag_delegate.add(name, variant) def add_feature_flags(self, feature_flags: List[FeatureFlag]) -> None: - self._feature_flag_delegate.merge(feature_flags) + self._context.feature_flag_delegate.merge(feature_flags) def clear_feature_flag(self, name: Union[str, bytes]) -> None: - self._feature_flag_delegate.remove(name) + self._context.feature_flag_delegate.remove(name) def clear_feature_flags(self) -> None: - self._feature_flag_delegate.clear() + self._context.feature_flag_delegate.clear() @property def breadcrumbs(self) -> List[Breadcrumb]: diff --git a/bugsnag/context.py b/bugsnag/context.py index d6cd4be1..1d617b34 100644 --- a/bugsnag/context.py +++ b/bugsnag/context.py @@ -1,4 +1,5 @@ from weakref import WeakKeyDictionary +from bugsnag.feature_flags import FeatureFlagDelegate try: @@ -39,14 +40,8 @@ def _raw_set(client, key, value): client_context[client][key] = value -def _raw_copy(client): - client_context = _client_contexts.get() - - # no need to copy if there is no existing value - if client_context is None: - return - - _client_contexts.set(client_context.copy()) +def create_new_context(): + _client_contexts.set(None) FEATURE_FLAG_DELEGATE_KEY = 'feature_flag_delegate' @@ -56,13 +51,13 @@ class ContextLocalState: def __init__(self, client): self._client = client - def create_copy_for_context(self): - _raw_copy(self._client) - @property - def feature_flag_delegate(self): - return _raw_get(self._client, FEATURE_FLAG_DELEGATE_KEY) + def feature_flag_delegate(self) -> FeatureFlagDelegate: + delegate = _raw_get(self._client, FEATURE_FLAG_DELEGATE_KEY) + + # create a new delegate if one does not exist already + if delegate is None: + delegate = FeatureFlagDelegate() + _raw_set(self._client, FEATURE_FLAG_DELEGATE_KEY, delegate) - @feature_flag_delegate.setter - def feature_flag_delegate(self, new_delegate): - _raw_set(self._client, FEATURE_FLAG_DELEGATE_KEY, new_delegate) + return delegate diff --git a/tests/test_context.py b/tests/test_context.py index 9cbc3969..5896c565 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -4,26 +4,23 @@ from time import sleep from threading import Thread from random import shuffle, randrange -from bugsnag.context import ContextLocalState - - -class Client: - def __init__(self): - self.state = ContextLocalState(self) +from bugsnag.client import Client +from bugsnag.feature_flags import FeatureFlag +from bugsnag.context import create_new_context def test_state_is_stored_separately_per_thread(): client1 = Client() client2 = Client() - assert client1.state.feature_flag_delegate is None - assert client2.state.feature_flag_delegate is None + assert client1.feature_flags == [] + assert client2.feature_flags == [] - client1.state.feature_flag_delegate = 'a delegate' - client2.state.feature_flag_delegate = 'another delegate' + client1.add_feature_flag('a') + client2.add_feature_flag('b') - assert client1.state.feature_flag_delegate == 'a delegate' - assert client2.state.feature_flag_delegate == 'another delegate' + assert client1.feature_flags == [FeatureFlag('a')] + assert client2.feature_flags == [FeatureFlag('b')] def thread_target(client1, client2, id): thread.exception = None @@ -32,17 +29,28 @@ def thread_target(client1, client2, id): # sleep for a bit to allow other threads time to interfere sleep(randrange(0, 100) / 1000) - assert client1.state.feature_flag_delegate is None - assert client2.state.feature_flag_delegate is None + assert client1.feature_flags == [] + assert client2.feature_flags == [] - client1.state.feature_flag_delegate = 'client1 (#%d)' % id - client2.state.feature_flag_delegate = 'client2 (#%d)' % id + client1.add_feature_flag('client1 (#%d) 1' % id) + client2.add_feature_flag('client2 (#%d) 1' % id) # sleep for a bit to allow other threads time to interfere sleep(randrange(0, 100) / 1000) - assert client1.state.feature_flag_delegate == 'client1 (#%d)' % id - assert client2.state.feature_flag_delegate == 'client2 (#%d)' % id + client1.add_feature_flag('client1 (#%d) 2' % id) + client2.add_feature_flag('client2 (#%d) 2' % id) + + assert client1.feature_flags == [ + FeatureFlag('client1 (#%d) 1' % id), + FeatureFlag('client1 (#%d) 2' % id) + ] + + assert client2.feature_flags == [ + FeatureFlag('client2 (#%d) 1' % id), + FeatureFlag('client2 (#%d) 2' % id) + ] + except Exception as e: thread.exception = e @@ -68,8 +76,8 @@ def thread_target(client1, client2, id): raise thread.exception # changes in other threads should not affect this thread - assert client1.state.feature_flag_delegate == 'a delegate' - assert client2.state.feature_flag_delegate == 'another delegate' + assert client1.feature_flags == [FeatureFlag('a')] + assert client2.feature_flags == [FeatureFlag('b')] @pytest.mark.skipif( @@ -80,42 +88,43 @@ def test_state_is_stored_separately_per_async_context(): client1 = Client() client2 = Client() - assert client1.state.feature_flag_delegate is None - assert client2.state.feature_flag_delegate is None + assert client1.feature_flags == [] + assert client2.feature_flags == [] async def mutate_state(id): - client1.state.create_copy_for_context() - client2.state.create_copy_for_context() - - client1.state.feature_flag_delegate = None - client2.state.feature_flag_delegate = None + create_new_context() await asyncio.sleep(randrange(0, 100) / 1000) - assert client1.state.feature_flag_delegate is None - assert client2.state.feature_flag_delegate is None + assert client1.feature_flags == [] + assert client2.feature_flags == [] - client1.state.feature_flag_delegate = 'client1 (#%d)' % id + client1.add_feature_flag('client1 (#%d) 1' % id) await asyncio.sleep(randrange(0, 100) / 1000) - client2.state.feature_flag_delegate = 'client2 (#%d)' % id + client2.add_feature_flag('client2 (#%d) 1' % id) await asyncio.sleep(randrange(0, 100) / 1000) - assert client1.state.feature_flag_delegate == 'client1 (#%d)' % id - assert client2.state.feature_flag_delegate == 'client2 (#%d)' % id + assert client1.feature_flags == [FeatureFlag('client1 (#%d) 1' % id)] + assert client2.feature_flags == [FeatureFlag('client2 (#%d) 1' % id)] - client1.state.feature_flag_delegate = 'client1 (#%d) 2' % id - client2.state.feature_flag_delegate = 'client2 (#%d) 2' % id + client1.add_feature_flag('client1 (#%d) 2' % id) + client2.add_feature_flag('client2 (#%d) 2' % id) await asyncio.sleep(randrange(0, 100) / 1000) - assert client1.state.feature_flag_delegate == 'client1 (#%d) 2' % id - assert client2.state.feature_flag_delegate == 'client2 (#%d) 2' % id + assert client1.feature_flags == [ + FeatureFlag('client1 (#%d) 1' % id), + FeatureFlag('client1 (#%d) 2' % id), + ] + + assert client2.feature_flags == [ + FeatureFlag('client2 (#%d) 1' % id), + FeatureFlag('client2 (#%d) 2' % id), + ] async def test(): - tasks = [] - for i in range(10): - tasks.append(mutate_state(i)) + tasks = [mutate_state(i) for i in range(10)] await asyncio.gather(*tasks) @@ -126,5 +135,5 @@ async def test(): finally: loop.close() - assert client1.state.feature_flag_delegate is None - assert client2.state.feature_flag_delegate is None + assert client1.feature_flags == [] + assert client2.feature_flags == []