Skip to content
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

ref(rules): Handle improperly formatted rule #70797

Merged
merged 2 commits into from
May 14, 2024
Merged

Conversation

ceorourke
Copy link
Member

One customer has a couple improperly formatted alert rules that are causing this error: https://sentry.sentry.io/issues/5245475486/. I've already edited the serializer so it's not possible to get into this state again (we don't save data with a value of "") and had support reach out to the customer to address this, but this issue keeps happening so we can just handle the empty string.

@ceorourke ceorourke requested a review from a team May 13, 2024 19:14
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 13, 2024
@saponifi3d
Copy link
Contributor

nit: it'd be nice to add a test case for this in the future as well (since we've seen it come up in prod already)

@ceorourke
Copy link
Member Author

nit: it'd be nice to add a test case for this in the future as well (since we've seen it come up in prod already)

I can add one, but there is no future possibility of this happening for new rules, it's just this one org's 2 rules.

Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.00%. Comparing base (4b1e575) to head (f9e5bb1).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #70797      +/-   ##
==========================================
- Coverage   80.00%   80.00%   -0.01%     
==========================================
  Files        6507     6507              
  Lines      290833   290855      +22     
  Branches    50123    50132       +9     
==========================================
+ Hits       232686   232703      +17     
- Misses      57711    57716       +5     
  Partials      436      436              
Files Coverage Δ
src/sentry/rules/conditions/event_frequency.py 92.36% <100.00%> (+0.05%) ⬆️

... and 14 files with indirect coverage changes

@ceorourke ceorourke force-pushed the ceorourke/SENTRY-38A8 branch from be8544b to 19e129d Compare May 13, 2024 23:59
@ceorourke ceorourke merged commit 0c724d5 into master May 14, 2024
49 checks passed
@ceorourke ceorourke deleted the ceorourke/SENTRY-38A8 branch May 14, 2024 16:30
Copy link

sentry-io bot commented May 26, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ IntegrityError: UniqueViolation('duplicate key value violates unique constraint "auth_user_username_key"\nDETAIL:... pytest.runtest.protocol tests/snuba/rules/condi... View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants