Skip to content

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

Merged
antonpirker merged 3 commits into
masterfrom
revert-3890-aliu/ref-launchdarkly
Jan 7, 2025
Merged

Revert "ref(flags): register LD hook in setup instead of init, and don't check for initialization"#3900
antonpirker merged 3 commits into
masterfrom
revert-3890-aliu/ref-launchdarkly

Conversation

@cmanallen

@cmanallen cmanallen commented Jan 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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

@cmanallen cmanallen mentioned this pull request Jan 3, 2025
Merged
@codecov

codecov Bot commented Jan 3, 2025 •
edited
Loading

Copy link
Copy Markdown

❌ 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

cmanallen commented Jan 6, 2025

Copy link
Copy Markdown
Member Author

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

@aliu39

aliu39 commented Jan 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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

antonpirker commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

Yes, Sanic is sometimes flaky

@antonpirker

antonpirker commented Jan 7, 2025

Copy link
Copy Markdown
Contributor

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
@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

Reviewers

@aliu39 aliu39 aliu39 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cmanallen @aliu39 @antonpirker