-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
meta(crons): Update API help text #58048
Conversation
src/sentry/monitors/validators.py
Outdated
class MonitorValidator(CamelSnakeSerializer, PreventNumericSlugMixin): | ||
project = ProjectField(scope="project:read") | ||
name = serializers.CharField(max_length=128) | ||
name = serializers.CharField( | ||
max_length=128, help_text="Name of the monitor. Used for notifications." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Let's slap a magic trailing comma at the end so it's consistent with the other fields?
src/sentry/monitors/validators.py
Outdated
) | ||
status = serializers.ChoiceField( | ||
choices=list(zip(MONITOR_STATUSES.keys(), MONITOR_STATUSES.keys())), | ||
default="active", | ||
help_text="One of [active, disabled]. Disabled monitors do not generate events or notifications.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the API spec already shows the options, let's leave it off of the help text since we would have to keep it in sync otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's possible to create documentation for the enums themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it in the docs now https://docs.sentry.io/api/crons/create-a-monitor/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right was looking in the wrong place
src/sentry/monitors/validators.py
Outdated
help_text="Duration of the job run, in milliseconds.", | ||
) | ||
environment = serializers.CharField( | ||
required=False, allow_null=True, help_text="Name of the environment." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
required=False, allow_null=True, help_text="Name of the environment." | |
required=False, | |
allow_null=True, | |
help_text="Name of the environment.", |
@@ -288,7 +292,7 @@ class ContextsValidator(serializers.Serializer): | |||
trace = TraceContextValidator(required=False) | |||
|
|||
|
|||
@extend_schema_serializer(exclude_fields=["duration", "environment", "monitor_config", "contexts"]) | |||
@extend_schema_serializer(exclude_fields=["monitor_config", "contexts"]) | |||
class MonitorCheckInValidator(serializers.Serializer): | |||
status = serializers.ChoiceField( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No help text on status?
Codecov Report
@@ Coverage Diff @@
## master #58048 +/- ##
==========================================
- Coverage 79.03% 79.02% -0.01%
==========================================
Files 5130 5130
Lines 223044 223045 +1
Branches 37560 37562 +2
==========================================
- Hits 176276 176270 -6
- Misses 41135 41141 +6
- Partials 5633 5634 +1
|
Updates API help text for Crons validators
Follow up from #57885
Closes #58051