Skip to content

Commit

Permalink
Simplify ContextLocalState API
Browse files Browse the repository at this point in the history
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
  • Loading branch information
imjoehaines committed Aug 25, 2023
1 parent f18f551 commit 7ab3310
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 76 deletions.
26 changes: 8 additions & 18 deletions bugsnag/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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]:
Expand Down
27 changes: 11 additions & 16 deletions bugsnag/context.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from weakref import WeakKeyDictionary
from bugsnag.feature_flags import FeatureFlagDelegate


try:
Expand Down Expand Up @@ -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'
Expand All @@ -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
93 changes: 51 additions & 42 deletions tests/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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(
Expand All @@ -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)

Expand All @@ -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 == []

0 comments on commit 7ab3310

Please sign in to comment.