chore(issues): Expand stronger typing enforcement for issues #69374
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We're can't quite fully enable sentry.issues.* but since we already have a lot of files already passing let's get them enabled. This also has a couple of trivial improvements for some endpoints I noticed along the way.
github-actions
bot
added
the
Scope: Backend
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #69374 +/- ##
==========================================
+ Coverage 78.66% 79.67% +1.00%
==========================================
Files 6622 6453 -169
Lines 294737 286637 -8100
Branches 49377 49379 +2
==========================================
- Hits 231855 228373 -3482
+ Misses 62460 57842 -4618
Partials 422 422
|
armenzg
reviewed
Apr 24, 2024
| "sentry_plugins.base", | ||
| "tests.sentry.api.endpoints.issues.*", | ||
| "tests.sentry.grouping.test_fingerprinting", | ||
| "tests.sentry.issues.endpoints.test_actionable_items", |
There was a problem hiding this comment.
You can use tests.sentry.issues.* if you've covered them all.
There was a problem hiding this comment.
Not yet unfortunately, same for sentry.issues.*
| "sentry.grouping.strategies", | ||
| "sentry.grouping.strategies.message", | ||
| "sentry.issues.analytics", | ||
| "sentry.issues.apps", |
There was a problem hiding this comment.
You can use sentry.issues.* if you have covered all of them.
| option_name: str # Set in subclasses | ||
|
|
||
| def _get_config_id(self, project): | ||
| def _get_config_id(self, project: Project): |
There was a problem hiding this comment.
You need a return type. Basically every time there's no ) -> :
There was a problem hiding this comment.
Yeah - most of these were cases where the typing was more complex than a 30s change, this at least gets us closer
| return f"[ {self.inner.description} ] |" | ||
|
|
||
| def _to_config_structure(self, version): | ||
| def _to_config_structure(self, version) -> str: |
There was a problem hiding this comment.
I'm surprised that version is not typed and it is not complaining.
|
Closing this as I'll be pulling out pieces of it into separate PRs |
mrduncan
added a commit
that referenced
this pull request
May 7, 2024
…#69828) `sentry.issues.*` and `test.sentry.issues.*` are not close to passing but in the mean time we can get incremental benefits and prevent regressions by opting in modules which are already passing. #69374 includes a bit more details and outlines additional fixes we can follow up this change with.
iamrajjoshi
added a commit
that referenced
this pull request
May 8, 2024
commit 1354478 Author: Raj Joshi <[email protected]> Date: Wed May 8 09:56:24 2024 -0700 feat(chartcuterie): Change Chart Stylings (#70489) two quick updates: 1. Changes the label for regressed to a darker color so we can read it better  2. Updated some chart styling for slack so the legend is on the left  commit 56514bc Author: Catherine Lee <[email protected]> Date: Wed May 8 12:47:19 2024 -0400 ref(replay): Rage click clicked element name (#70493) Since we now use react component names in the selector path, we should modify clicked element to provide more specific info such as class, role etc. This prevents giving the react component name twice and would give more info for debugging since react component names aren't very specific Before: <img width="845" alt="image" src="https://github.com/getsentry/sentry/assets/55311782/87b6c8cd-1b29-40bb-b448-715595cb255b"> commit 35b38d9 Author: Steven Eubank <[email protected]> Date: Wed May 8 18:38:57 2024 +0200 Add Deno Runtime Icon (#69100) QoL, this has been bugging me Should show the Deno icon, from Deno runtimes hosted on Supabase and others after:   commit a96f1ff Author: Evan Purkhiser <[email protected]> Date: Wed May 8 12:34:37 2024 -0400 ref(crons): Improve details legend (#70515) The legend now differentiates between timeout and failed <img alt="clipboard.png" width="267" src="https://i.imgur.com/yQAfXaB.png" /> --------- Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com> commit c3b9fdb Author: Matt Duncan <[email protected]> Date: Wed May 8 09:23:28 2024 -0700 chore(issues): Enable stronger typing on two endpoints (#70488) This is another quick follow up to #69828 since these two type errors are trivial to resolve. commit dbc926a Author: Evan Purkhiser <[email protected]> Date: Wed May 8 12:22:14 2024 -0400 ref(crons): Move timezone to schedule text (#70511) <img width="295" alt="image" src="https://github.com/getsentry/sentry/assets/1421724/fd919e5b-0dd6-4b41-8e84-d3b99dfe6042"> It is now next to the schedule. This is more logical commit 9a90b57 Author: Evan Purkhiser <[email protected]> Date: Wed May 8 12:18:49 2024 -0400 ref(crons): Use constant for DEFAULT_CHECKIN_MARGIN (#70510) commit f161ebb Author: Colleen O'Rourke <[email protected]> Date: Wed May 8 09:16:58 2024 -0700 ref(daily summary): Disable notification (#70295) I don't currently have any time to dedicate to bug fixing and it's been sending multiple times, so this is going to be disabled for now. We'll likely have to keep track of which users per org received the notification and check that before sending, and then clear it out every hour to avoid duplicate sending. commit 7d1cf85 Author: Matt Duncan <[email protected]> Date: Wed May 8 08:58:45 2024 -0700 chore(issues): Enable stronger typing on occurrence_consumer (#70487) This is a quick follow up to #69828 since these two type errors are trivial to resolve. commit 6b530ff Author: colin-sentry <[email protected]> Date: Wed May 8 11:50:48 2024 -0400 chore(ai-monitoring): Add a unit to total cost (#70484) commit 50b4ed3 Author: Evan Purkhiser <[email protected]> Date: Wed May 8 11:32:28 2024 -0400 ref(routes): A few more routes using withOrgPath (#70449) commit 4d124bd Author: Evan Hicks <[email protected]> Date: Wed May 8 11:19:07 2024 -0400 fix: Add a metric for non-success Snuba requests (#70452) Tracking this on the Sentry side allows alerts to be created that are separate from the Snuba API itself, in case the API is in a broken state and can't accurately report what is happening. commit 11a80a3 Author: Dominik Buszowiecki <[email protected]> Date: Wed May 8 10:36:32 2024 -0400 feat(cache): add average transaction duration to sample sidebar (#70445) New metric readout for avg transaction duration in sample sidebar <img width="746" alt="image" src="https://github.com/getsentry/sentry/assets/44422760/35146610-5c41-4e12-917e-e58310da84a9"> commit eb4de59 Author: Ash <[email protected]> Date: Wed May 8 10:25:30 2024 -0400 feat(perf): Add backend referrers for span summary and span metrics (#70466) Adds backend referrers to be used on the new span summary and span metrics pages Relevant PR: #69159 commit d3d6bdf Author: Mark Story <[email protected]> Date: Wed May 8 10:20:37 2024 -0400 chore(actor) Remove Actor model from django state (#70439) Remove the Actor model from django state. Refs HC-1183 commit 5bff933 Author: anthony sottile <[email protected]> Date: Wed May 8 08:34:04 2024 -0400 ref: remove unused partition parameter from buffer (#70441) <!-- Describe your PR here. --> commit 7512ed6 Author: Yagiz Nizipli <[email protected]> Date: Wed May 8 07:29:26 2024 -0400 perf: use orjson in all middlewares (#70456) commit 4623b5d Author: Tony Xiao <[email protected]> Date: Wed May 8 06:07:45 2024 -0400 fix(trace-explorer): Date range narrowing condition is backwards (#70496) This was changing the end timestamp to be too narrow and missing some spans. commit fc8b666 Author: Ryan Hiebert <[email protected]> Date: Wed May 8 02:33:09 2024 -0500 Use last forwarded IP (#68884) Well-behaved forwarders will append the IP they're forwarding for to an existing list. In the most typical case, this means that only the last one is trustworthy from a spoofing request-maker. And all this is assuming that the proxy itself is trusted. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#security_and_privacy_concerns <!-- Describe your PR here. --> <!-- Sentry employees and contractors can delete or ignore the following. --> ### Legal Boilerplate Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. commit 01160aa Author: Katie Byers <[email protected]> Date: Tue May 7 16:41:32 2024 -0700 ref(seer-grouping): Switch to using `hash` and `parent_hash` from `group_hash` and `parent_group_hash` (#70383) As we've been thinking about the switch from sending and receiving group ids when communicating with Seer to doing so with hashes, all along we've been talking about those hashes as "group hashes." In truth, though, hash values are based on the data in a particular event (not the group overall), and indeed, that's how we're using them in Seer - pairing up hashes not with what group they're in but which event data they represent. There _is_ a pairing of groups and hashes - on the Sentry side, in the form of the `GroupHash` table - but entries from that table aren't what we're using with Seer. With Seer, we only care about the "hash" part of `GroupHash`. So, both for accuracy and so as to be able to differentiate in Seer-related Sentry code between hashes (hex strings) and grouphashes (association table records), we're switching from using `group_hash` and `parent_group_hash` to using `hash` and `parent_hash`. This PR makes the change on the Sentry side. Fortunately, nothing in Seer is yet relying on hashes, so as long as we wait for this to go live, we can then add hash support on the seer side using the new names from the get-go. commit 173b690 Author: Raj Joshi <[email protected]> Date: Tue May 7 16:32:23 2024 -0700 fix(chartcuterie): Added Visual Map Field for Endpoint Regression (#70477) There is mismatch in the way we build the EChart Options object in our FE code and how Chartcuterie handles it. In our FE [code,](https://github.com/getsentry/sentry/blob/master/static/app/components/events/eventStatisticalDetector/breakpointChartOptions.tsx#L89-L104), we wrap the visualMap object in an extra option, which allows us to maintain the hierarchy for styling. However, Chartcuterie cannot handle the wrapped object, so when we pass the service the options, we unwrap it. I also created a modifier option to modify chart options specifically for slack and removed the icon from the legend icon from there.  commit e624ee9 Author: Matt Duncan <[email protected]> Date: Tue May 7 16:17:45 2024 -0700 chore(issues): Opt in already passing issues files to stronger typing (#69828) `sentry.issues.*` and `test.sentry.issues.*` are not close to passing but in the mean time we can get incremental benefits and prevent regressions by opting in modules which are already passing. #69374 includes a bit more details and outlines additional fixes we can follow up this change with. commit dbf524c Author: Seiji Chew <[email protected]> Date: Tue May 7 15:58:22 2024 -0700 fix(ui): Revert to using project release commmit API (#70485) In #63860 we switched from a class component to a FC and also switched from hitting `ProjectReleaseCommitsEndpoint` to `OrganizationReleaseCommitsEndpoint`. The latter doesn't respect repo name or id in the query param, which makes it so selecting a repo in the dropdown will show you commits from other repos (and also messes up pagination as well, not showing you all commits). Related to #70411 commit 0b96de4 Author: Dan Fuller <[email protected]> Date: Tue May 7 15:53:42 2024 -0700 chore(crons): Rename badly named api file (#70480) I accidentally pluralised this when creating it, just fixing. commit 2489a4a Author: Dominik Buszowiecki <[email protected]> Date: Tue May 7 18:19:21 2024 -0400 fix(cache): update docs link (#70475) The link to the docs will be plural commit 7ea571e Author: Katie Byers <[email protected]> Date: Tue May 7 15:17:08 2024 -0700 chore(events): Move `PLACEHOLDER_EVENT_TITLES` to a neutral location (#70470) I tried to use `PLACEHOLDER_EVENT_TITLES` in an upcoming PR, and landed in a circular dependency with `event_manager.py`, where it currently lives. This moves it to `constants.py`, which solves the problem. commit a97dccf Author: Michael Sun <[email protected]> Date: Tue May 7 18:11:47 2024 -0400 chore(issue-stream): Register feature flag for upcoming changes to issues stream events graph (#70471) [Project details](#69691) * [SOA PR ](https://github.com/getsentry/sentry-options-automator/pull/1378) * ~~getSentry PR~~ (No longer necessary) commit fadd0b5 Author: Evan Purkhiser <[email protected]> Date: Tue May 7 18:00:45 2024 -0400 fix(ui): Use new illustration for tracing keyboard shortcuts (#70474) <img alt="clipboard.png" width="665" src="https://i.imgur.com/h8p3E1H.png" /> Looks like the rest of the product now commit 277f026 Author: Scott Cooper <[email protected]> Date: Tue May 7 14:54:57 2024 -0700 feat(metrics): Add metricSecond to allowed category (#70442) commit ef6c79a Author: Evan Purkhiser <[email protected]> Date: Tue May 7 17:50:52 2024 -0400 Revert "ref(crons): Normalize crons incident issues (#70289)" (#70469) This reverts commit 9d56889. commit 45d2d2e Author: Cathy Teng <[email protected]> Date: Tue May 7 14:46:55 2024 -0700 feat(slack): EA :white_circle: for actions by adding to issue alert threads FF (#70468) commit 80ed536 Author: Tony Xiao <[email protected]> Date: Tue May 7 17:25:18 2024 -0400 fix(trace-explorer): Breakdown by project and sdk (#70463) For full stack projects that use a single project for all the data, we need to break it down further using sdk. commit 874db7e Author: Katie Byers <[email protected]> Date: Tue May 7 14:24:19 2024 -0700 ref(seer-grouping): Send group hash to Seer (#70244) This adds the group hash to the outgoing payload sent to Seer. Once this is merged, it will be safe to remove the Seer logic handling an incoming group id, since we'll now be able to rely on the group hash being present. Taken together, this PR and the ones leading up to it (links below) mean will mean that on the Sentry side we'll be sending in the request and handling in the response both hash and group id (and transforming hashes in the response into group ids if hashes are all that's sent). Once Seer is adjusted to only deal in hashes, we can then remove the sending and handling of group ids from Sentry and the transition will be complete. Other PRs which are part of the group-id-to-group-hash switch: - #70005, #70236, and #70237 - various small fixes and tweaks - #70070 and #70238 - updates to associated types - #70240 - automatic conversion from hash to group id when handling Seer similar group data commit 78d2f28 Author: Evan Purkhiser <[email protected]> Date: Tue May 7 17:23:38 2024 -0400 ref(routes): Prioritize customer domains route (#70451) In a936fb0 we introduced a new withOrgPath prop to the Route component. This caused the route to have the same behaviour as doing ```tsx <Framgnet> {USING_CUSTOMER_DOMAIN && ( <Route path="/some-path/" component={withDomainRequired(make(() => import('sentry/views/someView')))} /> )} <Route path="/organizations/:orgId/some-path/" component={withDomainRedirect(make(() => import('sentry/views/someView')))} /> </Framgnet> ``` However the introduced logic generated the routes in the opposite order, where the org slug version would get priority. For some future changes this becomes important so I am bringing back this matching behaviour commit 3a7e9b3 Author: Evan Purkhiser <[email protected]> Date: Tue May 7 17:23:31 2024 -0400 fix(crons): Teams -> Owners (#70464) commit 2dbccba Author: anthony sottile <[email protected]> Date: Tue May 7 17:13:26 2024 -0400 ref: upgrade djangorestframework-stubs (#70461) upgrading this has no differences in ignored mypy errors <!-- Describe your PR here. --> commit 0c8f916 Author: Mark Story <[email protected]> Date: Tue May 7 16:49:43 2024 -0400 chore(database) Drop tables for project and team avatar (#68616) These tables no longer have django models, and can be deleted. commit 4f71b8e Author: Leander Rodrigues <[email protected]> Date: Tue May 7 16:39:00 2024 -0400 feat(highlights): Variety of fixes/changes to highlights work (#70355) This PR makes a variety of changes to the highlights/tags areas: - [x] Add tests for analytics and user friendly names - [x] Add search to the highlights modal  - [x] Allow `replayId`, `transaction` and tags with URLs to be clickable  - [x] Adds a feedback button on Highlight section  **todo** - [x] Add tests for search - [x] Add tests for new tag links - [x] Add screenshots of changes commit aa40406 Author: Snigdha Sharma <[email protected]> Date: Tue May 7 13:36:54 2024 -0700 fix(escalating-issues): Cleanup query for escalating forecasts (#70443) Cleans up some cruft left behind by feature flags and pushes the escalation check out of the query to help with timeouts. Fixes SENTRY-18AS commit 81d3fef Author: Seiji Chew <[email protected]> Date: Tue May 7 13:32:10 2024 -0700 nit: Remove authenticators from state (#70365) Remove `authenticators` from state because we only set it once
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal here is to get some early eyes on this, I plan to split this up into separate PRs if we're all good with it.
I had some time on a flight and took at quick pass at what is already passing here but not opted into the stronger type checking. I'm not aware of any reason not to do this, but open to thoughts. We can't quite fully enable
sentry.issues.*ortests.sentry.issues.*yet.This also has a number of trivial improvements I noticed along the way.