Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "ref(flags): register LD hook in setup instead of init, and don't check for initialization" #3900

Merged
merged 3 commits into from
Jan 7, 2025

Conversation

cmanallen
Copy link
Member

@cmanallen cmanallen commented Jan 3, 2025

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:

>>> 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 #3890

Copy link

codecov bot commented Jan 3, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
13770 1 13769 4167
View the top 1 failed tests by shortest run time
tests.test_basics test_classmethod_tracing
Stack Traces | 0.148s run time
tests/test_basics.py:946: in test_classmethod_tracing
    assert fake_start_child.call_count == 1
E   AssertionError: assert 2 == 1
E    +  where 2 = <MagicMock name='mock.start_child' id='139907889185552'>.call_count

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@cmanallen
Copy link
Member Author

@antonpirker Is this a known flake? ERROR tests/integrations/sanic/test_sanic.py - TypeError: 'type' object is not subscriptable

@aliu39
Copy link
Member

aliu39 commented Jan 6, 2025

ERROR tests/integrations/sanic/test_sanic.py - TypeError: 'type' object is not subscriptable

+1, There's a similar error at https://github.com/getsentry/sentry-python/actions/runs/12600825778/job/35120696927?pr=3888

@antonpirker
Copy link
Member

Yes, Sanic is sometimes flaky

@antonpirker
Copy link
Member

Oh, the new Sanic release from 2025-01-01 is breaking our tests, so not flaky. I will fix this.

@antonpirker antonpirker merged commit 8fa6d3d into master Jan 7, 2025
138 checks passed
@antonpirker antonpirker deleted the revert-3890-aliu/ref-launchdarkly branch January 7, 2025 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants