-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
[Serve] add blocking to serve.run() #43227
[Serve] add blocking to serve.run() #43227
Conversation
Signed-off-by: Gene Su <e870252314@gmail.com>
Signed-off-by: Gene Su <e870252314@gmail.com>
Signed-off-by: Gene Su <e870252314@gmail.com>
Also ensured the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Please also unify the CLI to call into the same flag
Signed-off-by: Gene Su <e870252314@gmail.com>
This reverts commit b8a2fd6. Signed-off-by: Gene Su <e870252314@gmail.com>
9fb3ccd
to
6aff081
Compare
Signed-off-by: Gene Su <e870252314@gmail.com>
@@ -590,8 +590,14 @@ def run( | |||
if is_config: | |||
client.deploy_apps(config, _blocking=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edoakes do you think as is is good enough or if we also want to add blocking
to client.deploy_apps
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm that's an internal API anyways
Rename the existing serve.run to serve._run and added blocking option to the new serve.run --------- Signed-off-by: Gene Su <e870252314@gmail.com>
Why are these changes needed?
Rename the existing
serve.run
toserve._run
and addedblocking
option to the newserve.run
Related issue number
Closes #43211
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.Manually tested and seeing it behaves correctly