Register Trial in DB #530
Register Trial in DB #530
Conversation
k8s-ci-robot
requested review from
gaocegege,
hougangliu,
johnugeorge and
richardsliu
pkg/api/v1alpha2/api.proto
Outdated
| @@ -378,8 +378,8 @@ message TrialStatus { | |||
|
|
|||
| message Trial { | |||
| string name = 1; | |||
| TrialSpec spec = 2; | |||
| TrialStatus status = 3; | |||
| TrialSpec trial_spec = 2; | |||
There was a problem hiding this comment.
In fact, I like spec/status rather than trial_spec/trial_status. Maybe we should refactor experiment_spec/experiment_status instead of here
There was a problem hiding this comment.
I can rename experiment_spec to spec, if we want to.
What do you think? @johnugeorge @gaocegege @richardsliu
There was a problem hiding this comment.
Prefer spec/status but it may require lots of work to update the code base.
There was a problem hiding this comment.
I will update it in this PR, no problem.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer kcc.Conn.Close() |
There was a problem hiding this comment.
It is a little hack here. Could we define a func in katibManagerClientAndConn to close the conn?
| @@ -18,11 +18,20 @@ package util | |||
| import ( | |||
| //v1 "k8s.io/api/core/v1" | |||
There was a problem hiding this comment.
Please remove the never used package here
|
@gaocegege @hougangliu Done. |
|
/test kubeflow-katib-presubmit |
|
@gaocegege @hougangliu @johnugeorge I fixed all tests, I think, we can merge this PR. |
| trial.Spec.RunSpec = instance.Spec.RunSpec | ||
|
|
||
| trial.Spec.MetricsCollectorSpec = instance.Spec.MetricsCollectorSpec | ||
|
|
|
@hougangliu I have added trial status and change status names in |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hougangliu 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 |
I added part to register Trial in DB.
As well, I rename Status and Spec to TrialStatus and TrailSpec here: https://github.com/andreyvelich/katib/blob/issue-510-register-trial-db/pkg/api/v1alpha2/api.proto#L381,
like we have with Experiment.
/cc @hougangliu @johnugeorge @richardsliu @gaocegege
Part of #510.
This change is