Skip to content

feat(integration): Add support for Sanic >=21.3#1146

Merged
ahmedetefy merged 8 commits into
masterfrom
ahmed-sanic-integration-latest
Jul 8, 2021
Merged

feat(integration): Add support for Sanic >=21.3#1146
ahmedetefy merged 8 commits into
masterfrom
ahmed-sanic-integration-latest

Conversation

@ahmedetefy

@ahmedetefy ahmedetefy commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

This PR:

  • Adds support for latest Sanic versions >= 21.3 because it was initially broken due to breaking changes after LTS of 20.12

Note/Todo:

  • There MIGHT be missing support for new added functionality in version 21.3 that should be integrated in future work
  • This PR is merely there to not break support for what we already have

Fixes: #1066, #1128

Comment thread sentry_sdk/integrations/sanic.py Outdated
Comment thread tox.ini Outdated
@ahmedetefy
ahmedetefy requested a review from untitaker July 7, 2021 10:49
Comment thread tests/integrations/sanic/test_sanic.py Outdated
Comment thread tox.ini Outdated
@ahmedetefy
ahmedetefy requested a review from AbhiPrasad July 7, 2021 12:32
@rhcarvalho
rhcarvalho removed their request for review July 7, 2021 12:33
@ahmedetefy
ahmedetefy force-pushed the ahmed-sanic-integration-latest branch 2 times, most recently from 35a8ed9 to 7f9a482 Compare July 7, 2021 12:41
@ahmedetefy
ahmedetefy force-pushed the ahmed-sanic-integration-latest branch from 7f9a482 to d266c21 Compare July 7, 2021 12:42
Comment thread sentry_sdk/integrations/sanic.py Outdated
sanic_app_name = self.ctx.app.name
scope.transaction = rv[0].name.split(
"%s." % sanic_app_name
)[1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could the name.split list length ever be 0 or 1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

self.name cannot be null

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will add safe guards regardless

@ahmedetefy
ahmedetefy requested a review from AbhiPrasad July 7, 2021 13:43
@ahmedetefy
ahmedetefy force-pushed the ahmed-sanic-integration-latest branch from e2ea2d1 to c876801 Compare July 7, 2021 13:45

@AbhiPrasad AbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's ship to unblock users.

@ahmedetefy
ahmedetefy merged commit 73bb478 into master Jul 8, 2021
@ahmedetefy
ahmedetefy deleted the ahmed-sanic-integration-latest branch July 8, 2021 06:15
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.

3 participants