-
Notifications
You must be signed in to change notification settings - Fork 5
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
DR-2889: Upgrade Github actions due to Node 12 Deprecation #1404
Conversation
f12ac6a
to
ecccada
Compare
…y action produced by gradle
comment out steps for now Uncomment for merge inherit secrets bump version switch job dependent update consolidate to one slack notification update space
update remaining vault actions
add distribution for java setup
Remove need for external library and call git command for tag Revert "[test] replace github-action-get-previous-tag" This reverts commit aa5c648. leave workspace_dispatch
.github/workflows/helmtagbumper.yaml
Outdated
- name: "Notify Slack" | ||
if: always() | ||
uses: broadinstitute/action-slack@v3.8.0 | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
with: | ||
status: ${{ job.status }} | ||
fields: job,repo,message,author,took | ||
author_name: "[API] [datarepo-helm-definitions] Version update for Integration namespaces" | ||
mention: fb,muscles | ||
mention_if: failure |
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.
Note to reviewers: Since we're using a reusable workflow, I have consolidated it so that we have one slack notification after all of the helm tasks run, instead of three.
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.
Update: I decided to leave this step in place, but I have a flag that can be set as to whether or not to notify slack. I have it set to default true when the workflow is triggered manually via workflow_dispatch. See the image attached - the user can set this variable on manually kicking off the workflow.
I have it set to default false when the workflow is triggered automatically through workflow_call, where the workflow should already have it's own slack notification.
add payload back cleanup use valid fields test other format add payload
This reverts commit 36638c1.
- Setting the default to true when triggered manually via workflow_dispatch - Setting the default to false when triggered via workflow_call as those workflows should have their own slack notification
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.
👍
Motivation: The goal of this ticket is to handle the warning messages that we've been getting on our github actions about node.js 12 actions being depreciated. All actions need to now run at least on node 16.
While we're making these upgrades, I'm also making changes according to a few other guiding principles [In order of preference]: (official guide here, thanks @fboulnois for pointing me to this)
When none of these options are available, I upgraded the forked library and pointed our step to the latest version (nick-fields/retry, action-slack)
Note: I attempted to switch to the slack action actually built by slack. I spent a good chunk of time just trying to get something basic working according to their documentation, but couldn't figure it out. I don't feel like that's a good sign, so I've upgraded the forked repo we've been using instead.
Testing
Test runs on each action:
Skipping testing - no unique changes to these actions and I'd rather not mess with the versions promoted to alpha/staging/force early release of a version