-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(uptime): new snuba-uptime-results topic #359
Conversation
{ | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"title": "snuba_uptime_result", | ||
"definitions": { |
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.
these are copied from
"definitions": { |
i tried to link directly to them with a "$ref", but it doesn't seem like our framework supports that.
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.
@lynnagara do you know if it's possible to cross reference schemas somehow in this repo?
Would be nice to not need to duplicate these
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.
Oh cool, I totally missed it. Thanks!
- version: 1 | ||
compatibility_mode: backward | ||
type: json | ||
resource: snuba-uptime-results.v1.schema.json |
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 schemas are identical, so can we just point to uptime-results.yaml
here and avoid the duplication?
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.
they aren't identical, as the new schema contains organization_id, project_id, retention_days, and region_slug which the old one does not contain.
we could alternatively update the old schema to contain these values i suppose.
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.
👎 to having the other schema include these values. They won't be used since the uptime checker isn't aware of org / project_id (and can't be since the requests are de-duplicated right now)
@@ -88,6 +89,7 @@ | |||
/schemas/monitors-clock-tick.v1.schema.json @getsentry/crons | |||
/schemas/monitors-clock-tasks.v1.schema.json @getsentry/crons | |||
/schemas/uptime-results.v1.schema.json @getsentry/crons | |||
/schemas/uptime-results.v1.schema.json @getsentry/crons |
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.
Duplicate?
I handled avoiding the duplication here, after some changes to our schema library: #361 |
creates a new kafka topic
snuba-uptime-results
which will be used for sending uptime results to snuba.needed for https://github.com/getsentry/team-uptime/issues/23