Sanity check filtering/sorting options in list requests. #625
Conversation
If sort by or filtering criteria is specified in conjunction with a next page token, ensure they match up, otherwise return an error. Also, change the errors to be InvalidInputErrors instead of standard Go string errors to be consistent with the rest of apiserver.
|
/assign @IronPan |
|
|
||
| opts, err = list.NewOptions(&model.Experiment{}, int(request.PageSize), request.SortBy, f) | ||
| } | ||
| opts, err := validatedListOptions(&model.Experiment{}, request.PageToken, int(request.PageSize), request.SortBy, request.Filter) |
There was a problem hiding this comment.
nit consider making &model.Experiment{} a constant so it doesn't instantiate a new object for each API call.
There was a problem hiding this comment.
Hmm, I think any performance issue here is negligible, and the benefits of having this right here for readability (vs another indirection for the reader) outweighs it anyway. It's really cheap to create it and the performance cost of this call is elsewhere, so I'd rather leave it as is. What do you think?
|
/approve |
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan, vicaire The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan, vicaire The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Fixed merge conflicts. PTAL. |
|
/lgtm |
…the cluster (kubeflow#625) * Use Anthos Config Management to continually sync the label config to the cluster * Define a kustomize package to generate the config map from the labels * ACM requires the directory structure follow a certain format so we created the directory label_sync/acm_repo * A simple shell script runs kustomize to dump the manifests to the directory. Related to kubeflow#624 * Address comments.
…kubeflow#625) * fix nested recursion with condition custom task and parameter mapping * fix alpine commands and add licenses
If sort by or filtering criteria is specified in conjunction with a next
page token, ensure they match up, otherwise return an error.
Also, change the errors to be InvalidInputErrors instead of standard Go
string errors to be consistent with the rest of apiserver.
This change is