-
Notifications
You must be signed in to change notification settings - Fork 67
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
py: fix order by ID #355
py: fix order by ID #355
Conversation
cc: @tarilabs we could also ensure that every orderBy param gets uppercased on the go server, wdyt? |
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.
thank you @isinyaaa I have one question to start with please, below
ee8cf17
to
2e32888
Compare
Fixes: kubeflow#353 Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
Signed-off-by: Isabella do Amaral <idoamara@redhat.com>
2e32888
to
5a62ff0
Compare
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.
thanks a lot @isinyaaa also for fixing the other issues,
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tarilabs 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 |
Fixes: #353
Description
Updates the orderBy.Id -> ID as that's what's expected on MLMD.
How Has This Been Tested?
Merge criteria:
DCO
check)If you have UI changes