ref(arq): Use new scopes API #2878
Conversation
sentry_sdk/integrations/arq.py
Outdated
| return await coroutine(ctx, *args, **kwargs) | ||
|
|
||
| hub.scope.add_event_processor( | ||
| Scope.get_current_scope().add_event_processor( |
There was a problem hiding this comment.
Are we sure this event processor belongs on the scope, and not the isolation scope?
There was a problem hiding this comment.
This I wasn't sure about. Thinking about it again, I think isolation scope makes more sense. Will change.
There was a problem hiding this comment.
There's also another current_scope a bit up. I think this is what confused me. It feels like both should be isolation scopes. What do you think @szokeasaurusrex @antonpirker?
There was a problem hiding this comment.
I think event processors should be on the isolation scope, because they should be valid for the whole request.
There was a problem hiding this comment.
I think what @antonpirker said makes sense
There was a problem hiding this comment.
Tried it out now and changing either of them (or both) to work with the isolation scope makes the tests fail. 🙃 Investigating...
There was a problem hiding this comment.
Ok this works: 9d197a3
- the current transaction is saved on the current scope, so all scopes in the arq integration that try to update transaction attrs also have to go to the current scope
- the event processor itself is now on the isolation scope
General Notes
Thank you for contributing to
sentry-python!Please add tests to validate your changes, and lint your code using
tox -e linters.Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.
For maintainers
Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.
Before running sensitive test suites, please carefully check the PR. Then, apply the
Trigger: tests using secretslabel. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.