-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat: Ensure async workflow executions have access to shared container #8157
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
|
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.
Overall LGTM, but I'll leave the approval to Carlos and Adrien. I just had one thought, that I'd like your thoughts on
776e716
to
ef1f118
Compare
ef1f118
to
5f374a4
Compare
31ae462
to
f41dde9
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.
Sorry for the delay, just had time to review it now. It looks very good to me, just one thing, can we now omit or make optional the container to be provided when calling the run from the workflow engine public api?
Cc @shahednasser we are not obliged anymore to provide the container when using the workflow engine, which is a first step to later not have to pass the container to workflow either (still need some work such as using the workflow engine if provided to run the workflows under the hood) |
@adrien2p got it, I think once we have the second step (not having to pass the container to a workflow) we can update the docs to just be |
REF FRMW-2595