Skip to content

fix(crons): Don't drop invalid schema when writing checkin rows #97945

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

Merged
merged 3 commits into from
Aug 15, 2025

Conversation

wedamija
Copy link
Member

We had a bug where we accidentally didn't save all the required fields for the monitor config. This causes a bug when we copy the config over to the monitor checkin where we end up storing a null value.

We should instead treat this as a warning - if the config is already stored in the monitor, it should be replicated regardless of whether it's valid or not. The validation should happen at mutation time, which is being fixed in #97943

Fixes SENTRY-4DDN

We had a bug where we accidentally didn't save all the required fields for the monitor config. This causes a bug when we copy the config over to the monitor checkin where we end up storing a null value.

We should instead treat this as a warning - if the config is already stored in the monitor, it should be replicated regardless of whether it's valid or not. The validation should happen at mutation time, which is being fixed in #97943

Fixes SENTRY-4DDN
@wedamija wedamija requested a review from a team as a code owner August 15, 2025 18:53
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 15, 2025
Comment on lines +375 to +376
# We should always return the config here - just log an error if we detect that it doesn't
# match the schema
Copy link
Member Author

Choose a reason for hiding this comment

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

To some extent I wonder if this validation is even useful at this step... I guess it's good to know if somehow invalid configs are getting into the system so we can fix them

Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #97945      +/-   ##
==========================================
+ Coverage   79.55%   80.63%   +1.08%     
==========================================
  Files        8593     8595       +2     
  Lines      379066   379155      +89     
  Branches    24700    24700              
==========================================
+ Hits       301558   305728    +4170     
+ Misses      77130    73049    -4081     
  Partials      378      378              

@wedamija wedamija merged commit 1d78531 into master Aug 15, 2025
64 checks passed
@wedamija wedamija deleted the danf/crons-dont-drop-incorrectly-validated-schema branch August 15, 2025 22:15
evanh pushed a commit that referenced this pull request Aug 18, 2025
We had a bug where we accidentally didn't save all the required fields
for the monitor config. This causes a bug when we copy the config over
to the monitor checkin where we end up storing a null value.

We should instead treat this as a warning - if the config is already
stored in the monitor, it should be replicated regardless of whether
it's valid or not. The validation should happen at mutation time, which
is being fixed in #97943

Fixes SENTRY-4DDN
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.

2 participants