-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DNM: Size issues e2e demo #104698
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
DNM: Size issues e2e demo #104698
Conversation
chromy
commented
Dec 10, 2025
- feat(preprod): Add empty preprod settings page
- feat(preprod): Link settings page from build{List,Details}
- DNM: Size Issues e2e demo
This is a 'project' settings page behind the organizations:preprod-issues flag. In future it will host the settings for e.g. size analysis issue configuration and similar.
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
| def build(self) -> tuple[IssueOccurrence, dict[str, Any]]: | ||
| id = uuid4() | ||
| assert self.issue_title is not None, "issue_title must be set" | ||
| assert self.issue_title is not None, "issue_subtitle must be set" |
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.
Bug: Wrong variable checked in assertion for issue_subtitle
The assertion on line 19 checks self.issue_title is not None but the error message says "issue_subtitle must be set". This copy-paste error means issue_subtitle is never validated, so it could be None when passed to IssueOccurrence(subtitle=self.issue_subtitle, ...), potentially causing unexpected behavior or errors downstream.
| occurrence=occurrence, | ||
| event_data=event_data, | ||
| ) | ||
| logger.warning("!!!! done") |
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.
Bug: Debug logging statements accidentally committed to production code
Several logger.warning calls with !!!! markers appear to be debug logging statements that were accidentally included. These include messages like "!!!!!!!!!!!!!!!!? issue threshold", "!!!! trigger issue", and "!!!! done" which are not suitable for production logging and will clutter logs.
| issue_title=self.issue_title, | ||
| subtitle=self.issue_subtitle, | ||
| project_id=self.project_id, | ||
| fingerprint=uuid4().hex, |
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.
Bug: Fingerprint passed as string instead of list
The fingerprint parameter is passed as a single string uuid4().hex, but IssueOccurrence expects fingerprint to be a Sequence[str] (a list of strings). All other usages in the codebase pass a list like fingerprint=[...]. This type mismatch will cause issues when the fingerprint is processed downstream.
| "head_artifact_id": self.head_artifact_id, | ||
| "base_artifact_id": self.base_artifact_id, | ||
| }, | ||
| evidence_display={}, |
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.
Bug: Evidence display passed as dict instead of list
The evidence_display parameter is passed as an empty dict {}, but IssueOccurrence expects Sequence[IssueEvidence]. All other usages in the codebase pass an empty list [] when no evidence display is needed. This type mismatch could cause runtime errors when iterating over evidence display.
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |