Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Always set _spotlight_url #4186

Merged
merged 4 commits into from
Mar 24, 2025
Merged

fix: Always set _spotlight_url #4186

merged 4 commits into from
Mar 24, 2025

Conversation

BYK
Copy link
Member

@BYK BYK commented Mar 21, 2025

The conditional early exit in SpotlightMiddleware may cause attribute access errors when trying to check if _spotlight_url is set or not. This patch sets it to None explicitly at class level.

The conditional early exit in `SpotlightMiddleware` may cause attribute access errors when trying to check if `_spotlight_script` is set or not. This patch sets it to `None` explicitly and always in the constructor to avoid/fix the issue.
@BYK BYK requested a review from a team as a code owner March 21, 2025 15:32
@BYK BYK enabled auto-merge (squash) March 21, 2025 15:32
Copy link

codecov bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.55%. Comparing base (f8ec572) to head (cc69ef1).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/spotlight.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4186      +/-   ##
==========================================
- Coverage   79.59%   79.55%   -0.05%     
==========================================
  Files         141      141              
  Lines       15736    15737       +1     
  Branches     2675     2675              
==========================================
- Hits        12525    12519       -6     
- Misses       2367     2374       +7     
  Partials      844      844              
Files with missing lines Coverage Δ
sentry_sdk/spotlight.py 56.75% <33.33%> (+0.39%) ⬆️

... and 2 files with indirect coverage changes

@BYK
Copy link
Member Author

BYK commented Mar 21, 2025

Test fail looks unrelated?

@BYK BYK changed the title fix: Always set _spotlight_script fix: Always set _spotlight_url Mar 21, 2025
@BYK BYK disabled auto-merge March 21, 2025 16:11
@BYK BYK enabled auto-merge (squash) March 21, 2025 16:11
@BYK BYK merged commit fafe8f6 into master Mar 24, 2025
139 of 140 checks passed
@BYK BYK deleted the byk/fix/spotlight_script branch March 24, 2025 08:58
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.

2 participants