From 8fa6d3d814c76faf72098e4f4ba2d2207e87f5b9 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Tue, 7 Jan 2025 07:12:47 -0600 Subject: [PATCH] =?UTF-8?q?Revert=20"ref(flags):=20register=20LD=20hook=20?= =?UTF-8?q?in=20setup=20instead=20of=20init,=20and=20don't=20chec=E2=80=A6?= =?UTF-8?q?"=20(#3900)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutating a class attribute on `__init__` violates encapsulation and will lead to strange errors. We need to rethink how we want to implement this before we merge any code. A simple reproduction of the issue: ```python >>> class X: ... y = 0 ... def __init__(self, z): ... self.__class__.y = z ... >>> a = X(1) >>> b = X(2) >>> X.y 2 >>> a.y 2 >>> b.y 2 ``` Reverts getsentry/sentry-python#3890 This reverts commit c3516db643af20396ea981393431646f1a3ef123. Co-authored-by: Anton Pirker --- sentry_sdk/integrations/launchdarkly.py | 14 ++++++------- .../launchdarkly/test_launchdarkly.py | 21 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/sentry_sdk/integrations/launchdarkly.py b/sentry_sdk/integrations/launchdarkly.py index 066464cc22..a9eef9e1a9 100644 --- a/sentry_sdk/integrations/launchdarkly.py +++ b/sentry_sdk/integrations/launchdarkly.py @@ -20,7 +20,6 @@ class LaunchDarklyIntegration(Integration): identifier = "launchdarkly" - _ld_client = None # type: LDClient | None def __init__(self, ld_client=None): # type: (LDClient | None) -> None @@ -28,19 +27,20 @@ def __init__(self, ld_client=None): :param client: An initialized LDClient instance. If a client is not provided, this integration will attempt to use the shared global instance. """ - self.__class__._ld_client = ld_client - - @staticmethod - def setup_once(): - # type: () -> None try: - client = LaunchDarklyIntegration._ld_client or ldclient.get() + client = ld_client or ldclient.get() except Exception as exc: raise DidNotEnable("Error getting LaunchDarkly client. " + repr(exc)) + if not client.is_initialized(): + raise DidNotEnable("LaunchDarkly client is not initialized.") + # Register the flag collection hook with the LD client. client.add_hook(LaunchDarklyHook()) + @staticmethod + def setup_once(): + # type: () -> None scope = sentry_sdk.get_current_scope() scope.add_error_processor(flag_error_processor) diff --git a/tests/integrations/launchdarkly/test_launchdarkly.py b/tests/integrations/launchdarkly/test_launchdarkly.py index 9b2bbb6b86..20566ce09a 100644 --- a/tests/integrations/launchdarkly/test_launchdarkly.py +++ b/tests/integrations/launchdarkly/test_launchdarkly.py @@ -183,14 +183,10 @@ async def runner(): } -def test_launchdarkly_integration_did_not_enable(sentry_init, uninstall_integration): - """ - Setup should fail when using global client and ldclient.set_config wasn't called. - - We're accessing ldclient internals to set up this test, so it might break if launchdarkly's - implementation changes. - """ - +def test_launchdarkly_integration_did_not_enable(monkeypatch): + # Client is not passed in and set_config wasn't called. + # TODO: Bad practice to access internals like this. We can skip this test, or remove this + # case entirely (force user to pass in a client instance). ldclient._reset_client() try: ldclient.__lock.lock() @@ -198,6 +194,11 @@ def test_launchdarkly_integration_did_not_enable(sentry_init, uninstall_integrat finally: ldclient.__lock.unlock() - uninstall_integration(LaunchDarklyIntegration.identifier) with pytest.raises(DidNotEnable): - sentry_init(integrations=[LaunchDarklyIntegration()]) + LaunchDarklyIntegration() + + # Client not initialized. + client = LDClient(config=Config("sdk-key")) + monkeypatch.setattr(client, "is_initialized", lambda: False) + with pytest.raises(DidNotEnable): + LaunchDarklyIntegration(ld_client=client)