[primer] Parallelize primer workflow into package batches (#8697)

Co-authored-by: Pierre Sassoulas <[email protected]>

Author: jacobtylerwalls

.github/workflows/primer_comment.yaml

@@ -103,8 +103,9 @@ jobs:
           . venv/bin/activate
           python tests/primer/__main__.py compare \
           --commit=${{ github.event.workflow_run.head_sha }} \
-          --base-file=output_${{ steps.python.outputs.python-version }}_main.txt \
-          --new-file=output_${{ steps.python.outputs.python-version }}_pr.txt
+          --base-file=output_${{ steps.python.outputs.python-version }}_main_BATCHIDX.txt \
+          --new-file=output_${{ steps.python.outputs.python-version }}_pr_BATCHIDX.txt \
+          --batches=4
       - name: Post comment
         id: post-comment
         uses: actions/[email protected]

.github/workflows/primer_run_main.yaml

@@ -24,12 +24,14 @@ permissions:
 
 jobs:
   run-primer:
-    name: Run / ${{ matrix.python-version }}
+    name: Run / ${{ matrix.python-version }} / batch index ${{ matrix.batchIdx }}
     runs-on: ubuntu-latest
-    timeout-minutes: 120
+    timeout-minutes: 45
     strategy:
       matrix:
         python-version: ["3.8", "3.11"]
+        batches: [4]
+        batchIdx: [0, 1, 2, 3]
     steps:
       - name: Check out code from GitHub
         uses: actions/[email protected]
@@ -91,7 +93,10 @@ jobs:
         run: |
           . venv/bin/activate
           pip install -e .
-          python tests/primer/__main__.py run --type=main 2>warnings.txt
+          python tests/primer/__main__.py run --type=main --batches=${{ matrix.batches }} --batchIdx=${{ matrix.batchIdx }} 2>warnings.txt
+      - name: Echo warnings
+        if: success() || failure()
+        run: |
           WARNINGS=$(head -c 65000 < warnings.txt)
           if [[ $WARNINGS ]]
           then echo "::warning ::$WARNINGS"
@@ -102,4 +107,4 @@ jobs:
           name: primer_output
           path: >-
             tests/.pylint_primer_tests/output_${{ steps.python.outputs.python-version
-            }}_main.txt
+            }}_main_batch${{ matrix.batchIdx }}.txt

.github/workflows/primer_run_pr.yaml

@@ -33,12 +33,14 @@ permissions:
 
 jobs:
   run-primer:
-    name: Run / ${{ matrix.python-version }}
+    name: Run / ${{ matrix.python-version }} / batch index ${{ matrix.batchIdx }}
     runs-on: ubuntu-latest
-    timeout-minutes: 120
+    timeout-minutes: 45
     strategy:
       matrix:
         python-version: ["3.8", "3.11"]
+        batches: [4]
+        batchIdx: [0, 1, 2, 3]
     steps:
       - name: Check out code from GitHub
         uses: actions/[email protected]
@@ -156,7 +158,10 @@ jobs:
         run: |
           . venv/bin/activate
           pip install -e .
-          python tests/primer/__main__.py run --type=pr 2>warnings.txt
+          python tests/primer/__main__.py run --type=pr --batches=${{ matrix.batches }} --batchIdx=${{ matrix.batchIdx }} 2>warnings.txt
+      - name: Echo warnings
+        if: success() || failure()
+        run: |
           WARNINGS=$(head -c 65000 < warnings.txt)
           if [[ $WARNINGS ]]
           then echo "::warning ::$WARNINGS"
@@ -167,12 +172,14 @@ jobs:
           name: primer_output_pr
           path:
             tests/.pylint_primer_tests/output_${{ steps.python.outputs.python-version
-            }}_pr.txt
+            }}_pr_batch${{ matrix.batchIdx }}.txt
       - name: Upload output of 'main'
         uses: actions/[email protected]
         with:
           name: primer_output_main
