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.

Sign up for GitHub

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

Jump to bottom

Add more APIs for TFJob done #1116

Merged

k8s-ci-robot merged 1 commit into kubeflow:master from jinchihe:add_more_apis

Dec 18, 2019

Member

jinchihe commented Dec 16, 2019 •

edited

Loading

From feedback of the kubeflow-tfjob SDK, add more APIs (some of them are ehanced from py/kubeflow/tf_operator/tf_job_client.py):

wait_for_job: Wait for the specified job to finish
wait_for_condition: Waits until any of the specified conditions occur
get_job_status: Quick way to get the TFJob status
if_job_running: Quick way to check if the TFJob running
if_job_succeeded: Quick way to check if the TFJob Succeeded

This change is

k8s-ci-robot added the do-not-merge/work-in-progress label

k8s-ci-robot requested review from ChanYiLin and jimexist

December 16, 2019 09:57

k8s-ci-robot added the size/L label

coveralls commented Dec 16, 2019 •

edited

Loading

Coverage remained the same at 96.512% when pulling 4898d49 on jinchihe:add_more_apis into 0267710 on kubeflow:master.

TravisBuddy commented Dec 16, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: f7ebde60-1fea-11ea-ba47-7f442aed9c1e

k8s-ci-robot added size/XL and removed size/L labels

TravisBuddy commented Dec 17, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: e8a337b0-2098-11ea-830b-038034041c48

TravisBuddy commented Dec 17, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: 67b92220-209a-11ea-830b-038034041c48

TravisBuddy commented Dec 17, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: 25ada500-209e-11ea-830b-038034041c48

TravisBuddy commented Dec 17, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: acedae30-20a2-11ea-830b-038034041c48

TravisBuddy commented Dec 17, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: b4826360-20a8-11ea-830b-038034041c48

Member Author

jinchihe commented Dec 17, 2019

/retest


          add API to wait for TFJob done

4898d49

TravisBuddy commented Dec 18, 2019

Hey @jinchihe,
Your changes look good to me!

TravisBuddy Request Identifier: fc17bd30-213c-11ea-a044-67124c782b74

Member Author

jinchihe commented Dec 18, 2019

/retest

jinchihe changed the title ~~WIP: add API to wait for TFJob done~~ Add more APIs for TFJob done

k8s-ci-robot removed the do-not-merge/work-in-progress label

Member Author

jinchihe commented Dec 18, 2019

/cc @gaocegege @johnugeorge
Could you please take a look? Thanks a lot!

k8s-ci-robot requested review from gaocegege and johnugeorge

December 18, 2019 03:06

gaocegege reviewed

View reviewed changes

sdk/python/kubeflow/tfjob/api/tf_job_client.py

+                  Args:
+                    name: The TFJob name.
+                    namespace: defaults to current or default namespace.

Member

gaocegege Dec 18, 2019

current or default?

Member Author

jinchihe Dec 18, 2019

Thanks Ce! Yes, will get the current namespace if running in the cluster from /var/run/secrets/kubernetes.io/serviceaccount/namespace, otherwise will get the default namespace. please see below for the logic, thanks.

https://github.com/kubeflow/tf-operator/blob/0267710c69f2f92e4a65db597f7ee0d2e734462f/sdk/python/kubeflow/tfjob/utils/utils.py#L26

Member

gaocegege Dec 18, 2019

SGTM.

Member

gaocegege commented Dec 18, 2019

@jinchihe
Thanks for your contribution! 🎉 👍

Generally LGTM

Member

gaocegege commented Dec 18, 2019

/lgtm

k8s-ci-robot assigned gaocegege

k8s-ci-robot added the lgtm label

Member

johnugeorge commented Dec 18, 2019

/approve

k8s-ci-robot commented Dec 18, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

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:

~~OWNERS~~ [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the approved label

k8s-ci-robot merged commit f448d5f into kubeflow:master

Member Author

jinchihe commented Dec 18, 2019

Thanks @gaocegege and @johnugeorge

jinchihe deleted the add_more_apis branch

December 18, 2019 05:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm size/XL