Skip to content
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 APIs to wait for isvc ready and check status #610

Merged
merged 1 commit into from Jan 10, 2020
Merged

add APIs to wait for isvc ready and check status #610

merged 1 commit into from Jan 10, 2020

Conversation

jinchihe
Copy link
Contributor

@jinchihe jinchihe commented Dec 18, 2019 •
edited by jlewi

What this PR does / why we need it:

Got feedback, Need API in KFServing Python SDK Client to quick check the KFServing status and has ability to wait for the inferenceservice ready, and then user can do following things once ready. In fact there is same function in test utils. The PR is going to add below two APIs and update tests.

  • wait_for_ready: Wait for the InferenceService ready
  • is_isvc_ready: Quick way to check if the InferenceService is ready

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 #609

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

This change is Reviewable

@jinchihe
Copy link
Contributor Author

/cc @yuzisun

Could you please take a look? Thanks a lot!

@jinchihe
Copy link
Contributor Author

/retest

2 similar comments
@jinchihe
Copy link
Contributor Author

/retest

@jinchihe
Copy link
Contributor Author

/retest

yuzisun
yuzisun reviewed Dec 22, 2019
View reviewed changes
python/kfserving/README.md Outdated Show resolved Hide resolved
python/kfserving/docs/KFServingClient.md Outdated Show resolved Hide resolved
python/kfserving/docs/KFServingClient.md Show resolved Hide resolved
python/kfserving/kfserving/api/kf_serving_client.py Outdated
if self.is_isvc_ready(name, namespace=namespace):
return

if debug:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is user facing API not sure if we should print out the controller logs, I suggest printing all the inference service conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuzisun OK, I think we can print the inference service that includes status/conditions and other infomations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the debug flag and print as Exception message ? If the debug flag is not on then the RuntimeError still says See above log not there is no log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuzisun Updated, thanks!

@jinchihe
Copy link
Contributor Author

@yuzisun Addressed your comments, thanks a lot!

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 2, 2020

/retest

6 similar comments
@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 2, 2020

/retest

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 2, 2020

/retest

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 3, 2020

/retest

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 3, 2020

/retest

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 3, 2020

/retest

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 5, 2020

/retest

@yuzisun
Copy link
Member

yuzisun commented Jan 8, 2020

@jinchihe can you rebase the CI fix ?

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 8, 2020

@jinchihe can you rebase the CI fix ?

Thanks @yuzisun rebased, and also added watch function for waiting_isvc_ready since user may want to check the status during waiting :-)

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 8, 2020

/test kubeflow-kfserving-presubmit

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 8, 2020

Why timeout in unit tests

--- FAIL: TestInferenceServiceWithDefaultAndCanaryPredictor (30.46s)
   controller_test.go:554:
       Timed out after 30.000s.

@jinchihe
Copy link
Contributor Author

jinchihe commented Jan 9, 2020

/retest

@yuzisun
Copy link
Member

yuzisun commented Jan 9, 2020

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuzisun

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 4d727aa into kserve:master Jan 10, 2020
@jinchihe jinchihe deleted the add_wait_api branch January 10, 2020 00:35
ellistarn pushed a commit to ellistarn/kfserving that referenced this pull request Jul 28, 2020
@jinchihe @k8s-ci-robot
add APIs to wait for isvc ready and check status (kserve#610)
1ac1908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuzisun yuzisun yuzisun left review comments

@bhupc bhupc Awaiting requested review from bhupc

@hougangliu hougangliu Awaiting requested review from hougangliu

Assignees

@yuzisun yuzisun

Labels
approved lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need a API for user to wait for kfserving ready in SDK.
3 participants
@jinchihe @yuzisun @k8s-ci-robot