v2 (#2)

* Refactor the checkgroup loop. Now runs straight away. Also manages the timeout itself * Add comment that suggests pinging maintainers when it times out.
Lightning-AI · Nov 10, 2022 · 951a2fe · 951a2fe
1 parent babecd7
commit 951a2fe
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 90 deletions.
diff --git a/.eslintrc.yml b/.eslintrc.yml
@@ -0,0 +1,14 @@
+env:
+  browser: true
+  es2021: true
+extends:
+  - eslint:recommended
+  - plugin:@typescript-eslint/recommended
+overrides: []
+parser: '@typescript-eslint/parser'
+parserOptions:
+  ecmaVersion: latest
+  sourceType: module
+plugins:
+  - '@typescript-eslint'
+rules: {}
diff --git a/action.yml b/action.yml
@@ -4,9 +4,16 @@ inputs:
   job:
     required: true
     default: auto-cc
+  maintainers:
+    required: true
+  owner:
+    required: true
+  timeout:
+    required: false
+    default: 60  # minutes
   interval:
     required: false
-    default: 120
+    default: 120  # seconds
 runs:
   using: 'node16'
   main: 'dist/action.js'
diff --git a/dist/auto-cc/auto-cc-bot.js b/dist/auto-cc/auto-cc-bot.js
@@ -63,10 +63,8 @@ function myBot(app) {
                         labels = context.payload[name]['labels'].map(function (e) { return e['name']; });
                         context.log({ labels: labels });
                         cc = new Set();
-                        // eslint-disable-next-line github/array-foreach
                         labels.forEach(function (l) {
                             if (l in subscriptions) {
-                                // eslint-disable-next-line github/array-foreach
                                 subscriptions[l].forEach(function (u) { return cc.add(u); });
                             }
                         });
@@ -89,7 +87,6 @@ function myBot(app) {
                         }
                         if (!(prevCC.size !== cc.size)) return [3 /*break*/, 6];
                         newCCString_1 = 'cc';
-                        // eslint-disable-next-line github/array-foreach
                         cc.forEach(function (u) {
                             newCCString_1 += " @".concat(u);
                         });

diff --git a/dist/auto-cc/utils.js b/dist/auto-cc/utils.js
@@ -168,7 +168,6 @@ function parseSubscriptions(rawSubsText) {
     if (subsRows == null) {
         return subscriptions;
     }
-    // eslint-disable-next-line github/array-foreach
     subsRows.forEach(function (row) {
         var labelMatch = row.match(/^\* +([^@]+)/);
         if (labelMatch) {

diff --git a/dist/check-group/core/index.js b/dist/check-group/core/index.js
@@ -74,14 +74,17 @@ var utils_3 = require("../utils");
  */
 var CheckGroup = /** @class */ (function () {
     function CheckGroup(pullRequestNumber, config, context, sha) {
+        this.intervalTimer = setTimeout(function () { return ''; }, 0);
+        this.timeoutTimer = setTimeout(function () { return ''; }, 0);
         this.pullRequestNumber = pullRequestNumber;
         this.config = config;
         this.context = context;
         this.sha = sha;
     }
     CheckGroup.prototype.run = function () {
         return __awaiter(this, void 0, void 0, function () {
-            var filenames, subprojs, expectedChecks, interval, tries, conclusion, loop;
+            var filenames, subprojs, expectedChecks, interval, timeout;
+            var _this = this;
             return __generator(this, function (_a) {
                 switch (_a.label) {
                     case 0: return [4 /*yield*/, this.files()];
@@ -96,46 +99,62 @@ var CheckGroup = /** @class */ (function () {
                         }
                         interval = parseInt(core.getInput('interval'));
                         core.info("Check interval: ".concat(interval));
-                        tries = 0;
-                        conclusion = undefined;
-                        loop = setInterval(function (that) {
-                            return __awaiter(this, void 0, void 0, function () {
-                                var postedChecks, summary, details, error_1;
-                                return __generator(this, function (_a) {
-                                    switch (_a.label) {
-                                        case 0:
-                                            _a.trys.push([0, 2, , 3]);
-                                            tries += 1;
-                                            core.startGroup("Check ".concat(tries));
-                                            return [4 /*yield*/, getPostedChecks(that.context, that.sha)];
-                                        case 1:
-                                            postedChecks = _a.sent();
-                                            core.debug("postedChecks: ".concat(JSON.stringify(postedChecks)));
-                                            conclusion = (0, utils_3.satisfyExpectedChecks)(subprojs, postedChecks);
-                                            summary = (0, utils_1.generateProgressSummary)(subprojs, postedChecks);
-                                            details = (0, utils_1.generateProgressDetails)(subprojs, postedChecks);
-                                            core.info("".concat(that.config.customServiceName, " conclusion: '").concat(conclusion, "':\n").concat(summary, "\n").concat(details));
-                                            core.endGroup();
-                                            if (conclusion === "all_passing") {
-                                                core.info("Required checks were successful!");
-                                                clearInterval(loop);
-                                            }
-                                            return [3 /*break*/, 3];
-                                        case 2:
-                                            error_1 = _a.sent();
-                                            core.setFailed(error_1);
-                                            clearInterval(loop);
-                                            return [3 /*break*/, 3];
-                                        case 3: return [2 /*return*/];
-                                    }
-                                });
-                            });
-                        }, interval * 1000, this);
+                        this.runCheck(subprojs, 1, interval * 1000);
+                        timeout = parseInt(core.getInput('timeout'));
+                        core.info("Timeout: ".concat(timeout));
+                        // set a timeout that will stop the job to avoid polling the GitHub API infinitely
+                        this.timeoutTimer = setTimeout(function () {
+                            clearTimeout(_this.intervalTimer);
+                            core.setFailed("The timeout of ".concat(timeout, " minutes has triggered but not all required jobs were passing.")
+                                + " This job will need to be re-run to merge your PR."
+                                + " If you do not have write access to the repository you can ask ".concat(core.getInput('maintainers'), " to re-run it for you.")
+                                + " If you have any other questions, you can reach out to ".concat(core.getInput('owner'), " for help."));
+                        }, timeout * 60 * 1000);
                         return [2 /*return*/];
                 }
             });
         });
     };
+    CheckGroup.prototype.runCheck = function (subprojs, tries, interval) {
+        return __awaiter(this, void 0, void 0, function () {
+            var postedChecks, conclusion, summary, details, error_1;
+            var _this = this;
+            return __generator(this, function (_a) {
+                switch (_a.label) {
+                    case 0:
+                        _a.trys.push([0, 2, , 3]);
+                        // print in a group to reduce verbosity
+                        core.startGroup("Check ".concat(tries));
+                        return [4 /*yield*/, getPostedChecks(this.context, this.sha)];
+                    case 1:
+                        postedChecks = _a.sent();
+                        core.debug("postedChecks: ".concat(JSON.stringify(postedChecks)));
+                        conclusion = (0, utils_3.satisfyExpectedChecks)(subprojs, postedChecks);
+                        summary = (0, utils_1.generateProgressSummary)(subprojs, postedChecks);
+                        details = (0, utils_1.generateProgressDetails)(subprojs, postedChecks);
+                        core.info("".concat(this.config.customServiceName, " conclusion: '").concat(conclusion, "':\n").concat(summary, "\n").concat(details));
+                        core.endGroup();
+                        if (conclusion === "all_passing") {
+                            core.info("All required checks were successful!");
+                            clearTimeout(this.intervalTimer);
+                            clearTimeout(this.timeoutTimer);
+                        }
+                        else {
+                            this.intervalTimer = setTimeout(function () { return _this.runCheck(subprojs, tries + 1, interval); }, interval);
+                        }
+                        return [3 /*break*/, 3];
+                    case 2:
+                        error_1 = _a.sent();
+                        // bubble up the error to the job
+                        core.setFailed(error_1);
+                        clearTimeout(this.intervalTimer);
+                        clearTimeout(this.timeoutTimer);
+                        return [3 /*break*/, 3];
+                    case 3: return [2 /*return*/];
+                }
+            });
+        });
+    };
     /**
      * Gets a list of files that are modified in
      * a pull request.

diff --git a/dist/index.js b/dist/index.js
@@ -30,11 +30,11 @@ var auto_cc_bot_1 = __importDefault(require("./auto-cc/auto-cc-bot"));
 var index_1 = __importDefault(require("./check-group/index"));
 var core = __importStar(require("@actions/core"));
 function botSteps(app) {
-    var job = core.getInput('job');
-    if (job === 'auto-cc') {
+    var job = core.getInput("job");
+    if (job === "auto-cc") {
         (0, auto_cc_bot_1.default)(app);
     }
-    else if (job === 'check-group') {
+    else if (job === "check-group") {
         (0, index_1.default)(app);
     }
     else {

diff --git a/src/action.ts b/src/action.ts
@@ -1,7 +1,7 @@
 // Require your Probot app's entrypoint, usually this is just index.js
-import botSteps from './index';
+import botSteps from "./index";
 // Require the adapter
-import probot from '@probot/adapter-github-actions';
+import probot from "@probot/adapter-github-actions";
 
 // Adapt the Probot app for Actions
 // This also acts as the main entrypoint for the Action

diff --git a/src/auto-cc/auto-cc-bot.ts b/src/auto-cc/auto-cc-bot.ts
@@ -25,10 +25,8 @@ function myBot(app: Probot): void {
     const labels = context.payload[name]['labels'].map(e => e['name']);
     context.log({labels});
     const cc = new Set();
-    // eslint-disable-next-line github/array-foreach
     labels.forEach(l => {
       if (l in subscriptions) {
-        // eslint-disable-next-line github/array-foreach
         subscriptions[l].forEach(u => cc.add(u));
       }
     });
@@ -52,7 +50,6 @@ function myBot(app: Probot): void {
       // Invariant: prevCC is a subset of cc
       if (prevCC.size !== cc.size) {
         let newCCString = 'cc';
-        // eslint-disable-next-line github/array-foreach
         cc.forEach(u => {
           newCCString += ` @${u}`;
         });

diff --git a/src/auto-cc/utils.ts b/src/auto-cc/utils.ts
@@ -80,7 +80,6 @@ export function parseSubscriptions(rawSubsText): object {
   if (subsRows == null) {
     return subscriptions;
   }
-  // eslint-disable-next-line github/array-foreach
   subsRows.forEach((row: string) => {
     const labelMatch = row.match(/^\* +([^@]+)/);
     if (labelMatch) {

diff --git a/src/check-group/core/index.ts b/src/check-group/core/index.ts
@@ -23,6 +23,9 @@ export class CheckGroup {
   context: Context;
   sha: string;
 
+  intervalTimer: ReturnType<typeof setTimeout> = setTimeout(() => '', 0);
+  timeoutTimer: ReturnType<typeof setTimeout> = setTimeout(() => '', 0);
+
   constructor(
     pullRequestNumber: number,
     config: CheckGroupConfig,
@@ -49,39 +52,55 @@ export class CheckGroup {
 
     const interval = parseInt(core.getInput('interval'))
     core.info(`Check interval: ${interval}`);
-
-    let tries = 0;
-    let conclusion = undefined;
-    // IMPORTANT: a timeout should be set in the action workflow
-    const loop = setInterval(
-      async function(that) {
-        try {
-          tries += 1;
-          core.startGroup(`Check ${tries}`);
+    this.runCheck(subprojs, 1, interval * 1000)
 
-          const postedChecks = await getPostedChecks(that.context, that.sha);
-          core.debug(`postedChecks: ${JSON.stringify(postedChecks)}`);
-
-          conclusion = satisfyExpectedChecks(subprojs, postedChecks);
-          const summary = generateProgressSummary(subprojs, postedChecks)
-          const details = generateProgressDetails(subprojs, postedChecks)
-          core.info(
-            `${that.config.customServiceName} conclusion: '${conclusion}':\n${summary}\n${details}`
-          )
-          core.endGroup();
-
-          if (conclusion === "all_passing") {
-            core.info("Required checks were successful!")
-            clearInterval(loop)
-          }
-        } catch (error) {
-          core.setFailed(error);
-          clearInterval(loop)
-        }
-      }, interval * 1000, this
+    const timeout = parseInt(core.getInput('timeout'))
+    core.info(`Timeout: ${timeout}`);
+    // set a timeout that will stop the job to avoid polling the GitHub API infinitely
+    this.timeoutTimer = setTimeout(
+      () => {
+        clearTimeout(this.intervalTimer)
+        core.setFailed(
+          `The timeout of ${timeout} minutes has triggered but not all required jobs were passing.`
+          + ` This job will need to be re-run to merge your PR.`
+          + ` If you do not have write access to the repository you can ask ${core.getInput('maintainers')} to re-run it for you.`
+          + ` If you have any other questions, you can reach out to ${core.getInput('owner')} for help.`
+        )
+      }, timeout * 60 * 1000 
     )
   }
 
+  async runCheck(subprojs, tries: number, interval: number) {
+    try {
+      // print in a group to reduce verbosity
+      core.startGroup(`Check ${tries}`);
+      const postedChecks = await getPostedChecks(this.context, this.sha);
+      core.debug(`postedChecks: ${JSON.stringify(postedChecks)}`);
+
+      const conclusion = satisfyExpectedChecks(subprojs, postedChecks);
+      const summary = generateProgressSummary(subprojs, postedChecks)
+      const details = generateProgressDetails(subprojs, postedChecks)
+      core.info(
+        `${this.config.customServiceName} conclusion: '${conclusion}':\n${summary}\n${details}`
+      )
+      core.endGroup();
+
+      if (conclusion === "all_passing") {
+        core.info("All required checks were successful!")
+        clearTimeout(this.intervalTimer)
+        clearTimeout(this.timeoutTimer)
+      } else {
+        this.intervalTimer = setTimeout(() => this.runCheck(subprojs, tries + 1, interval), interval);
+      }
+
+    } catch (error) {
+      // bubble up the error to the job
+      core.setFailed(error);
+      clearTimeout(this.intervalTimer)
+      clearTimeout(this.timeoutTimer)
+    }
+  } 
+
   /**
    * Gets a list of files that are modified in
    * a pull request.

diff --git a/src/check-group/utils/generate_progress.ts b/src/check-group/utils/generate_progress.ts
@@ -1,7 +1,4 @@
-/* eslint-disable @typescript-eslint/no-unused-vars */
-import { CheckGroupConfig, ProgressReport, SubProjConfig } from "../types";
-/* eslint-enable @typescript-eslint/no-unused-vars */
-import { defaultCheckId } from "../config";
+import { ProgressReport, SubProjConfig } from "../types";
 
 export const generateProgressReport = (
   subprojects: SubProjConfig[],

diff --git a/src/index.ts b/src/index.ts
@@ -1,13 +1,13 @@
-import autoCcBot from './auto-cc/auto-cc-bot';
-import checkGroupApp from './check-group/index';
-import {Probot} from 'probot';
-import * as core from '@actions/core';
+import autoCcBot from "./auto-cc/auto-cc-bot";
+import checkGroupApp from "./check-group/index";
+import { Probot } from "probot";
+import * as core from "@actions/core";
 
 export default function botSteps(app: Probot): void {
-  const job = core.getInput('job');
-  if (job === 'auto-cc') {
+  const job = core.getInput("job");
+  if (job === "auto-cc") {
     autoCcBot(app);
-  } else if (job === 'check-group') {
+  } else if (job === "check-group") {
     checkGroupApp(app);
   } else {
     throw new Error(`Job ${job} should be one of ['auto-cc', 'check-group']`);