-
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
Added a new rpc call workflow.GetWorkflowContextList to be used by boots #237
Conversation
Signed-off-by: Gaurav Gahlot <[email protected]>
Signed-off-by: Gaurav Gahlot <[email protected]>
Signed-off-by: Gaurav Gahlot <[email protected]>
Let me first test it with boots then merge it. |
changes done in accordance with #238 |
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.
Can we write a unit test for this?! I would like to have a table test that insert a set of fixtures and checks what we expect. This is a crucial part of the application @gauravgahlot can you add that to this PR?
Tested the workflow execution using this PR. changes are working correctly. |
@gianarb Will it be fine if we merge this PR and @gauravgahlot can add table test for it in a separate PR. To fix issues related to #222 , this is a kind of priority. |
Yes I agree but let's get it done as soon as it is merged please |
Thanks @gianarb , |
Description
As the worker starts, Boots checks if there is a workflow defined for that worker. If yes, it will continue to PXE and it will not otherwise. (more context here).
The PR introduces a new RPC that can be used by Boots to get the workflow contexts without having to maintain a gRPC stream.
Why is this needed
Recently there have been changes in Tink, to allow a worker to maintain a gRPC stream with the server to get the workflows. The same RPC was being by boots.
How Has This Been Tested?
Still a WIP.
How are existing users impacted? What migration steps/scripts do we need?
No direct impact on users.
Checklist:
I have: