Get experiment config from the instance #474
Conversation
|
/retest |
|
My opinion, we don't need to distinguish NASJobs from HPJobs. |
|
@YujiOshima I can make it in the same function, like you said. |
agree with @YujiOshima |
|
SGTM |
andreyvelich
changed the title
|
@hougangliu @YujiOshima @johnugeorge |
|
/retest |
| //Experiment not created in DB | ||
| err = util.CreateExperimentInDB(instance) | ||
| err = util.CreateExperimentInDB(experimentConfig) |
There was a problem hiding this comment.
As we would like to remove dependency of api manager from controller, I would recommend to keep grpc api related config out of the controller code. Let util take care of getting the config and calling the API
There was a problem hiding this comment.
I put it inside CreateExperimentInDB function in util. Is it fine?
|
@andreyvelich can you rebase |
|
@johnugeorge Done |
|
/lgtm |
| } | ||
|
|
||
| experiment.Name = instance.ObjectMeta.Name | ||
|
|
There was a problem hiding this comment.
s/instance.ObjectMeta.Name/instance.Name
|
/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 |
In this PR I have added few thing in the v1alpha2 experiment controller:
ParameterSpecinOperationinside api.proto. I think, it should be the same asParameterSpecsinExperimentSpecWe need to think, what status of Experiment config should be on the step of getting
Experiment configfrom instance.As well, do we need a JobType inside
Experiment configlike here or we can handle it with checking nasConfig ?/assign @johnugeorge @hougangliu @YujiOshima
/cc @richardsliu
This change is