fix(uptime): Allow strings in config thresholds#104706
Merged
evanpurkhiser merged 1 commit intomasterfrom Dec 12, 2025
Merged
Conversation
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/in/12637?s=60&v=4){kind=small}
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
6211ae2 to
311d266
Compare
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
311d266 to
53215e3
Compare
src/sentry/workflow_engine/endpoints/validators/base/detector.py
Outdated
Show resolved
Hide resolved
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
53215e3 to
5b3b5a7
Compare
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
5b3b5a7 to
55f6954
Compare
wedamija
reviewed
Dec 11, 2025
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
55f6954 to
29d1339
Compare
Comment on lines
+531
to
+541
| elif ( | ||
| mode_in_request | ||
| and requested_mode != current_mode | ||
| and requested_mode != UptimeMonitorMode.MANUAL | ||
| and not is_superuser | ||
| ): | ||
| raise serializers.ValidationError({"mode": ["Only superusers can modify `mode`"]}) | ||
| else: | ||
| # On create, non-superusers can only set MANUAL mode | ||
| if "mode" in attrs and attrs["mode"] != UptimeMonitorMode.MANUAL and not is_superuser: | ||
| raise serializers.ValidationError({"mode": ["Only superusers can modify `mode`"]}) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Member
Author
There was a problem hiding this comment.
This was correct. Fixed now
saponifi3d
reviewed
Dec 12, 2025
Contributor
saponifi3d
left a comment
saponifi3d
left a comment
There was a problem hiding this comment.
seems 👍 to me from the detector platform perspective; i'm not sure about the details involved so probably good to have dan give it the final +1.
| class UptimeDomainCheckFailureValidator(BaseDetectorTypeValidator): | ||
| enforce_single_datasource = True | ||
| data_sources = serializers.ListField(child=UptimeMonitorDataSourceValidator(), required=False) | ||
| config = UptimeDomainCheckFailureConfigValidator(required=False) # type: ignore[assignment] |
Contributor
There was a problem hiding this comment.
any thoughts on how we might be able to have a similar abstraction enforced through the base detector? my guess is that most detectors will have the not great error handling for the json config field.
it'd also be cool if we updated the base detector to allow for this kind of override without having to add a lint exception. 🤔
anywho, no action needed for this pr, just curious to get your take.
Fixes [NEW-664: Fix setting failure threshold in uptime detector creation](https://linear.app/getsentry/issue/NEW-664/fix-setting-failure-threshold-in-uptime-detector-creation)
evanpurkhiser
force-pushed
the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
from
29d1339 to
7f3a110
Compare
wedamija
approved these changes
Dec 12, 2025
evanpurkhiser
deleted the
evanpurkhiser/fix-uptime-allow-strings-in-config-thresholds
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes NEW-664: Fix setting failure threshold in uptime detector creation
Adds a DRF validator for the config field, replacing the generic
JSONField. This addresses a few problemsThe DRF validator will handle coercing strings to numbers (for fields like
failureThresholdandrecoveryThreshold. Without the DRF validator the JSON Schema simply rejects numbers as strings, which is how our frontend forms send data.Previously validation errors of the config were reported under the
configkey. But since many fields on the frontend may map to the config, there's no simple way to display config errors on their associated fields. With the DRF serializer we're able to correctly report errors tied back to specific fields (see the updated tests)