-
Notifications
You must be signed in to change notification settings - Fork 35
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
Deprectated warning for function sentry_sdk.configure_scope, fix migration for sentry-sdk 2.x #95
Conversation
|
||
with configure_scope() as scope: | ||
if sentry_sdk.VERSION >= '2.12.0': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is 2.12.0 the version where the warning was added, or when the get_isolation_scope
function was added? It seems like it would be best to use the version where get_isolation_scope
was added, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method get_isolation_scope
was added in 2.0 but only in the class Scope, it wasn't added in the init.py, that means we must use sentry_sdk.Scope.get_isolation_scope()
to use the method. The warning and the function was added in init.py in version 2.12, that's why I did this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can check it here:
Added in __init__.py
2.12: https://github.com/getsentry/sentry-python/blob/2.12.0/sentry_sdk/__init__.py
Doesn't added in __init__.py
version less than or equal to 2.11: https://github.com/getsentry/sentry-python/blob/2.11.0/sentry_sdk/__init__.py
Added in 2.0 but without the warning and also without being added the symbol in the var __all__
in the __init__.py
file (there is also a symbol called isolation_scope in the var __all__
but it's not even a method): https://github.com/getsentry/sentry-python/blob/2.0.0/sentry_sdk/scope.py#L273
Added warning in 2.12 here: https://github.com/getsentry/sentry-python/blob/2.12.0/sentry_sdk/api.py#L223
|
||
with configure_scope() as scope: | ||
if sentry_sdk.VERSION >= '2.12.0': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to be careful when comparing strings here:
>>> "2.2.0" >= "2.12.0"
True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx, already fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me 👍
Me too 😊 |
Here #94 is the issue reported, I made the fix in this PR updating the libs using
poetry update
and fixing the unit test using the new function calledget_isolation_scope