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

Remove manual Hub clones #63591

Closed
Tracked by #56248
szokeasaurusrex opened this issue Jan 22, 2024 · 2 comments · Fixed by #65541
Closed
Tracked by #56248

Remove manual Hub clones #63591

szokeasaurusrex opened this issue Jan 22, 2024 · 2 comments · Fixed by #65541
Assignees

Comments

@szokeasaurusrex
Copy link
Member

szokeasaurusrex commented Jan 22, 2024

Manual Hub clones can cause scope leakage issues. Also, with the Hub & Scope changes (getsentry/sentry-python#2533), the SDK will no longer have a concept of a Hub, so these clones likely won't do what they were intended to do anymore.

See also: https://www.notion.so/sentry/Sentry-SDK-Custom-Code-and-why-we-Should-fix-it-932c8493c29b4bb2bf18c2fc4b729ef1?pvs=4#966901ec61a140f1a18d8f63ba9c7719

Parent issue: #56248

@szokeasaurusrex
Copy link
Member Author

szokeasaurusrex commented Feb 19, 2024

Following discussion with @sl0thentr0py, it appears we can only remove the Hub clone in reader.py.

[edit]: We cannot even remove the hub clone in reader.py, but we can move it, so we only make the hub clone when executing in the thread pool executor

@sl0thentr0py
Copy link
Member

yes, for future reference, the others are all some kind of thread pool executor where you want task level isolation and not thread level (since the same thread will be reused for processing tasks), so we cannot really remove those unless we have first class thread pool integration support.

@szokeasaurusrex szokeasaurusrex self-assigned this Feb 20, 2024
szokeasaurusrex added a commit that referenced this issue Feb 26, 2024
The hub clone in `download_segment` is unnecessary because the function
is sometimes called from outside a Thread Pool Executor. Since cloning
the hub is only necessary in the Thread Pool Executor, we move the hub
clone there

Closes GH-63591
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants