nchammas commented on PR #57015: URL: https://github.com/apache/spark/pull/57015#issuecomment-4964974269
OK, I did a deep dive on the history of the "checkout-and-sync" pattern to better understand why it was introduced. > We do this so that we know PRs are always tested against the latest `master`. That way tests don't pass on the PR but later fail post-merge because `master` changed. This analysis was a reasonable deduction from looking at the code, but it's incomplete. @HyukjinKwon actually introduced this pattern in #32092 back in 2021 primarily to distribute the CI cost to forks vs. having the cost all fall on `apache/spark` and count against ASF Infra limits. The key is the switch from `on: pull_request` to `on: push`. The former counts CI cost against the target repo (i.e. `apache/spark`). The latter counts CI cost against the fork. So we have multiple CI concerns that are in tension with one another: 1. Distribute CI cost to forks. This is why we use `on: push` instead of `on: pull_request`. 2. Test against the latest `master` to prevent post-merge CI failures. This is part of why we merge against upstream. `on: pull_request` used to do that for us, but we needed to do it ourselves after switching to `on: push`. 3. Test a fixed commit across all CI jobs that are running concurrently. This becomes a problem because of the merge to upstream since `master` is sometimes a rapidly moving target. 4. Detect what was changed so we can skip unnecessary build/test steps and save CI cost. This also motivates the merge against upstream. 5. Annotate PRs inline so reacting to CI failures is easier. This works only if the runner checkout is the actual PR commit SHA, not a synthetic merge, so this is blocked by the merge to upstream. I'm going to see if we can address all 5 concerns by fetching upstream _without merging_. This will allow us to run diffs, compute PR staleness, etc. but also get a stable commit and working inline annotations. And most importantly, CI cost will remain pushed out to forks and not count against this repo. -- 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]
