-
Notifications
You must be signed in to change notification settings - Fork 252
fix(ci): handle sentinel input in request_queue gracefully #589
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
* Added a new exception class _StopLoopError to handle sentinel values in request processing loops. * Updated collate_requests and loop classes to raise _StopLoopError when a sentinel value is encountered, allowing for clean exit from loops. * Refactored request handling in SingleLoop and StreamingLoop to improve clarity and maintainability.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #589 +/- ##
===================================
- Coverage 85% 85% -0%
===================================
Files 39 39
Lines 3017 3049 +32
===================================
+ Hits 2564 2588 +24
- Misses 453 461 +8 🚀 New features to boost your workflow:
|
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 introduces graceful handling of sentinel values in request queue processing to reduce failure messages in CI. It implements a new exception mechanism for cleanly exiting processing loops when shutdown signals are received.
- Added
_StopLoopErrorexception class for handling sentinel value shutdown signals - Updated all request processing loops to check for sentinel values and exit gracefully
- Refactored request queue handling across streaming, single, and batched loops for consistency
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/litserve/loops/base.py |
Added _StopLoopError exception and sentinel handling in collate_requests function |
src/litserve/loops/simple_loops.py |
Updated single, async, and batched loops to handle sentinel values gracefully |
src/litserve/loops/streaming_loops.py |
Added sentinel value detection and graceful exit in streaming loop |
tests/unit/test_batch.py |
Added test coverage for sentinel value handling in collate_requests |
Comments suppressed due to low confidence (1)
src/litserve/loops/simple_loops.py:301
- The method call has been changed from
collate_requeststoself.get_batch_requestsbut the import forcollate_requestswas removed. This suggestsget_batch_requestsis being used instead, but this change is inconsistent with the other loops which still usecollate_requestsdirectly.
batches, timed_out_uids = self.get_batch_requests(
* Introduced FakeTransport class to simulate message transport in tests. * Updated test_batched_loop to utilize FakeTransport for response handling. * Improved assertions to verify the correctness of responses from the batched loop.
What does this PR do?
Devs won't see a lot of error msgs in the CI.
We exit the background process loop by sending
(None, None, None)to the queue. We don't handle this gracefully and even though the loop is exited, we end up printing a lot of errors in the CI. This PR, will gracefully handle the termination for clearer CI.Follow up
Changes
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in GitHub issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