From 0a2ac6ee050a1466409732b52c160bf634bc9d51 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sat, 9 Apr 2022 09:18:59 -0500 Subject: [PATCH 1/4] add mypy support --- .github/workflows/connect.py | 2 +- .mypy.ini | 35 ++++++++++++++++ .pre-commit-config.yaml | 8 ++++ meeseeksdev/__init__.py | 10 ++--- meeseeksdev/commands.py | 5 ++- meeseeksdev/meeseeksbox/commands.py | 40 +++++++++++++------ meeseeksdev/meeseeksbox/core.py | 17 ++++---- meeseeksdev/meeseeksbox/utils.py | 62 +++++++++++++++++++---------- meeseeksdev/tests/test_misc.py | 4 +- meeseeksdev/tests/test_webhook.py | 2 +- 10 files changed, 134 insertions(+), 51 deletions(-) create mode 100644 .mypy.ini diff --git a/.github/workflows/connect.py b/.github/workflows/connect.py index 7ccf815..392efe6 100644 --- a/.github/workflows/connect.py +++ b/.github/workflows/connect.py @@ -13,7 +13,7 @@ found = True break except URLError as e: - if e.reason.errno == errno.ECONNREFUSED: + if e.reason.errno == errno.ECONNREFUSED: # type:ignore time.sleep(1) continue raise diff --git a/.mypy.ini b/.mypy.ini new file mode 100644 index 0000000..a473347 --- /dev/null +++ b/.mypy.ini @@ -0,0 +1,35 @@ +[mypy] +check_untyped_defs = true +disallow_incomplete_defs = true +no_implicit_optional = true +pretty = true +show_error_context = true +show_error_codes = true +strict_equality = true +strict_optional = true +warn_no_return = true +warn_return_any = true +warn_unused_configs = true +warn_unused_ignores = true +warn_redundant_casts = true + +[mypy-black] +ignore_missing_imports = True + +[mypy-jwt] +ignore_missing_imports = True + +[mypy-git] +ignore_missing_imports = True + +[mypy-keen] +ignore_missing_imports = True + +[mypy-pytest] +ignore_missing_imports = True + +[mypy-there] +ignore_missing_imports = True + +[mypy-yieldbreaker] +ignore_missing_imports = True diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9de6e68..66bef1c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -36,6 +36,14 @@ repos: ] stages: [manual] +- repo: https://github.com/pre-commit/mirrors-mypy + rev: "v0.942" + hooks: + - id: mypy + args: ["--config-file", ".mypy.ini"] + additional_dependencies: [types-requests, types-PyYAML, types-mock, tornado] + stages: [manual] + - repo: https://github.com/sirosen/check-jsonschema rev: 0.14.2 hooks: diff --git a/meeseeksdev/__init__.py b/meeseeksdev/__init__.py index 6f1534a..d8ee694 100644 --- a/meeseeksdev/__init__.py +++ b/meeseeksdev/__init__.py @@ -39,7 +39,7 @@ "scikit-image", ] -usr_denylist = [] +usr_denylist: list = [] usr_allowlist = [ "Carreau", @@ -91,12 +91,12 @@ def load_config_from_env(): """ Load the configuration, for now stored in the environment """ - config = {} + config: dict = {} - integration_id = os.environ.get("GITHUB_INTEGRATION_ID") + integration_id_str = os.environ.get("GITHUB_INTEGRATION_ID") botname = os.environ.get("GITHUB_BOT_NAME", None) - if not integration_id: + if not integration_id_str: raise ValueError("Please set GITHUB_INTEGRATION_ID") if not botname: @@ -106,7 +106,7 @@ def load_config_from_env(): botname = botname.replace("@", "") at_botname = "@" + botname - integration_id = int(integration_id) + integration_id = int(integration_id_str) if "B64KEY" in os.environ: config["key"] = base64.b64decode(bytes(os.environ["B64KEY"], "ASCII")) diff --git a/meeseeksdev/commands.py b/meeseeksdev/commands.py index d04d0e5..babc56c 100644 --- a/meeseeksdev/commands.py +++ b/meeseeksdev/commands.py @@ -3,6 +3,7 @@ """ from textwrap import dedent +from typing import Generator, Optional from .meeseeksbox.commands import tag, untag from .meeseeksbox.scopes import everyone, pr_author, write @@ -49,7 +50,9 @@ def open(*, session, payload, arguments, local_config=None): @write -def migrate_issue_request(*, session: Session, payload: dict, arguments: str, local_config=None): +def migrate_issue_request( + *, session: Session, payload: dict, arguments: str, local_config: Optional[dict] = None +) -> Generator: """[to] {org}/{repo} Need to be admin on target repo. Replicate all comments on target repo and close current on. diff --git a/meeseeksdev/meeseeksbox/commands.py b/meeseeksdev/meeseeksbox/commands.py index 0589478..3a4cf44 100644 --- a/meeseeksdev/meeseeksbox/commands.py +++ b/meeseeksdev/meeseeksbox/commands.py @@ -11,6 +11,7 @@ import traceback from pathlib import Path from textwrap import dedent +from typing import Generator, Optional import git import mock @@ -117,12 +118,12 @@ def _compute_pwd_changes(allowlist): print("== pwd", os.getcwd()) print("== listdir", os.listdir()) - for p in glob.glob("**/*.py", recursive=True): - print("=== scanning", p, p in allowlist) - if p not in allowlist: + for path in glob.glob("**/*.py", recursive=True): + print("=== scanning", path, path in allowlist) + if path not in allowlist: # we don't touch files not in this PR. continue - p = Path(p) + p = Path(path) old = p.read_text() new = black.format_str(old, mode=black.FileMode()) if new != old: @@ -309,7 +310,9 @@ def black_suggest(*, session, payload, arguments, local_config=None): print("== Done cleaning ") -def prep_for_command(name, session, payload, arguments, local_config=None): +def prep_for_command( + name: str, session: Session, payload: dict, arguments: str, local_config: Optional[dict] = None +) -> Generator: """Prepare to run a command against a local checkout of a repo.""" print(f"===== running command {name} =====") print("===== ============ =====") @@ -335,8 +338,11 @@ def prep_for_command(name, session, payload, arguments, local_config=None): repo_name = pr_data["head"]["repo"]["name"] maintainer_can_modify = pr_data["maintainer_can_modify"] + print(f"Got author login {author_login}") + # Check to see if we can successfully push changees to the PR. target_session = yield "{}/{}".format(author_login, repo_name) + if target_session: print("installed on target repository") atk = target_session.token() @@ -429,7 +435,9 @@ def push_the_work(session, payload, arguments, local_config=None): @admin -def precommit(*, session, payload, arguments, local_config=None): +def precommit( + *, session: Session, payload: dict, arguments: str, local_config: Optional[dict] = None +) -> Generator: comment_url = payload["issue"]["comments_url"] """Run pre-commit against a PR and push the changes.""" @@ -602,12 +610,13 @@ def safe_backport(session, payload, arguments, local_config=None): print = lambda *args, **kwargs: builtins.print(" [backport]", *args, **kwargs) - s_clone_time = 0 + s_clone_time = 0.0 s_success = False s_reason = "unknown" - s_fork_time = 0 - s_clean_time = 0 - s_ff_time = 0 + s_fork_time = 0.0 + s_clean_time = 0.0 + s_ff_time = 0.0 + s_slug = "" def keen_stats(): nonlocal s_slug @@ -816,7 +825,7 @@ def keen_stats(): print("== All has been fetched correctly") print("Cherry-picking %s" % merge_sha) - args = ("-x", "-m", "1", merge_sha) + args: tuple = ("-x", "-m", "1", merge_sha) msg = "Backport PR #%i: %s" % (prnumber, prtitle) remote_submit_branch = f"auto-backport-of-pr-{prnumber}-on-{target_branch}" @@ -925,7 +934,10 @@ def keen_stats(): session.post_comment( comment_url, "Hum, I actually crashed, that should not have happened." ) - print("\n" + e.stderr.decode("utf8", "replace"), file=sys.stderr) + if hasattr(e, "stderr"): + print( + "\n" + e.stderr.decode("utf8", "replace"), file=sys.stderr + ) # type:ignore[attr-defined] print("\n" + repo.git.status(), file=sys.stderr) add_event("error", {"git_crash": 1}) s_reason = "Unknown error line 501" @@ -1133,7 +1145,9 @@ def untag(session, payload, arguments, local_config=None): @write -def migrate_issue_request(*, session: Session, payload: dict, arguments: str, local_config=None): +def migrate_issue_request( + *, session: Session, payload: dict, arguments: str, local_config: Optional[dict] = None +) -> Generator: """Todo: - Works through pagination of comments diff --git a/meeseeksdev/meeseeksbox/core.py b/meeseeksdev/meeseeksbox/core.py index 681835b..c8bbd03 100644 --- a/meeseeksdev/meeseeksbox/core.py +++ b/meeseeksdev/meeseeksbox/core.py @@ -4,7 +4,9 @@ import json import re import time +from asyncio import Future from concurrent.futures import ThreadPoolExecutor as Pool +from typing import Optional import tornado.httpserver import tornado.ioloop @@ -26,7 +28,7 @@ class Config: botname = None - integration_id = None + integration_id = -1 key = None botname = None at_botname = None @@ -79,7 +81,7 @@ def _strip_extras(c): return c -def process_mentionning_comment(body, bot_re): +def process_mentioning_comment(body: str, bot_re: re.Pattern) -> list: """ Given a comment body and a bot name parse this into a tuple of (command, arguments) """ @@ -233,7 +235,7 @@ def mention_bot_re(self): botname = self.config.botname return re.compile("@?" + re.escape(botname) + r"(?:\[bot\])?", re.IGNORECASE) - def dispatch_action(self, type_, payload): + def dispatch_action(self, type_: str, payload: dict) -> Future: botname = self.config.botname repo = payload.get("repository", {}).get("full_name", red + "" + normal) # new issue/PR opened @@ -349,13 +351,13 @@ def dispatch_action(self, type_, payload): milestone = is_pr.get("milestone", {}) if milestone: description.append(milestone.get("description", "") or "") - description = "\n".join(description) + description_str = "\n".join(description) if "on-merge:" in description and is_pr["base"]["ref"] in ( "master", "main", ): did_backport = False - for description_line in description.splitlines(): + for description_line in description_str.splitlines(): line = description_line.strip() if line.startswith("on-merge:"): todo = line[len("on-merge:") :].strip() @@ -381,6 +383,7 @@ def dispatch_action(self, type_, payload): else: pass # print(f"({repo}) can't deal with `{type_}` yet") + return self.finish() # def _action_allowed(args): # """ @@ -393,7 +396,7 @@ def dispatch_action(self, type_, payload): # - If pull-request, the requester is the author. # """ - def dispatch_on_mention(self, body, payload, user): + def dispatch_on_mention(self, body: str, payload: dict, user: str) -> None: """ Core of the logic that let people require actions from the bot. @@ -445,7 +448,7 @@ def dispatch_on_mention(self, body, payload, user): print(user, "is legitimate author of this PR, letting commands go through") permission_level = session._get_permission(org, repo, user) - command_args = process_mentionning_comment(body, self.mention_bot_re) + command_args = process_mentioning_comment(body, self.mention_bot_re) for (command, arguments) in command_args: print(" :: treating", command, arguments) add_event( diff --git a/meeseeksdev/meeseeksbox/utils.py b/meeseeksdev/meeseeksbox/utils.py index 0f5230a..54895d4 100644 --- a/meeseeksdev/meeseeksbox/utils.py +++ b/meeseeksdev/meeseeksbox/utils.py @@ -7,6 +7,7 @@ import re import shlex import subprocess +from typing import Any, Dict, Optional, cast import jwt import requests @@ -90,7 +91,13 @@ def fix_comment_body(body, original_poster, original_url, original_org, original class Authenticator: - def __init__(self, integration_id, rsadata, personal_account_token, personal_account_name): + def __init__( + self, + integration_id: int, + rsadata: Optional[str], + personal_account_token: Optional[str], + personal_account_name: Optional[str], + ): self.since = int(datetime.datetime.now().timestamp()) self.duration = 60 * 10 self._token = None @@ -100,10 +107,10 @@ def __init__(self, integration_id, rsadata, personal_account_token, personal_acc self.personal_account_name = personal_account_name # TODO: this mapping is built at startup, we should update it when we # have new / deleted installations - self.idmap = {} + self.idmap: Dict[str, str] = {} self._session_class = Session - def session(self, installation_id): + def session(self, installation_id: str) -> "Session": """ Given and installation id, return a session with the right credentials """ @@ -117,7 +124,7 @@ def session(self, installation_id): self.personal_account_name, ) - def list_installations(self): + def list_installations(self) -> Any: """ Todo: Pagination """ @@ -198,13 +205,14 @@ def __init__( super().__init__(integration_id, rsadata, personal_account_token, personal_account_name) self.installation_id = installation_id - def token(self): + def token(self) -> str: now = datetime.datetime.now().timestamp() if (now > self.since + self.duration - 60) or (self._token is None): self.regen_token() + assert self._token is not None return self._token - def regen_token(self): + def regen_token(self) -> None: method = "POST" url = f"https://api.github.com/app/installations/{self.installation_id}/access_tokens" resp = self._integration_authenticated_request(method, url) @@ -216,7 +224,9 @@ def regen_token(self): except Exception: raise ValueError(resp.content, url) - def personal_request(self, method, url, json=None, raise_for_status=True): + def personal_request( + self, method: str, url: str, json: Optional[dict] = None, raise_for_status: bool = True + ) -> requests.Response: """ Does a request but using the personal account name and token """ @@ -243,13 +253,13 @@ def prepare(): def ghrequest( self, - method, - url, - json=None, + method: str, + url: str, + json: Optional[dict] = None, *, - override_accept_header=None, - raise_for_status=True, - ): + override_accept_header: Optional[str] = None, + raise_for_status: Optional[bool] = True, + ) -> requests.Response: accept = ACCEPT_HEADER if override_accept_header: accept = override_accept_header @@ -294,7 +304,7 @@ def prepare(): ) return response - def _get_permission(self, org, repo, username): + def _get_permission(self, org: str, repo: str, username: str) -> Permission: get_collaborators_query = API_COLLABORATORS_TEMPLATE.format( org=org, repo=repo, username=username ) @@ -307,19 +317,21 @@ def _get_permission(self, org, repo, username): resp.raise_for_status() permission = resp.json()["permission"] print("found permission", permission, "for user ", username, "on ", org, repo) - return getattr(Permission, permission) + return cast(Permission, getattr(Permission, permission)) - def has_permission(self, org, repo, username, level=None): + def has_permission( + self, org: str, repo: str, username: str, level: Optional[Permission] = None + ) -> bool: """ """ if not level: level = Permission.none return self._get_permission(org, repo, username).value >= level.value - def post_comment(self, comment_url, body): + def post_comment(self, comment_url: str, body: str) -> None: self.ghrequest("POST", comment_url, json={"body": body}) - def get_collaborator_list(self, org, repo): + def get_collaborator_list(self, org: str, repo: str) -> Optional[Any]: get_collaborators_query = "https://api.github.com/repos/{org}/{repo}/collaborators".format( org=org, repo=repo ) @@ -328,11 +340,19 @@ def get_collaborator_list(self, org, repo): return resp.json() else: resp.raise_for_status() + return None def create_issue( - self, org: str, repo: str, title: str, body: str, *, labels=None, assignees=None - ): - arguments = {"title": title, "body": body} + self, + org: str, + repo: str, + title: str, + body: str, + *, + labels: Optional[list] = None, + assignees: Optional[list] = None, + ) -> requests.Response: + arguments: dict = {"title": title, "body": body} if labels: if type(labels) in (list, tuple): diff --git a/meeseeksdev/tests/test_misc.py b/meeseeksdev/tests/test_misc.py index bc6c7f6..b474cf3 100644 --- a/meeseeksdev/tests/test_misc.py +++ b/meeseeksdev/tests/test_misc.py @@ -1,7 +1,7 @@ import re import textwrap -from ..meeseeksbox.core import process_mentionning_comment +from ..meeseeksbox.core import process_mentioning_comment def test1(): @@ -9,7 +9,7 @@ def test1(): reg = re.compile("@?" + re.escape(botname) + r"(?:\[bot\])?", re.IGNORECASE) assert ( - process_mentionning_comment( + process_mentioning_comment( textwrap.dedent( """ @meeseeksdev nothing diff --git a/meeseeksdev/tests/test_webhook.py b/meeseeksdev/tests/test_webhook.py index 29edab3..2639136 100644 --- a/meeseeksdev/tests/test_webhook.py +++ b/meeseeksdev/tests/test_webhook.py @@ -5,7 +5,7 @@ from ..meeseeksbox.core import Authenticator, Config, WebHookHandler -commands = {} +commands: dict = {} config = Config( integration_id=100, From e5d445ea163dde80c5544a225d9655812d237ea9 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sat, 9 Apr 2022 09:25:16 -0500 Subject: [PATCH 2/4] pre-commit --- meeseeksdev/meeseeksbox/commands.py | 5 +++-- meeseeksdev/meeseeksbox/core.py | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/meeseeksdev/meeseeksbox/commands.py b/meeseeksdev/meeseeksbox/commands.py index 3a4cf44..8b261da 100644 --- a/meeseeksdev/meeseeksbox/commands.py +++ b/meeseeksdev/meeseeksbox/commands.py @@ -936,8 +936,9 @@ def keen_stats(): ) if hasattr(e, "stderr"): print( - "\n" + e.stderr.decode("utf8", "replace"), file=sys.stderr - ) # type:ignore[attr-defined] + "\n" + e.stderr.decode("utf8", "replace"), # type:ignore[attr-defined] + file=sys.stderr, + ) print("\n" + repo.git.status(), file=sys.stderr) add_event("error", {"git_crash": 1}) s_reason = "Unknown error line 501" diff --git a/meeseeksdev/meeseeksbox/core.py b/meeseeksdev/meeseeksbox/core.py index c8bbd03..3629603 100644 --- a/meeseeksdev/meeseeksbox/core.py +++ b/meeseeksdev/meeseeksbox/core.py @@ -6,7 +6,6 @@ import time from asyncio import Future from concurrent.futures import ThreadPoolExecutor as Pool -from typing import Optional import tornado.httpserver import tornado.ioloop From 69bac99d6665147cd9e9f90bbd280c42e8f8b2b7 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sat, 9 Apr 2022 09:36:05 -0500 Subject: [PATCH 3/4] improve support --- .mypy.ini | 12 ------------ .pre-commit-config.yaml | 2 +- meeseeksdev/meeseeksbox/commands.py | 4 ++-- meeseeksdev/meeseeksbox/utils.py | 1 + 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/.mypy.ini b/.mypy.ini index a473347..3767677 100644 --- a/.mypy.ini +++ b/.mypy.ini @@ -13,21 +13,9 @@ warn_unused_configs = true warn_unused_ignores = true warn_redundant_casts = true -[mypy-black] -ignore_missing_imports = True - -[mypy-jwt] -ignore_missing_imports = True - -[mypy-git] -ignore_missing_imports = True - [mypy-keen] ignore_missing_imports = True -[mypy-pytest] -ignore_missing_imports = True - [mypy-there] ignore_missing_imports = True diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 66bef1c..e5ba856 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -41,7 +41,7 @@ repos: hooks: - id: mypy args: ["--config-file", ".mypy.ini"] - additional_dependencies: [types-requests, types-PyYAML, types-mock, tornado] + additional_dependencies: [types-requests, types-PyYAML, types-mock, tornado, black, pytest, gitpython, pyjwt] stages: [manual] - repo: https://github.com/sirosen/check-jsonschema diff --git a/meeseeksdev/meeseeksbox/commands.py b/meeseeksdev/meeseeksbox/commands.py index 8b261da..cf2f369 100644 --- a/meeseeksdev/meeseeksbox/commands.py +++ b/meeseeksdev/meeseeksbox/commands.py @@ -430,7 +430,7 @@ def push_the_work(session, payload, arguments, local_config=None): "GET", f"https://api.github.com/repos/{org_name}/{repo_name}" ).json()["default_branch"] repo.git.checkout(default_branch) - repo.branches.workbranch.delete(repo, "workbranch", force=True) + repo.branches.workbranch.delete(repo, "workbranch", force=True) # type:ignore[attr-defined] return succeeded @@ -971,7 +971,7 @@ def keen_stats(): # TODO comment on issue print(e) repo.git.checkout(default_branch) - repo.branches.workbranch.delete(repo, "workbranch", force=True) + repo.branches.workbranch.delete(repo, "workbranch", force=True) # type:ignore[attr-defined] # TODO checkout the default_branch and get rid of branch diff --git a/meeseeksdev/meeseeksbox/utils.py b/meeseeksdev/meeseeksbox/utils.py index 54895d4..a108c63 100644 --- a/meeseeksdev/meeseeksbox/utils.py +++ b/meeseeksdev/meeseeksbox/utils.py @@ -175,6 +175,7 @@ def _integration_authenticated_request(self, method, url): } ) + assert self.rsadata is not None tok = jwt.encode(payload, key=self.rsadata, algorithm="RS256") headers = { From 7cc8f8faeddf543ace875b4f038eae4310f6b810 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sat, 9 Apr 2022 09:37:55 -0500 Subject: [PATCH 4/4] remove extra comment --- meeseeksdev/meeseeksbox/commands.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/meeseeksdev/meeseeksbox/commands.py b/meeseeksdev/meeseeksbox/commands.py index cf2f369..e801494 100644 --- a/meeseeksdev/meeseeksbox/commands.py +++ b/meeseeksdev/meeseeksbox/commands.py @@ -338,8 +338,6 @@ def prep_for_command( repo_name = pr_data["head"]["repo"]["name"] maintainer_can_modify = pr_data["maintainer_can_modify"] - print(f"Got author login {author_login}") - # Check to see if we can successfully push changees to the PR. target_session = yield "{}/{}".format(author_login, repo_name)