chore(cells) Add deprecation notices for a few issue endpoints #104471
Conversation
As part of the cells project we need to deprecate URLs that don't have organization_slug_or_id in them. This change adds deprecations to a few issues endpoints for the org-less URL. Refs INFRENG-208
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| class OrganizationGroupSuspectFlagsEndpoint(GroupEndpoint): | ||
| publish_status = {"GET": ApiPublishStatus.PRIVATE} | ||
|
|
||
| @deprecated(CELL_API_DEPRECATION_DATE, url_names=["sentry-api-0-suspect-flags"]) | ||
| def get(self, request: Request, group: Group) -> Response: | ||
| """Stats bucketed by time.""" | ||
| if not features.has( |
There was a problem hiding this comment.
Bug: The deprecated decorator uses incorrect url_names, causing the deprecation logic to be entirely skipped for the target endpoints.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The @deprecated decorator is configured with url_names such as "sentry-api-0-suspect-flags" and "sentry-api-0-suspect-tags". However, the actual registered URL names, as constructed by create_group_urls(), are "sentry-api-0-organization-group-suspect-flags" and "sentry-api-0-organization-group-suspect-tags". This mismatch causes the url_name in url_names check within the decorator to always evaluate to False, leading to the deprecation logic being entirely skipped. Consequently, deprecation headers are not added, brownout blocking is not applied, and deprecation metrics are not incremented.
💡 Suggested Fix
Update the url_names parameter in the @deprecated decorator to match the full registered URL names, for example, url_names=["sentry-api-0-organization-group-suspect-flags"].
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/issues/endpoints/organization_group_suspect_flags.py#L34-L40
Potential issue: The `@deprecated` decorator is configured with `url_names` such as
`"sentry-api-0-suspect-flags"` and `"sentry-api-0-suspect-tags"`. However, the actual
registered URL names, as constructed by `create_group_urls()`, are
`"sentry-api-0-organization-group-suspect-flags"` and
`"sentry-api-0-organization-group-suspect-tags"`. This mismatch causes the `url_name in
url_names` check within the decorator to always evaluate to `False`, leading to the
deprecation logic being entirely skipped. Consequently, deprecation headers are not
added, brownout blocking is not applied, and deprecation metrics are not incremented.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5887053
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #104471 +/- ##
===========================================
+ Coverage 80.51% 80.53% +0.01%
===========================================
Files 9348 9349 +1
Lines 399919 400009 +90
Branches 25651 25651
===========================================
+ Hits 322005 322143 +138
+ Misses 77466 77418 -48
Partials 448 448 |
| :param url_names: A list of URL names that are deprecated if an endpoint has multiple URLs | ||
| and you need to deprecate one of the URLs. |
There was a problem hiding this comment.
note: not a blocker, but this feels a pretty circular to me.
in general i think urls are supposed to reference views rather than the other way round
If possible, I would probably prefer to go the other way and create a new view (which subclasses + reuses the other one) with the correct deprecation markers, or maybe write a helper function that dynamically attaches the deprecations to the original view...
curious what you think?
As part of the cells project we need to deprecate URLs that don't have organization_slug_or_id in them. This change adds deprecations to a few issues endpoints for the org-less URL.
I've only updated a couple endpoints to start with so that I can validate behavior before updating the remaining issue endpoints.
Refs INFRENG-208