chore: run CI on forks#4292
Conversation
|
I used to have github actions enabled on my (free github) repository and I recall that Comet ci used to run every time I pushed to the repo, was very slow in getting resources, and also used to fail frequently. How do we test whether this will work? |
I could use a volunteer later this week. My repo would be as upstream and with volunteer we need to fork it and simulate PR activities |
Okay, I'll bite. Let me know when you want to try this out. |
| # 1. precondition: when running in a fork, merges the fork HEAD with | ||
| # upstream/main so downstream jobs test "fork changes applied on top | ||
| # of current upstream", giving the upstream check meaningful signal. |
There was a problem hiding this comment.
The precondition job in pr_build_entry.yml does the merge work but downstream jobs do not consume it. The merge_sha output is
declared and computed, but no build-linux, build-macos, spark-sql, etc. job reads it. Each downstream workflow runs actions/checkout@v6
on its own and gets the fork branch's HEAD as-is. So:
- The merge step verifies mergeability (the job fails if the merge conflicts), which gates downstream via needs: precondition. That
is useful.
- But the comment in pr_build_entry.yml and the contributing doc both claim downstream jobs run "fork changes applied on top of
current upstream". They do not. They run the fork branch as it exists.
| // Give the fork push event a moment to start its workflow run. | ||
| await new Promise(r => setTimeout(r, 3000)) |
There was a problem hiding this comment.
notify_test_workflow.yml waits 3 seconds before querying for the fork's run. On a slow event-delivery day that may not be enough and
the contributor lands in the action_required "Workflow run detection failed" branch even though their fork CI is fine. Spark's
equivalent retries with backoff. Worth at least a couple of retries here.
3 participants
Which issue does this PR close?
Closes #4289 .
Rationale for this change
What changes are included in this PR?
How are these changes tested?