-          path: output_${{ steps.python.outputs.python-version }}_main.txt
+          path:
+            output_${{ steps.python.outputs.python-version }}_main_batch${{
+            matrix.batchIdx }}.txt
 
       # Save PR number so we know which PR to comment on
       - name: Save PR number

pylint/testutils/_primer/primer.py

@@ -56,6 +56,18 @@ def __init__(self, primer_directory: Path, json_path: Path) -> None:
         run_parser.add_argument(
             "--type", choices=["main", "pr"], required=True, help="Type of primer run."
         )
+        run_parser.add_argument(
+            "--batches",
+            required=False,
+            type=int,
+            help="Number of batches",
+        )
+        run_parser.add_argument(
+            "--batchIdx",
+            required=False,
+            type=int,
+            help="Portion of primer packages to run.",
+        )
 
         # All arguments for the compare parser
         compare_parser = self._subparsers.add_parser("compare")
@@ -74,6 +86,12 @@ def __init__(self, primer_directory: Path, json_path: Path) -> None:
             required=True,
             help="Commit hash of the PR commit being checked.",
         )
+        compare_parser.add_argument(
+            "--batches",
+            required=False,
+            type=int,
+            help="Number of batches (filepaths with the placeholder BATCHIDX will be numbered)",
+        )
 
         # Storing arguments
         self.config = self._argument_parser.parse_args()

pylint/testutils/_primer/primer_compare_command.py

@@ -18,8 +18,24 @@
 
 class CompareCommand(PrimerCommand):
     def run(self) -> None:
-        main_data = self._load_json(self.config.base_file)
-        pr_data = self._load_json(self.config.new_file)
+        if self.config.batches is None:
+            main_data = self._load_json(self.config.base_file)
+            pr_data = self._load_json(self.config.new_file)
+        else:
+            main_data = {}
+            pr_data = {}
+            for idx in range(self.config.batches):
+                main_data.update(
+                    self._load_json(
+                        self.config.base_file.replace("BATCHIDX", "batch" + str(idx))
+                    )
+                )
+                pr_data.update(
+                    self._load_json(
+                        self.config.new_file.replace("BATCHIDX", "batch" + str(idx))
+                    )
+                )
+
         missing_messages_data, new_messages_data = self._cross_reference(
             main_data, pr_data
         )

pylint/testutils/_primer/primer_run_command.py

@@ -30,14 +30,21 @@ class RunCommand(PrimerCommand):
     def run(self) -> None:
         packages: PackageMessages = {}
         fatal_msgs: list[Message] = []
-        for package, data in self.packages.items():
+        package_data_iter = (
+            self.packages.items()
+            if self.config.batches is None
+            else list(self.packages.items())[
+                self.config.batchIdx :: self.config.batches
+            ]
+        )
+        for package, data in package_data_iter:
             messages, p_fatal_msgs = self._lint_package(package, data)
             fatal_msgs += p_fatal_msgs
             local_commit = Repo(data.clone_directory).head.object.hexsha
             packages[package] = PackageData(commit=local_commit, messages=messages)
-        path = (
-            self.primer_directory
-            / f"output_{'.'.join(str(i) for i in sys.version_info[:3])}_{self.config.type}.txt"
+        path = self.primer_directory / (
+            f"output_{'.'.join(str(i) for i in sys.version_info[:3])}_{self.config.type}"
+            + (f"_batch{self.config.batchIdx}.txt" if self.config.batches else "")
         )
         print(f"Writing result in {path}")
         with open(path, "w", encoding="utf-8") as f:

tests/testutils/_primer/fixtures/batched/expected.txt

@@ -0,0 +1,19 @@
+🤖 **Effect of this PR on checked open source code:** 🤖
+
+
+
+**Effect on [astroid](https://github.com/pylint-dev/astroid):**
+The following messages are no longer emitted:
+
+<details>
+
+1) locally-disabled:
+*Locally disabling redefined-builtin (W0622)*
+https://github.com/pylint-dev/astroid/blob/1234567890abcdef/astroid/__init__.py#L91
+2) unknown-option-value:
+*Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'*
+https://github.com/pylint-dev/astroid/blob/1234567890abcdef/astroid/__init__.py#L91
+
+</details>
+
+*This comment was generated for commit v2.14.2*

