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

ref(init): Move sentry_sdk.init out of hub.py #3276

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

szokeasaurusrex
Copy link
Member

Now that the Hub-based API is deprecated, sentry_sdk.init should no longer be in hub.py. Since it is kind of its own thing, it makes sense to implement init in its own file.

Closes #3233

Comment on lines +51 to +53
# Use `ClientConstructor` to define the argument types of `init` and
# `ContextManager[Any]` to tell static analyzers about the return type.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is inaccurate, since we actually use _InitGuard in the class definition. However, since I am just copying this code over from hub for now, I think it is out of scope to fix here. Perhaps, we can consider fixing in a separate PR later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't really know what the purpose of the _InitGuard is since we never tell people to do the following anywhere, so this is just vague unadvertised functionality.

with sentry_sdk.init():
    pass

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, perhaps we should explicitly deprecate this context manager functionality and remove in the next major, in that case? Or is this a potentially valuable feature we should advertise more?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init is generally one per process anyway. In case people want DSN isolation, multiple clients is the way to go now. So I really don't see the use case for this.
Let's deprecate and remove.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sl0thentr0py, I opened an issue to make this change, since I think it belongs in a separate PR. Would appreciate an approval here if everything else looks good to you.

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 64.28571% with 10 lines in your changes missing coverage. Please review.

Project coverage is 79.43%. Comparing base (06d5da1) to head (d5920bc).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3276      +/-   ##
==========================================
- Coverage   79.44%   79.43%   -0.01%     
==========================================
  Files         132      133       +1     
  Lines       14278    14282       +4     
  Branches     2999     3000       +1     
==========================================
+ Hits        11343    11345       +2     
- Misses       2088     2089       +1     
- Partials      847      848       +1     
Files Coverage Δ
sentry_sdk/__init__.py 100.00% <100.00%> (ø)
sentry_sdk/hub.py 53.76% <ø> (-1.19%) ⬇️
sentry_sdk/_init_implementation.py 61.53% <61.53%> (ø)

... and 2 files with indirect coverage changes

Base automatically changed from szokeasaurusrex/no-hub-init to master July 11, 2024 09:12
Now that the `Hub`-based API is deprecated, `sentry_sdk.init` should no longer be in `hub.py`. Since it is kind of its own thing, it makes sense to implement `init` in its own file.

Closes #3233
@szokeasaurusrex szokeasaurusrex merged commit 41e4bb4 into master Jul 16, 2024
121 of 122 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/init-in-client branch July 16, 2024 09:04
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.

Move sentry_sdk.init implementation out of hub.py
2 participants