chore: fix ntsc api annotations #5702
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -42,6 +42,9 @@ message GetNotebooksRequest { | |||
| } | |||
| // Response to GetNotebooksRequest. | |||
| message GetNotebooksResponse { | |||
| option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = { | |||
| json_schema: { required: [ "notebooks", "pagination" ] } | |||
There was a problem hiding this comment.
pagination is required? should it really be?
There was a problem hiding this comment.
required meaning that backed always includes it but I just assumed and didn't check. I can double check.
There was a problem hiding this comment.
we seem to always use which does set it. from what I've seen the apis that do offer a pagination state bit always include it. wdy think?
// paginate returns a paginated subset of the values and sets the pagination response.
func (a *apiServer) paginate(p **apiv1.Pagination, values interface{}, offset, limit int32) error {
There was a problem hiding this comment.
i just think it makes sense for nothing to work and return all (or some default limit), but maybe that is a pain with grpc. i've approved for now, we can revisit if it makes sense later.
There was a problem hiding this comment.
we can as easily in our handlers return nil and this field will be omitted the question is more about do we want to promise that we won't do that or not. if we already do this and we add this promise it'd be easier on the clients. I'll skip changing that part since we're not sure
|
added two missing |
Description
Test Plan
Commentary (optional)
Checklist
docs/release-notes/.See Release Note for details.
Ticket