-
Notifications
You must be signed in to change notification settings - Fork 215
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
Remove sentry exception capture for thumbnail errors #3709
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some suggestions but the PR works fine as it is, so I won't block.
Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR: @sarayourfriend Excluding weekend1 days, this PR was ready for review 2 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)2. @AetherUnbound, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto to Dhruv's comments. I'll approve with the changes to remove the unused code, but the test change is a non-blocker for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, meant to request changes for the unused code cleanup from Dhruv's comment.
2991688
to
2554558
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes
Fixes #3708 by @AetherUnbound
Description
This PR removes the Sentry-shipping of errors that occur in thumbnails on the API. It also removes settings related to that effect.
Testing Instructions
SENTRY_DSN
environment variable to point to Sentry.just api/init
and then navigate to http://localhost:50270/v1/images/5c173952-6a45-4380-b9e4-c935263730ab/thumb/just docker/cache/cli -n 3
, thenKEYS thumbnail_error:builtins.ValueError*
and observe that a key is present in the tally databaseChecklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin