fix: Fix cancellation flow in python component graph #765
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👋 Hi pankajroark! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview:
My understanding is that cancellation flow works in a daisy chain way. When downstream service can't send response to upstream service it realizes the request is stopped. This was broken for python component flow though it seems. Say the Processor is reading response from Worker with an async iterator. If Processor observes cancellation from frontend via context being
stoppedit can exist out of for loop reading Worker responses. But Worker doesn't know about it right now, it keeps continuing and sending responses. There result in a lot ofFailed to send response:messages in logs but no cancellation on Worker. By existing out of theprocess_streamloop on this error downstream gets to know of the cancellation and it then works properly.Details:
Added the break out of
process_streamon encountering an error.Where should the reviewer start?
It's a one line change.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)