workflows/bot: improve for treewides (#459170)

ci/github-script/bot.js

@@ -576,7 +576,10 @@ module.exports = async ({ github, context, core, dry }) => {
 
   // Controls level of parallelism. Applies to both the number of concurrent requests
   // as well as the number of concurrent workers going through the list of PRs.
-  const maxConcurrent = 20
+  // We'll only boost concurrency when we're running many PRs in parallel on a schedule,
+  // but not for single PRs. This avoids things going wild, when we accidentally make
+  // too many API requests on treewides.
+  const maxConcurrent = context.eventName === 'pull_request' ? 1 : 20
 
   await withRateLimit({ github, core, maxConcurrent }, async (stats) => {
     if (context.payload.pull_request) {

ci/github-script/merge.js

@@ -128,11 +128,20 @@ async function handleMerge({
     (await getTeamMembers('nixpkgs-committers')).map(({ id }) => id),
   )
 
-  const files = await github.paginate(github.rest.pulls.listFiles, {
-    ...context.repo,
-    pull_number,
-    per_page: 100,
-  })
+  const files = (
+    await github.rest.pulls.listFiles({
+      ...context.repo,
+      pull_number,
+      per_page: 100,
+    })
+  ).data
+
+  // Early exit to prevent treewides from using up a lot of API requests (and time!) to list
+  // all the files in the pull request. For now, the merge-bot will not work when 100 or more
+  // files are touched in a PR - which should be more than fine.
+  // TODO: Find a more efficient way of downloading all the *names* of the touched files,
+  // including an early exit when the first non-by-name file is found.
+  if (files.length >= 100) return false
 
   // Only look through comments *after* the latest (force) push.
   const lastPush = events.findLastIndex(

ci/github-script/reviewers.js

@@ -13,6 +13,34 @@ async function handleReviewers({
 }) {
   const pull_number = pull_request.number
 
+  const requested_reviewers = new Set(
+    pull_request.requested_reviewers.map(({ login }) => login),
+  )
+  log(
+    'reviewers - requested_reviewers',
+    Array.from(requested_reviewers).join(', '),
+  )
+
+  const existing_reviewers = new Set(
+    reviews.map(({ user }) => user?.login).filter(Boolean),
+  )
+  log(
+    'reviewers - existing_reviewers',
+    Array.from(existing_reviewers).join(', '),
+  )
+
+  // Early sanity check, before we start making any API requests. The list of maintainers
+  // does not have duplicates so the only user to filter out from this list would be the
+  // PR author. Therefore, we check for a limit of 15+1, where 15 is the limit we check
+  // further down again.
+  // This is to protect against huge treewides consuming all our API requests for no
+  // reason.
+  if (maintainers.length > 16) {
+    core.warning('Too many potential reviewers, skipping review requests.')
+    // Return a boolean on whether the "needs: reviewers" label should be set.
+    return existing_reviewers.size === 0 && requested_reviewers.size === 0
+  }
+
   const users = new Set([
     ...(await Promise.all(
       maintainers.map(async (id) => (await getUser(id)).login),
@@ -47,6 +75,9 @@ async function handleReviewers({
   const reviewers = (
     await Promise.all(
       Array.from(new_reviewers, async (username) => {
+        // TODO: Restructure this file to only do the collaborator check for those users
+        // who were not already part of a team. Being a member of a team makes them
+        // collaborators by definition.
         try {
           await github.rest.repos.checkCollaborator({
             ...context.repo,
@@ -64,24 +95,8 @@ async function handleReviewers({
   ).filter(Boolean)
   log('reviewers - reviewers', reviewers.join(', '))
 
-  const requested_reviewers = new Set(
-    pull_request.requested_reviewers.map(({ login }) => login),
-  )
-  log(
-    'reviewers - requested_reviewers',
-    Array.from(requested_reviewers).join(', '),
-  )
-
-  const existing_reviewers = new Set(
-    reviews.map(({ user }) => user?.login).filter(Boolean),
-  )
-  log(
-    'reviewers - existing_reviewers',
-    Array.from(existing_reviewers).join(', '),
-  )
-
   if (reviewers.length > 15) {
-    log(
+    core.warning(
       `Too many reviewers (${reviewers.join(', ')}), skipping review requests.`,
     )
     // Return a boolean on whether the "needs: reviewers" label should be set.