[autorevert] Remove some vibes from the code (#7013)

Author: jeanschmidt

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/__main__.py

@@ -10,7 +10,6 @@
 from .clickhouse_client_helper import CHCliFactory
 from .github_client_helper import GHClientFactory
 from .testers.autorevert import autorevert_checker
-from .testers.do_restart import do_restart_workflow
 from .testers.restart_checker import workflow_restart_checker
 
 
@@ -65,7 +64,7 @@ def get_opts() -> argparse.Namespace:
     parser.add_argument(
         "--dry-run",
         action="store_true",
-        help="Show what would be restarted without actually doing it (use with --do-restart)",
+        help="Show what would be restarted without actually doing it",
     )
 
     # no subcommand runs the lambda flow
@@ -125,19 +124,6 @@ def get_opts() -> argparse.Namespace:
         help="If no `--commit` specified, look back days for bulk query (default: 7)",
     )
 
-    # do-restart subcommand
-    do_restart_parser = subparsers.add_parser(
-        "do-restart", help="Restart a workflow for a specific commit"
-    )
-    do_restart_parser.add_argument(
-        "workflow",
-        help="Workflow file name to restart (e.g., trunk.yml)",
-    )
-    do_restart_parser.add_argument(
-        "commit",
-        help="Commit SHA to restart the workflow for",
-    )
-
     return parser.parse_args()
 
 
@@ -191,8 +177,6 @@ def main(*args, **kwargs) -> None:
         )
     elif opts.subcommand == "workflow-restart-checker":
         workflow_restart_checker(opts.workflow, commit=opts.commit, days=opts.days)
-    elif opts.subcommand == "do-restart":
-        do_restart_workflow(opts.workflow, commit=opts.commit)
 
 
 if __name__ == "__main__":

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/autorevert_checker.py

@@ -45,11 +45,9 @@ def has_pending_jobs(self) -> bool:
         """Check if any jobs are not yet completed (queued/in_progress)."""
         return any(j.status != "completed" for j in self.jobs)
 
-    @property
+    @cached_property
     def job_base_names(self) -> Set[str]:
-        if not hasattr(self, "_job_base_names"):
-            self._job_base_names = self.get_job_base_names()
-        return self._job_base_names
+        return self.get_job_base_names()
 
     def normalize_job_name(self, name: str) -> str:
         """Normalize job name to a stable base for matching across commits.
@@ -100,7 +98,6 @@ def workflow_commits(self) -> List[CommitJobs]:
 
     @cached_property
     def commit_history(self) -> List[Dict]:
-        """Get commit history, fetching if needed."""
         return self._fetch_commit_history()
 
     @cached_property

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/event_logger.py

@@ -22,7 +22,6 @@ def log_autorevert_event(
     Uses the misc.autorevert_events table. Best-effort: logs and continues on failure.
     """
     try:
-        client = CHCliFactory().client
         columns = [
             "workflow",
             "action",
@@ -48,7 +47,7 @@ def log_autorevert_event(
             ]
         ]
         # Specify database explicitly since this table lives in 'misc'
