Skip to content

ref(integrations): Use ensure_integration_enabled decorator#2906

Merged
sentrivana merged 18 commits intosentry-sdk-2.0from
ivana/2.0/new-deco
Mar 28, 2024
Merged

ref(integrations): Use ensure_integration_enabled decorator#2906
sentrivana merged 18 commits intosentry-sdk-2.0from
ivana/2.0/new-deco

Conversation

@sentrivana
Copy link
Copy Markdown
Contributor

@sentrivana sentrivana commented Mar 26, 2024 •
edited
Loading

Using the ensure_integration_enabled decorator which now supports not providing a function.

This is a best effort PR. We can't use the decorator everywhere.

  • For instance, in the Mongo integration and couple more places, the integration class hasn't been defined yet at the point in time where we want to use the decorator. We can't move the definition up either because of interdependencies between things.
  • Sometimes, applying the decorator makes the tests fail (this is the fix for one of those cases). Also e.g. in Starlite or one place in Starlette.

@sentrivana sentrivana changed the title ref(integrations): Use ensure_integration_enabled decorator ref(integrations): Use ensure_integration_enabled decorator Mar 26, 2024
@sentrivana sentrivana added this to the Better async support milestone Mar 28, 2024
@sentrivana sentrivana marked this pull request as ready for review March 28, 2024 09:44
@sentrivana sentrivana removed this from the Better async support milestone Mar 28, 2024
@sentrivana
Copy link
Copy Markdown
Contributor Author

sentrivana commented Mar 28, 2024

I'm aware of the failing Lint step but very confused by it:

sentry_sdk/integrations/flask.py:79: error: Untyped decorator makes function "sentry_patched_wsgi_app" untyped  [misc]
  1. I didn't even touch this part of the code in this PR.
  2. The decorator is properly typed...

antonpirker
antonpirker approved these changes Mar 28, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Yeah!

Just some small nitpicking

szokeasaurusrex
szokeasaurusrex approved these changes Mar 28, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Looks generally good to me, please check @antonpirker's comments before merging

@sentrivana sentrivana merged commit b38f9c7 into sentry-sdk-2.0 Mar 28, 2024
@sentrivana sentrivana deleted the ivana/2.0/new-deco branch March 28, 2024 15:22
@andersk andersk mentioned this pull request May 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szokeasaurusrex szokeasaurusrex szokeasaurusrex approved these changes

+1 more reviewer

@antonpirker antonpirker antonpirker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sentrivana @antonpirker @szokeasaurusrex