ref(boto3): Split integration into internal modules - #7537
pabloDeputter wants to merge 3 commits into
Conversation
Codecov Results 📊✅ 126963 passed | ❌ 59 failed | ⏭️ 7165 skipped | Total: 134187 | Pass Rate: 94.62% | Execution Time: 436m 2s ❌ Failed Tests
|
| File | Patch % | Lines |
|---|---|---|
| sentry_sdk/integrations/boto3/init.py | 87.50% | |
| sentry_sdk/integrations/boto3/_client.py | 91.67% |
Coverage diff
@@ Coverage Diff @@
## master #PR +/-##
==========================================
- Coverage 90.22% 90.19% -0.03%
==========================================
Files 194 197 +3
Lines 25997 26022 +25
Branches 9662 9662 —
==========================================
+ Hits 23454 23468 +14
- Misses 2543 2554 +11
- Partials 1451 1451 —Generated by Codecov Action
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bf4bb5d. Configure here.
ericapisani
left a comment
ericapisani
left a comment
There was a problem hiding this comment.
Thanks for making the earlier changes. There's one more change that we can try and make to avoid the need for dynamic imports altogether, have left a comment within one of the methods explaining.
Otherwise, this LGTM, approving in order to not block once that's been addressed
| def _sentry_before_sign( | ||
| request: "AWSRequest", signature_version: "Any", **kwargs: "Any" | ||
| ) -> None: | ||
| from sentry_sdk.integrations.boto3 import Boto3Integration |
There was a problem hiding this comment.
A fun thing that you can do to try and get around the need for this dynamic import - you can provide the string rather than the class to client.get_integration.
So I think lines 117 and 118 below would look like this:
if client.get_integration("Boto3Integration") is None:
return
There was a problem hiding this comment.
Okay good to know, I didn't knew this was possible. Then we'll probably use client.get_integration("boto3") instead, right? Since we use boto3 as identifier for the integration.
There was a problem hiding this comment.
I'll do this probably in a later PR, cause I don't want to do merge hell once again 😭
4c7d088 to
5bbe368
Compare
5bbe368 to
591151d
Compare
This branch has not been deployed
3 participants
Description
Move
Boto3Integrationfrom single module into package with smaller internal modules._client.py._instrumentation.py.from sentry_sdk.integrations.boto3 import Boto3Integration.