Skip to content

Conversation

@kcons
Copy link
Member

@kcons kcons commented Nov 12, 2025

It's unclear that this ever worked properly, but presumably this is a narrow enough case that it hasn't been terribly noticeable.
Fixes SENTRY-54ZZ.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 12, 2025
@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/api/endpoints/project_rule_details.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #103204      +/-   ##
===========================================
- Coverage   80.67%    80.67%   -0.01%     
===========================================
  Files        9241      9241              
  Lines      393627    393654      +27     
  Branches    25053     25053              
===========================================
+ Hits       317543    317561      +18     
- Misses      75622     75631       +9     
  Partials      462       462              

@kcons kcons marked this pull request as ready for review November 12, 2025 01:21
@kcons kcons requested review from a team as code owners November 12, 2025 01:21
@kcons kcons requested review from a team and mifu67 November 12, 2025 17:12
kwargs.update({"uuid": client.uuid, "rule_id": rule.id})
find_channel_id_for_rule.apply_async(kwargs=kwargs)
# Serialize Actor object to string identifier for task queueing
task_kwargs = kwargs.copy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to copy the kwargs if this is the last time they could possibly be modified in this function? We'd overwrite the owner from earlier with json serializable version

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to, but it's effectively free and keeps this mutation just for the task, new uses of this dict can be added later without needing to know the task requirements.

@kcons kcons merged commit 70c1f12 into master Nov 12, 2025
67 checks passed
@kcons kcons deleted the kcons/fixweird branch November 12, 2025 18:38
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
…rule (#103204)

It's unclear that this ever worked properly, but presumably this is a
narrow enough case that it hasn't been terribly noticeable.
Fixes SENTRY-54ZZ.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants