-
Notifications
You must be signed in to change notification settings - Fork 72
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
add endpoint spec to get events from a test run #2255
Conversation
api/testEvents.yaml
Outdated
PollingInfo: | ||
type: object | ||
properties: | ||
numberSpans: |
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.
can these properties be generalized in a way that's not so tied to the timing
profile? Meaning: right now, we only support time based polling profile, but we know we'll support different strategies in the near future. Is there a way we could model this so it's more "polymorphic"?
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.
we could add a type
field and then add objects to represent each type, would that be enough?
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.
makes sense, that's the approach we took on all similar situations
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.
Done
- warning | ||
- error |
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.
Should we include a info
type?
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 think it's worth unless we want to show in the event log what are the values for each output after they were resolved. @xoscar what do you think?
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 info type is not something we'll be using in the near future so I think its fine to not have it
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.
Yeah, makes sense @mathnogueira
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.
The team comments make sense, other than that this LGTM!
Actually, can we consider adding the specs for this as well? #2183 |
added it |
This PR adds the openAPI spec to get events generated from a test run. The model is based on diagram on #2040
Checklist