fix(auto_source_config): Pass group_id to avoid Snuba call #83650
Conversation
In #83475 we started passing the `event_id` instead of the Node data to prevent passing pickled data. Unfortunately, that causes a Snuba call in order to determine the group ID. If we pass the group_id when scheduling the task we won't need to call Snuba.
github-actions
bot
added
the
Scope: Backend
|
where do we actually use the group_id that's passed in? or is that to be done in a future PR |
| "event_id": event_id, | ||
| } | ||
|
|
||
| event = eventstore.backend.get_event_by_id(project_id, event_id, group_id=group_id) |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #83650 +/- ##
==========================================
- Coverage 87.63% 87.59% -0.04%
==========================================
Files 9490 9492 +2
Lines 541208 539139 -2069
Branches 21232 21180 -52
==========================================
- Hits 474272 472278 -1994
+ Misses 66588 66516 -72
+ Partials 348 345 -3 |
| @@ -85,27 +85,34 @@ def process_error(error: ApiError, extra: dict[str, str]) -> None: | |||
| default_retry_delay=60 * 10, | |||
| max_retries=3, | |||
| ) | |||
| def auto_source_code_config(project_id: int, event_id: str, **kwargs: Any) -> None: | |||
| def auto_source_code_config( | |||
| project_id: int, event_id: str, group_id: int | None = None, **kwargs: Any | |||
There was a problem hiding this comment.
@wedamija Made it optional.
| extra = { | ||
| "organization.slug": org.slug, | ||
| "project_id": project_id, | ||
| "group_id": group_id, |
There was a problem hiding this comment.
do you have to access it using kwargs["group_id"] too? or is making it optional in the signature enough?
There was a problem hiding this comment.
Let us not find out the wrong way.
I have changed it to None by default.
There was a problem hiding this comment.
It's fine to have it in the signature with = None
| extra = { | ||
| "organization.slug": org.slug, | ||
| "project_id": project_id, | ||
| "group_id": group_id, |
There was a problem hiding this comment.
It's fine to have it in the signature with = None
| @@ -1017,7 +1017,7 @@ def process_code_mappings(job: PostProcessJob) -> None: | |||
| else: | |||
| return | |||
|
|
|||
| auto_source_code_config.delay(project.id, event_id=event.event_id) | |||
| auto_source_code_config.delay(project.id, event_id=event.event_id, group_id=group_id) | |||
There was a problem hiding this comment.
Just make sure that the **kwargs in the definition is already deployed before this deploys. I assume it is, just want to be doubly sure
In #83475 we started passing the `event_id` instead of the Node data to prevent passing pickled data. Unfortunately, that causes an extra Snuba call (which can fail) to determine the group ID. If we pass the group_id when scheduling the task we won't need to call Snuba. Fixes [SENTRY-3MGG](https://sentry.sentry.io/issues/6222497528/)
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
In #83475 we started passing the
event_idinstead of the Node data to prevent passing pickled data.Unfortunately, that causes an extra Snuba call (which can fail) to determine the group ID.
If we pass the group_id when scheduling the task we won't need to call Snuba.
Fixes SENTRY-3MGG