tests/testutils/_primer/fixtures/batched/main_batch0.json

@@ -0,0 +1 @@
+{}

tests/testutils/_primer/fixtures/batched/main_batch1.json

@@ -0,0 +1,33 @@
+{
+  "astroid": {
+    "commit": "1234567890abcdef",
+    "messages": [
+      {
+        "type": "info",
+        "module": "astroid",
+        "obj": "",
+        "line": 91,
+        "column": 0,
+        "endLine": null,
+        "endColumn": null,
+        "path": "tests/.pylint_primer_tests/pylint-dev/astroid/astroid/__init__.py",
+        "symbol": "locally-disabled",
+        "message": "Locally disabling redefined-builtin (W0622)",
+        "message-id": "I0011"
+      },
+      {
+        "type": "warning",
+        "module": "astroid",
+        "obj": "",
+        "line": 91,
+        "column": 0,
+        "endLine": null,
+        "endColumn": null,
+        "path": "tests/.pylint_primer_tests/pylint-dev/astroid/astroid/__init__.py",
+        "symbol": "unknown-option-value",
+        "message": "Unknown option value for 'disable', expected a valid pylint message and got 'Ellipsis'",
+        "message-id": "W0012"
+      }
+    ]
+  }
+}

tests/testutils/_primer/fixtures/batched/pr_batch0.json

@@ -0,0 +1 @@
+{}

tests/testutils/_primer/fixtures/batched/pr_batch1.json

@@ -0,0 +1,6 @@
+{
+  "astroid": {
+    "commit": "1234567890abcdef",
+    "messages": []
+  }
+}

tests/testutils/_primer/test_primer.py

@@ -21,7 +21,7 @@
 PACKAGES_TO_PRIME_PATH = TEST_DIR_ROOT / "primer/packages_to_prime.json"
 FIXTURES_PATH = HERE / "fixtures"
 
-PRIMER_CURRENT_INTERPRETER = (3, 10)
+PRIMER_CURRENT_INTERPRETER = (3, 11)
 
 DEFAULT_ARGS = ["python tests/primer/__main__.py", "compare", "--commit=v2.14.2"]
 
@@ -49,7 +49,7 @@ class TestPrimer:
         [
             pytest.param(p, id=str(p.relative_to(FIXTURES_PATH)))
             for p in FIXTURES_PATH.iterdir()
-            if p.is_dir()
+            if p.is_dir() and p.name != "batched"  # tested separately
         ],
     )
     def test_compare(self, directory: Path) -> None:
@@ -58,6 +58,15 @@ def test_compare(self, directory: Path) -> None:
         Directory in 'fixtures/' with 'main.json', 'pr.json' and 'expected.txt'."""
         self.__assert_expected(directory)
 
+    def test_compare_batched(self) -> None:
+        fixture = FIXTURES_PATH / "batched"
+        self.__assert_expected(
+            fixture,
+            fixture / "main_BATCHIDX.json",
+            fixture / "pr_BATCHIDX.json",
+            batches=2,
+        )
+
     def test_truncated_compare(self) -> None:
         """Test for the truncation of comments that are too long."""
         max_comment_length = 525
@@ -77,6 +86,7 @@ def __assert_expected(
         main: Path | None = None,
         pr: Path | None = None,
         expected_file: Path | None = None,
+        batches: int = 0,
     ) -> str:
         if main is None:
             main = directory / "main.json"
@@ -85,6 +95,8 @@ def __assert_expected(
         if expected_file is None:
             expected_file = directory / "expected.txt"
         new_argv = [*DEFAULT_ARGS, f"--base-file={main}", f"--new-file={pr}"]
+        if batches:
+            new_argv.append(f"--batches={batches}")
         with patch("sys.argv", new_argv):
             Primer(PRIMER_DIRECTORY, PACKAGES_TO_PRIME_PATH).run()
         with open(PRIMER_DIRECTORY / "comment.txt", encoding="utf8") as f: