Support filtering on storage state #629
Conversation
|
/lgtm |
backend/src/apiserver/model/run.go
Outdated
| @@ -73,7 +73,7 @@ var runAPIToModelFieldMap = map[string]string{ | |||
| "created_at": "CreatedAtInSec", | |||
| "description": "Description", | |||
| "scheduled_at": "ScheduledAtInSec", | |||
| "storage_state": "StorageState", | |||
| "storageState": "StorageState", | |||
There was a problem hiding this comment.
Let's use storage_state here, and fix the case in the proto name instead. It's more consistent, and in line with the style guide for proto field naming.
There was a problem hiding this comment.
We should, but I'm kind of blocked on your bazel changes since I can't re-generate the client libs with the changes. If you think we'll land that change soon, we can block this PR on it. Otherwise, let's get this merged and refactor it separately?
There was a problem hiding this comment.
Oh yeah good point. I think the Bazel change is good to go.
@IronPan do you think you can get to that change soon? If not, maybe let's submit this first and change names later.
There was a problem hiding this comment.
IMO we desperately need #631.
There was a problem hiding this comment.
Sorry I was not review the bazel PR on time because I am not on the reviewer list.
In most of the time I only review the PR that assigned to me. I'll review it now
There was a problem hiding this comment.
Correction not on the assignee list
|
/lgtm |
|
do we still need this change? |
|
This change is needed, but will be updated with the right snake case when the bazel change is merged, since I'll need to rebuild the client libs and push them in this PR too. |
|
SGTM |
yebrahim
force-pushed
the
storage-state
branch
from
1588347 to
2a5d1ce
Compare
yebrahim
force-pushed
the
storage-state
branch
2 times, most recently
from
5419622 to
b8c7c47
Compare
yebrahim
force-pushed
the
storage-state
branch
from
2ef9b22 to
7052cdb
Compare
|
Rebased to pick up other fixes, PTAL. |
|
/lgtm |
|
/test kubeflow-pipeline-e2e-test |
1 similar comment
|
/test kubeflow-pipeline-e2e-test |
yebrahim
changed the title
yebrahim
force-pushed
the
storage-state
branch
from
bcc1e5b to
7be3712
Compare
yebrahim
changed the title
/area back-end
/assign @neuromage @IronPan
This change is