-
Notifications
You must be signed in to change notification settings - Fork 6.6k
[Docs] Fix async code in serving notebook #53864
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
Conversation
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
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.
Pull Request Overview
This PR updates async execution in Jupyter notebooks by adding a wait flag for notebook conversion and converting the serving test into proper async functions with error handling.
- Added
--wait
flag to ensure the notebook conversion completes before execution. - Changed
test_serve
to anasync def
, addedresponse.raise_for_status()
, and updated invocation to useawait
. - Adjusted
fetch_all_concurrently
usage to return predictions and print a completion message.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
doc/source/ray-overview/examples/e2e-xgboost/notebooks/01-Distributed_Training.ipynb | Added --wait flag to the Ray CLI invocation for notebook scripts |
doc/source/ray-overview/examples/e2e-timeseries/e2e_timeseries/03-Serving.ipynb | Updated serving test to async, improved HTTP error handling, and updated invocation |
Comments suppressed due to low confidence (2)
doc/source/ray-overview/examples/e2e-timeseries/e2e_timeseries/03-Serving.ipynb:302
- [nitpick] Since return_exceptions=True prevents errors from propagating, consider setting it to False or explicitly handling exceptions in the returned responses list to avoid silent failures.
responses = await asyncio.gather(*tasks, return_exceptions=True)
doc/source/ray-overview/examples/e2e-xgboost/notebooks/01-Distributed_Training.ipynb:890
- [nitpick] Consider adding a comment explaining why the
--wait
flag is necessary for notebook conversions to complete before execution, improving clarity for readers.
--wait \\
doc/source/ray-overview/examples/e2e-timeseries/e2e_timeseries/03-Serving.ipynb
Outdated
Show resolved
Hide resolved
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.
stamp
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
166048d
to
ce6d594
Compare
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Ricardo Decal <rdecal@anyscale.com>
Signed-off-by: Scott Lee <scott.lee@rebellions.ai>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: Goutam V <goutam@anyscale.com>
Why are these changes needed?
Async code works in scripts/CI but not on jupyter notebooks on Workspaces.
Related issue number
N/A
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.