Expose the scope getters to top level API and use them everywhere #3357
Merged
Conversation
sl0thentr0py
force-pushed
the
neel/prepare-scope-management
branch
from
July 26, 2024 14:09
4b39048 to
c9598b1
Compare
|
Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time. ❌ Failed Test Results:Completed 14758 tests with View the full list of failed testspy3.6-boto3-v1.12
py3.6-common
py3.6-gevent
py3.7-aiohttp-v3.4
py3.7-boto3-v1.23
py3.7-common
py3.7-httpx-v0.23
py3.8-aiohttp-latest
py3.8-common
py3.8-gevent
py3.9-aiohttp-v3.8
py3.9-common
py3.9-httpx-latest
py3.9-httpx-v0.27
|
sl0thentr0py
force-pushed
the
neel/prepare-scope-management
branch
from
July 26, 2024 14:25
c9598b1 to
1fb1170
Compare
sl0thentr0py
force-pushed
the
neel/prepare-scope-management
branch
2 times, most recently
from
July 26, 2024 14:32
88eaf8a to
e67291c
Compare
sentrivana
approved these changes
Jul 29, 2024
sentrivana
left a comment
Contributor
There was a problem hiding this comment.
LGTM, see small comments
antonpirker
approved these changes
Jul 29, 2024
antonpirker
left a comment
Contributor
There was a problem hiding this comment.
Looks good. Just a few nitpicks
|
|
||
| # Fall back to isolation scope's traceparent. It always has one | ||
| return Scope.get_isolation_scope().get_traceparent() | ||
| return self.get_isolation_scope().get_traceparent() |
Contributor
There was a problem hiding this comment.
In scope.py here we use self.get_x() because in the future we will have a check in api.get_x() for otel or sentry and we do not want to call the check all the time?
Member
Author
There was a problem hiding this comment.
yes, there will be a PotelScope class that will inherit from Scope but the internal getters/setters will be different
6 tasks
sl0thentr0py
force-pushed
the
neel/prepare-scope-management
branch
from
July 29, 2024 12:08
2a48e76 to
c577b31
Compare
* Going forward, we might have 2 different scope implementations so we can't have the `Scope` class being called everywhere directly since this will be abstracted away.
Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io>
sl0thentr0py
force-pushed
the
neel/prepare-scope-management
branch
from
July 29, 2024 12:17
c577b31 to
1d6420c
Compare
sl0thentr0py
marked this pull request as ready for review
July 29, 2024 12:17
szokeasaurusrex
added a commit
that referenced
this pull request
Aug 8, 2024
sentrivana
pushed a commit
that referenced
this pull request
Aug 12, 2024
arjennienhuis
pushed a commit
to arjennienhuis/sentry-python
that referenced
this pull request
Sep 30, 2024
…tsentry#3357) * Expose the scope getters to top level API and use them everywhere * Going forward, we might have 2 different scope implementations so we can't have the `Scope` class being called everywhere directly since this will be abstracted away. * Update CHANGELOG.md Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io> * remove Scope._capture_internal_exception * review fixes * remove staticmethod * Fix sphinx circular import bs --------- Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io>
arjennienhuis
pushed a commit
to arjennienhuis/sentry-python
that referenced
this pull request
Sep 30, 2024
We removed this line in getsentry#3354 since it is no longer needed, but it was apparently accidentally added back in getsentry#3357.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Going forward, we might have 2 different scope implementations for otel so we
can't have the
Scopeclass being called everywhere directly since thiswill be abstracted away.
The refactor had 2 main goals
import Scopeoutside of the main init/api filesScope.*calls outside the actual currentScopeimpl and replace them with top level api everywhere__init__andapifiles since some methods should've been inapi