sunchao commented on code in PR #7311:
URL: https://github.com/apache/datafusion-comet/pull/7311#discussion_r4419233936
##########
.github/workflows/ci.yml:
##########
@@ -73,9 +80,13 @@ name: Comet CI
# each other. Every other run is keyed on its event name: opened, synchronize
# and reopened all map to `pull_request`, so a new push still supersedes its
# predecessor, while a scheduled run at the tip of main never cancels that
-# commit's push run.
+# commit's push run. A pull request is keyed on its number, and not on its
+# branch name: nearly every pull request comes from a fork, and two forks'
+# branches can share a name, which would have one cancel the other's run. A
push
+# to a fork (ci_fork.yml) is keyed on its branch instead of its commit, so the
+# next push to the branch cancels the run in flight.
concurrency:
- group: ${{ github.repository }}-${{ github.head_ref || github.sha }}-${{
github.workflow }}-${{ github.event.action == 'labeled' &&
github.event.label.name || github.event_name }}
+ group: ${{ github.repository }}-${{ github.event.pull_request.number ||
(github.event_name == 'push' && github.repository != 'apache/datafusion-comet'
&& github.ref) || github.sha }}-${{ github.workflow }}-${{ github.event.action
== 'labeled' && github.event.label.name || github.event_name }}
Review Comment:
The exact-branch comparison in supersede fixes the different-commit case.
One case remains on dd828c0f. Suppose a contributor has a PR building commit X
and creates a backup branch at X. Both fork pushes get the same concurrency
group in ci.yml, and cancel-in-progress is true. The backup run can cancel the
PR run. The bridge looks up the PR’s exact branch, so it will still report the
cancelled run even if the backup branch passes.
I confirmed the key collision from the workflow expression. The new test at
lines 313–315 actually asserts that the two keys are equal. Using github.run_id
for fork pushes would leave same-branch cancellation to supersede. A regression
test could give the two pushes different run IDs, the same SHA, and assert
different groups. [Concurrency
group](https://github.com/apache/datafusion-comet/blob/dd828c0f0c576baf4be9e9c8978071c496266b39/.github/workflows/ci.yml#L85-L92)
·
[Test](https://github.com/apache/datafusion-comet/blob/dd828c0f0c576baf4be9e9c8978071c496266b39/dev/ci/test-fork-ci-status.cjs#L308-L316)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