-        client.insert(
+        CHCliFactory().client.insert(
             table="autorevert_events",
             data=data,
             column_names=columns,

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/testers/autorevert.py

@@ -116,12 +116,8 @@ def autorevert_checker(
 
             # For clarity in naming
             workflow_name = pattern["workflow_name"]
-            first_failing = pattern["newer_commits"][
-                1
-            ]  # the older of the two failing commits
-            previous_commit = pattern[
-                "older_commit"
-            ]  # previously successful commit for the matched job
+            first_failing = pattern["newer_commits"][1]
+            previous_commit = pattern["older_commit"]
             # Prepare base DTO for logging events for this pattern
             evt = AutoRevertEvent(
                 workflow=workflow_name,

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/testers/do_restart.py

@@ -2,14 +2,13 @@
 
 
 def workflow_restart_checker(workflow: str, commit: str = None, days: int = 7) -> None:
-    checker = WorkflowRestartChecker()
     if commit:
         # Check specific commit
-        result = checker.has_restarted_workflow(workflow, commit)
+        result = WorkflowRestartChecker().has_restarted_workflow(workflow, commit)
         print(f"Commit {commit}: {'✓ RESTARTED' if result else '✗ Not restarted'}")
     else:
         # Get all restarted commits in date range
-        commits = checker.get_restarted_commits(workflow, days)
+        commits = WorkflowRestartChecker().get_restarted_commits(workflow, days)
         print(f"Restarted commits for {workflow} (last {days} days):")
         if commits:
             for commit in sorted(commits):

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/testers/restart_checker.py

@@ -5,7 +5,6 @@
 
 import logging
 from datetime import datetime, timedelta
-from functools import cached_property
 from typing import Dict, Set
 
 from .clickhouse_client_helper import CHCliFactory
@@ -173,6 +172,5 @@ def restart_workflow(self, workflow_name: str, commit_sha: str) -> bool:
             logging.error(f"Error dispatching workflow {workflow_name}: {e}")
             return False
 
-    @cached_property
     def resolver(self) -> WorkflowResolver:
         return WorkflowResolver.get(f"{self.repo_owner}/{self.repo_name}")

aws/lambda/pytorch-auto-revert/pytorch_auto_revert/workflow_checker.py

@@ -8,8 +8,8 @@
 from __future__ import annotations
 
 import os
+import re
 from dataclasses import dataclass
-from functools import lru_cache
 from typing import Optional
 
 import github
@@ -34,6 +34,8 @@ class WorkflowResolver:
         wf = resolver.resolve("pull.yml")    # file basename
     """
 
+    _resolver_cache: dict[str, WorkflowResolver] = {}
+
     def __init__(
         self, repo_full_name: str, repository: "github.Repository.Repository"
     ) -> None:
@@ -43,22 +45,33 @@ def __init__(
         self._by_file: dict[str, WorkflowRef] = {}
         self._build_indices()
 
+    def __new__(
+        cls, repo_full_name: str, repository: "github.Repository.Repository"
+    ) -> WorkflowResolver:
+        """Create or return a cached resolver for the given repo."""
+
+        if re.match(r"^[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+$", repo_full_name) is None:
+            raise ValueError(
+                f"Invalid repo format: {repo_full_name}. Expected 'owner/repo'."
+            )
+
+        if repository is None or not isinstance(
+            repository, github.Repository.Repository
+        ):
+            raise ValueError(f"Invalid repository object for {repo_full_name}.")
+
+        if repo_full_name not in cls._resolver_cache:
+            cls._resolver_cache[repo_full_name] = super().__new__(cls)
+        return cls._resolver_cache[repo_full_name]
+
     @staticmethod
-    @lru_cache(maxsize=None)
     def get(repo: str) -> "WorkflowResolver":
         """Get a cached resolver for a repo in owner/repo format.
 
         Internally creates a GitHub Repository client using GHClientFactory when
         available; otherwise falls back to an anonymous client for public repos.
         """
-        # Build a client: prefer configured factory; fall back to anonymous
-        try:
-            client = GHClientFactory().client
-        except Exception:
-            # Anonymous client for public data; may be rate limited
-            client = github.Github()
-
-        repository = client.get_repo(repo)
+        repository = GHClientFactory().client.get_repo(repo)
         return WorkflowResolver(repo_full_name=repo, repository=repository)
 
     def resolve(self, input_name: str) -> Optional[WorkflowRef]:
@@ -76,7 +89,6 @@ def require(self, input_name: str) -> WorkflowRef:
         """Resolve or raise ValueError with a helpful message."""
         ref = self.resolve(input_name)
         if ref is None:
-            # Build an informative message with available names
             display = ", ".join(sorted(self._by_display))
             files = ", ".join(sorted(self._by_file))
             raise ValueError(
@@ -85,8 +97,6 @@ def require(self, input_name: str) -> WorkflowRef:
             )
         return ref
 
-    # Internal helpers
-
     def _build_indices(self) -> None:
         for w in self._repository.get_workflows():
             name = getattr(w, "name", "") or ""