katib
katib copied to clipboard
[WIP] add unit tests for katib sdk
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):
Fixes #2184
Checklist:
- [ ] Docs included if any changes are user facing
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: shashank-iitbhu Once this PR has been reviewed and has the lgtm label, please assign tenzen-y for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
@shashank-iitbhu Please let us know if you have time to address our comments this or next week. We are planing to cut a first Katib 0.17 RC.0 release soon.
Hello @andreyvelich I was wondering if I can pick up on this work if you are planning on including this in Katib 0.18.
Hello @andreyvelich I was wondering if I can pick up on this work if you are planning on including this in Katib 0.18.
That would be great @tariq-hasan! We can even include this in Katib 0.17 by cherry-pick this PR to the release branch. @shashank-iitbhu Are you ok to delegate this PR to @tariq-hasan so we can include that important feature to the next Katib release ?
Hello @andreyvelich I was wondering if I can pick up on this work if you are planning on including this in Katib 0.18.
That would be great @tariq-hasan! We can even include this in Katib 0.17 by cherry-pick this PR to the release branch. @shashank-iitbhu Are you ok to delegate this PR to @tariq-hasan so we can include that important feature to the next Katib release ?
@andreyvelich I'm okay with @tariq-hasan working on this PR.
Sounds good.
@andreyvelich I notice that the scope of testing in training_client_test.py is limited to create_job but that the tests were written before the train API was implemented.
I was therefore wondering if we limit the scope of katib_client_test.py to create_experiment or if we also include the tune API.
Do you plan to include tests for all the functions in training_client.py and katib_client.py in upcoming releases but limit the scope for this issue?
I plan to submit a PR within the next few days but I was also wondering if there is any strict timeline I would need to follow to get this code in the release.
Sounds good.
I was therefore wondering if we limit the scope of katib_client_test.py to create_experiment or if we also include the tune API.
I think, we can start with something, it would be nice to add tests for create_experiment
and tune
APIs, but if you want you can add tests just for one of these APIs.
Do you plan to include tests for all the functions in training_client.py and katib_client.py in upcoming releases but limit the scope for this issue?
I don't think we have enough time to include tests for all APIs before next release.
I plan to submit a PR within the next few days but I was also wondering if there is any strict timeline I would need to follow to get this code in the release.
Sure, that sound great! If we want to include that in the Katib 0.17 release, it would be nice to merge your PR before distribution testing phase, which is May 20th: https://github.com/kubeflow/community/tree/master/releases/release-1.9
Sounds good. I was therefore wondering if we limit the scope of katib_client_test.py to create_experiment or if we also include the tune API.
I think, we can start with something, it would be nice to add tests for
create_experiment
andtune
APIs, but if you want you can add tests just for one of these APIs.Do you plan to include tests for all the functions in training_client.py and katib_client.py in upcoming releases but limit the scope for this issue?
I don't think we have enough time to include tests for all APIs before next release.
I plan to submit a PR within the next few days but I was also wondering if there is any strict timeline I would need to follow to get this code in the release.
Sure, that sound great! If we want to include that in the Katib 0.17 release, it would be nice to merge your PR before distribution testing phase, which is May 20th: https://github.com/kubeflow/community/tree/master/releases/release-1.9
Sounds good. I created a PR for the create_experiment
function in the katib_client
module and plan to add another for the tune
function once the current PR is merged.
This was implemented here: https://github.com/kubeflow/katib/pull/2325