Factor the Slack notification into a local composite action - #6895
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
cf645bd to
e5cf664
Compare
qgallouedec
left a comment
There was a problem hiding this comment.
thanks, looks good; checked the local copy is identical to the shared action at the pinned SHA. The v1.10-release special case in tests_latest is handled correctly with the separate checkout path
| uses: huggingface/hf-workflows/.github/actions/post-slack@50acccfe9afb8927b9b74ed7e750f2b0d3991b62 # main | ||
| uses: ./.github/actions/post-slack | ||
| with: | ||
| slack_channel: ${{ secrets.CI_DOCKER_CHANNEL }} |
There was a problem hiding this comment.
out of the scope, but mentioning it: it would be easier if everything were in the same channel: this is the only action pointing to its own channel
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 67b5606. Configure here.
a037cd4 to
6c117ca
Compare

This PR replaces the
huggingface/hf-workflows/.github/actions/post-slackaction with a local composite action, kept as a faithful copy of the shared one.Related to #6890, which it does not resolve: the copied action still calls the same
slackapi/slack-github-actionversion, so theNode.js 20 is deprecatedannotation still appears.Motivation
Fixing #6890 requires moving
slackapi/slack-github-actionfrom v1 to a version that declares the Node 24 runtime, which is a breaking change in the way the action is called. Applying that change to the sharedhf-workflowsaction would affect around 135 call sites across roughly 15 repositories in the organization, and pinning does not protect them: Dependabot bumps SHA references that track a branch, as it has been doing for ourhuggingface/doc-builderreferences.Owning the action locally lets us make that change for TRL alone, on our own schedule, without touching any other project.
Keeping it factored, rather than inlining the Slack call into each workflow, avoids duplicating the message payload thirteen times.
Solution
Add
.github/actions/post-slack/action.ymlas a copy of the shared action at50acccfe9afb8927b9b74ed7e750f2b0d3991b62, the SHA the workflows were pinned to, and point the call sites at it withuses: ./.github/actions/post-slack.The copy is byte identical to the shared action except for the
# v1.25.0comment added to the pinnedslackapi/slack-github-actionreference, following the convention used for every other pinned action in our workflows. Inputs, message payload and dependency versions are all unchanged, so this PR is behavior neutral.A local action is loaded from the workspace, so it requires the repository to be checked out beforehand, and the checked out tree must be the one containing the action. That holds for twelve of the thirteen call sites: each of their jobs checks out the ref the workflow runs on, before posting to Slack.
It does not hold for
tests_latest.yml, which checks outv1.10-releaseon purpose, since it tests the latest release. That branch predates this action, and every future release branch will too, being cut before the change that adds it. So that job checks the action out from the branch running the workflow, into a separate path:The notification is CI infrastructure rather than part of the release under test, so taking it from the workflow ref is also the right thing semantically. The checkout is sparse, so it fetches
.github/actionsrather than a second full clone.Its
if: always()matters and is commented in the workflow: #6900 gives the Slack step of that job the same condition, and if this checkout were left on the defaultif: success()it would be skipped exactly when the tests fail, leaving the Slack step unable to resolve the action. Both merge orders are safe, since a checkout onalways()with a Slack step onsuccess()still works.Changes
.github/actions/post-slack/action.yml, copied from the shared action, with the same four inputs and the same pinnedslackapi/slack-github-actionversionPost to Slacksteps indocker-build.yml,slow-tests.yml,tests.yml,tests_latest.yml,tests_python_versions.ymlandtests_transformers_branch.ymlat the local actiontests_latest.yml, whose job checks out the release branchNote
Low Risk
CI-only refactor with behavior-neutral Slack messaging; the tests_latest checkout ordering is the main operational edge case and is explicitly guarded with
if: always().Overview
Replaces the org-wide
huggingface/hf-workflowspost-slack action with a local composite action at.github/actions/post-slack, copied from the previously pinned shared SHA so Slack payloads and inputs stay the same.Thirteen “Post to Slack” steps across
docker-build.yml,slow-tests.yml,tests.yml,tests_latest.yml,tests_python_versions.yml, andtests_transformers_branch.ymlnow useuses: ./.github/actions/post-slackinstead of the external action—keeping notifications centralized without inlining Block Kit in every workflow.tests_latest.ymlis special: the job checks out a release branch that won’t contain the new action, so it adds analways()sparse checkout of.github/actionsfrom the workflow ref intoslack-action/and calls./slack-action/.github/actions/post-slack, so failure notifications still resolve the action.Reviewed by Cursor Bugbot for commit 6c117ca. Bugbot is set up for automated code reviews on this repo. Configure here.