kumarUjjawal commented on code in PR #26683:
URL: https://github.com/apache/datafusion/pull/26683#discussion_r4500466618


##########
.github/workflows/fork_ci_status.yml:
##########
@@ -199,7 +213,22 @@ jobs:
             }
 
             async function notify() {
-              const pr = context.payload.pull_request;
+              // The same branch can also target a release branch, which gets 
no
+              // mirrored check, so look only at the pull request to `main`.
+              const pulls = await 
require('./ci/scripts/pull_requests_of_run.js')({
+                github,
+                context,
+                base: 'main',
+              });
+              // A run for an older commit finds nothing, and the scheduled
+              // update creates the check of the new head.
+              const pr = pulls.find(

Review Comment:
   When a second pull request from the same fork branch targets a release 
branch, its signal run creates a second `Rust (fork)` check on the pull request 
to `main`



##########
.github/workflows/pull_request_signal.yml:
##########
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: Apache-2.0
+# SPDX-FileCopyrightText: See the NOTICE file distributed with this work for 
additional information regarding copyright ownership
+# SPDX-FileContributor: Licensed to the Apache Software Foundation (ASF) under 
one or more contributor license agreements
+
+# Signal for the workflows that need a write token on a pull request.
+#
+# ASF infra policy disables `pull_request_target`
+# (https://infra.apache.org/github-actions-policy.html), and GitHub is adding a
+# default policy that blocks it. The `workflow_run` workflows below run from 
the
+# default branch with a write token and never check out or run pull request
+# code:
+#   - labeler.yml
+#   - fork_ci_status.yml
+#
+# They trigger on the `requested` event of this workflow. GitHub sends it at
+# once for every run, even one that waits for the approval of a first-time
+# contributor. This is not documented, but it is seen in
+# apache/spark-kubernetes-operator, which uses the same pattern.
+#
+# A pull request runs its own copy of this file, so it can change or delete it.
+# That only skips the labels and the immediate `Rust (fork)` check for that 
pull
+# request. GitHub also skips `pull_request` workflows while a pull request has
+# merge conflicts, so such a pull request gets its labels after a push that
+# resolves them. The scheduled update in `fork_ci_status.yml` creates the check
+# in both cases, and the merge queue stays the gate.
+name: Pull request signal
+
+on:
+  pull_request:

Review Comment:
   The signal uses `pull_request`, and GitHub skips that event when the head 
commit message contains `[skip ci]` (or `[ci skip]`, `[no ci]` and similar). 
The old pull_request_target triggers ran anyway, and the header comment does 
not list this gap.**



##########
ci/scripts/release_version_labeler.js:
##########
@@ -27,10 +48,44 @@ module.exports = async ({ github, context }) => {
         }
     }
 
-    await github.rest.issues.addLabels({
-        owner: context.repo.owner,
-        repo: context.repo.repo,
-        issue_number: context.payload.pull_request.number,
-        labels: [target_version]
-    });
+    // A push can include several merged pull requests. Look the commits up a
+    // batch at a time, so the retries of commits without one overlap. A failed
+    // lookup or label must not cost the other pull requests their label, so
+    // collect the errors and throw the first one after labeling.
+    const pushed = (context.payload.commits || []).map((commit) => commit.id);
+    const shas = pushed.length ? pushed : [context.sha];
+    const numbers = new Set();
+    const errors = [];
+    for (let i = 0; i < shas.length; i += BATCH_SIZE) {
+        const batch = shas.slice(i, i + BATCH_SIZE);
+        const settled = await Promise.allSettled(
+            batch.map((sha) => mergedPulls({ github, context, sha }))
+        );
+        for (const result of settled) {
+            if (result.status === 'fulfilled') {
+                result.value.forEach((pull) => numbers.add(pull.number));
+            } else {
+                errors.push(result.reason);
+            }
+        }
+    }
+    if (numbers.size === 0 && errors.length === 0) {
+        console.log(`No merged pull request found for ${shas.join(', ')}`);
+    }
+    for (const number of numbers) {
+        try {
+            await github.rest.issues.addLabels({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: number,
+                labels: [target_version]
+            });
+        } catch (error) {
+            errors.push(error);
+        }
+    }
+    if (errors.length > 0) {

Review Comment:
   Only the first error is thrown. The other lookup and label errors are 
counted but their messages are never logged.



-- 
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]

Reply via email to