-
Notifications
You must be signed in to change notification settings - Fork 137
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
Fix Issue #196: Filtered the workflow context sent to worker by server #216
Conversation
e7aff1a
to
207bfbf
Compare
593c7f0
to
514024e
Compare
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.
We still need to handle the scenarios where we have an already connected worker/stream, and 1) a new workflow gets created that targets that worker, or 2) an already executing workflow reaches a step where the worker gets new work. How is this achieved in this situation?
This handles both of your cases as follows:
|
…r by server There is a bit of change in worker as well: 1. Changed Info logs to Debug during sleep 2. Changed a bit of logic of starting the workflow action to execute
Following things are fixed: 1. Modified the filter from the server side while sending the workflow context 2. Added a gRPC stream for getting the workflow context 3. Continue the loop for worker even after completing the workflow
You're right. When we add the ability for workers to fetch-and-wait instead of fetch-and-quit, as addressed in issue #222, we will need to update this so that the server sends work to the workers that are already connected and waiting. |
Description
There are following change in this PR:
Following things are fixed in second commit of this PR
Why is this needed
This resolves the following issue.
Fixes: #196 #222 #72
How Has This Been Tested?
This has been tested by creating a sample worklfow and execute it.
Following scenarios was executed for the testing:
How are existing users impacted? What migration steps/scripts do we need?
It will enhance the user experience as logging will be lesser as compared to the current one on worker side.
There is no migration script needed.