-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add micromamba deinit in post.js
#89
Conversation
c7d0f1a
to
064a0cf
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.
GitHub broke my review
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.
getInput
is NOT safe in a post action because if the calling workflow uses templated inputs, they will generally evaluate to something different in the post action:
- using: foo
with:
# Using 'x' in post is not safe
x: ${{ some_templated_var }}
"Official workaround": https://github.com/actions/cache/blob/a7c34adf76222e77931dedbf4a45b2e4648ced19/src/restore.ts#L24-L25
Feel free to double check if this has changed in the past months
Note that NONE of the |
Done. |
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.
Looks good!
Let's add a test (would've caught the bug below!)
Unrelated to this PR, I just saw that we have our own implementation of |
Co-authored-by: Jonas Haag <jonas@lophus.org>
Checking the state after the post action is not easy. You need to execute another action before p-w-m that executes your commands in their post action. Luckily, there is an action for that: webiny/action-post-run. Unfortunately, this action does not pipe the errors out and thus won't fail if an error occurs. This is fixed in the fork lisanna-dettwyler/action-post-run. |
Co-authored-by: Jonas Haag <jonas@lophus.org>
Whoops, early merge, let’s hope CI is good |
Adds the micromamba deinitialization in the post-action.
Closes #79.
Tested here.