From 30c8e7e408322967e5beb2a64ef5f796cb8df226 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 27 Apr 2022 14:10:31 +0100 Subject: [PATCH] Make `scripts-dev` pass `mypy --disallow-untyped-defs` (#12356) Not enforced in config yet. One day. --- changelog.d/12356.misc | 1 + mypy.ini | 10 +++-- poetry.lock | 25 ++++++++++--- pyproject.toml | 4 +- scripts-dev/build_debian_packages.py | 21 +++++++---- scripts-dev/federation_client.py | 27 +++++++++----- scripts-dev/mypy_synapse_plugin.py | 4 +- scripts-dev/release.py | 55 +++++++++++++++++----------- scripts-dev/sign_json.py | 2 +- 9 files changed, 96 insertions(+), 53 deletions(-) create mode 100644 changelog.d/12356.misc diff --git a/changelog.d/12356.misc b/changelog.d/12356.misc new file mode 100644 index 0000000000..43e1929106 --- /dev/null +++ b/changelog.d/12356.misc @@ -0,0 +1 @@ +Fix scripts-dev to pass typechecking. \ No newline at end of file diff --git a/mypy.ini b/mypy.ini index 280f1e898e..ef28216418 100644 --- a/mypy.ini +++ b/mypy.ini @@ -24,10 +24,6 @@ files = # https://docs.python.org/3/library/re.html#re.X exclude = (?x) ^( - |scripts-dev/build_debian_packages.py - |scripts-dev/federation_client.py - |scripts-dev/release.py - |synapse/storage/databases/__init__.py |synapse/storage/databases/main/cache.py |synapse/storage/databases/main/devices.py @@ -308,6 +304,9 @@ ignore_missing_imports = True [mypy-pympler.*] ignore_missing_imports = True +[mypy-redbaron.*] +ignore_missing_imports = True + [mypy-rust_python_jaeger_reporter.*] ignore_missing_imports = True @@ -323,6 +322,9 @@ ignore_missing_imports = True [mypy-signedjson.*] ignore_missing_imports = True +[mypy-srvlookup.*] +ignore_missing_imports = True + [mypy-treq.*] ignore_missing_imports = True diff --git a/poetry.lock b/poetry.lock index 8c7af1fa1e..e27a44989c 100644 --- a/poetry.lock +++ b/poetry.lock @@ -309,14 +309,15 @@ smmap = ">=3.0.1,<6" [[package]] name = "gitpython" -version = "3.1.14" -description = "Python Git Library" +version = "3.1.27" +description = "GitPython is a python library used to interact with Git repositories" category = "dev" optional = false -python-versions = ">=3.4" +python-versions = ">=3.7" [package.dependencies] gitdb = ">=4.0.1,<5" +typing-extensions = {version = ">=3.7.4.3", markers = "python_version < \"3.8\""} [[package]] name = "hiredis" @@ -1315,6 +1316,14 @@ category = "dev" optional = false python-versions = "*" +[[package]] +name = "types-commonmark" +version = "0.9.2" +description = "Typing stubs for commonmark" +category = "dev" +optional = false +python-versions = "*" + [[package]] name = "types-cryptography" version = "3.3.15" @@ -1553,7 +1562,7 @@ url_preview = ["lxml"] [metadata] lock-version = "1.1" python-versions = "^3.7" -content-hash = "f482a4f594a165dfe01ce253a22510d5faf38647ab0dcebc35789350cafd9bf0" +content-hash = "3825cef058b8c9f520ef4b7acb92519be95db9a663a61c2e89a5fe431ed55655" [metadata.files] attrs = [ @@ -1766,8 +1775,8 @@ gitdb = [ {file = "gitdb-4.0.9.tar.gz", hash = "sha256:bac2fd45c0a1c9cf619e63a90d62bdc63892ef92387424b855792a6cabe789aa"}, ] gitpython = [ - {file = "GitPython-3.1.14-py3-none-any.whl", hash = "sha256:3283ae2fba31c913d857e12e5ba5f9a7772bbc064ae2bb09efafa71b0dd4939b"}, - {file = "GitPython-3.1.14.tar.gz", hash = "sha256:be27633e7509e58391f10207cd32b2a6cf5b908f92d9cd30da2e514e1137af61"}, + {file = "GitPython-3.1.27-py3-none-any.whl", hash = "sha256:5b68b000463593e05ff2b261acff0ff0972df8ab1b70d3cdbd41b546c8b8fc3d"}, + {file = "GitPython-3.1.27.tar.gz", hash = "sha256:1c885ce809e8ba2d88a29befeb385fcea06338d3640712b59ca623c220bb5704"}, ] hiredis = [ {file = "hiredis-2.0.0-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:b4c8b0bc5841e578d5fb32a16e0c305359b987b850a06964bd5a62739d688048"}, @@ -2588,6 +2597,10 @@ types-bleach = [ {file = "types-bleach-4.1.4.tar.gz", hash = "sha256:2d30c2c4fb6854088ac636471352c9a51bf6c089289800d2a8060820a01cd43a"}, {file = "types_bleach-4.1.4-py3-none-any.whl", hash = "sha256:edffe173ed6d7b6f3543036a96204a9319c3bf6c3645917b14274e43f000cc9b"}, ] +types-commonmark = [ + {file = "types-commonmark-0.9.2.tar.gz", hash = "sha256:b894b67750c52fd5abc9a40a9ceb9da4652a391d75c1b480bba9cef90f19fc86"}, + {file = "types_commonmark-0.9.2-py3-none-any.whl", hash = "sha256:56f20199a1f9a2924443211a0ef97f8b15a8a956a7f4e9186be6950bf38d6d02"}, +] types-cryptography = [ {file = "types-cryptography-3.3.15.tar.gz", hash = "sha256:a7983a75a7b88a18f88832008f0ef140b8d1097888ec1a0824ec8fb7e105273b"}, {file = "types_cryptography-3.3.15-py3-none-any.whl", hash = "sha256:d9b0dd5465d7898d400850e7f35e5518aa93a7e23d3e11757cd81b4777089046"}, diff --git a/pyproject.toml b/pyproject.toml index bdded78434..e6f2dc16cd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,6 +251,7 @@ flake8 = "*" mypy = "==0.931" mypy-zope = "==0.3.5" types-bleach = ">=4.1.0" +types-commonmark = ">=0.9.2" types-jsonschema = ">=3.2.0" types-opentracing = ">=2.4.2" types-Pillow = ">=8.3.4" @@ -270,7 +271,8 @@ idna = ">=2.5" # The following are used by the release script click = "==8.1.0" -GitPython = "==3.1.14" +# GitPython was == 3.1.14; bumped to 3.1.20, the first release with type hints. +GitPython = ">=3.1.20" commonmark = "==0.9.1" pygithub = "==1.55" # The following are executed as commands by the release script. diff --git a/scripts-dev/build_debian_packages.py b/scripts-dev/build_debian_packages.py index e3e6878686..38564893e9 100755 --- a/scripts-dev/build_debian_packages.py +++ b/scripts-dev/build_debian_packages.py @@ -17,7 +17,8 @@ import subprocess import sys import threading from concurrent.futures import ThreadPoolExecutor -from typing import Optional, Sequence +from types import FrameType +from typing import Collection, Optional, Sequence, Set DISTS = ( "debian:buster", # oldstable: EOL 2022-08 @@ -41,15 +42,17 @@ projdir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) class Builder(object): def __init__( - self, redirect_stdout=False, docker_build_args: Optional[Sequence[str]] = None + self, + redirect_stdout: bool = False, + docker_build_args: Optional[Sequence[str]] = None, ): self.redirect_stdout = redirect_stdout self._docker_build_args = tuple(docker_build_args or ()) - self.active_containers = set() + self.active_containers: Set[str] = set() self._lock = threading.Lock() self._failed = False - def run_build(self, dist, skip_tests=False): + def run_build(self, dist: str, skip_tests: bool = False) -> None: """Build deb for a single distribution""" if self._failed: @@ -63,7 +66,7 @@ class Builder(object): self._failed = True raise - def _inner_build(self, dist, skip_tests=False): + def _inner_build(self, dist: str, skip_tests: bool = False) -> None: tag = dist.split(":", 1)[1] # Make the dir where the debs will live. @@ -138,7 +141,7 @@ class Builder(object): stdout.close() print("Completed build of %s" % (dist,)) - def kill_containers(self): + def kill_containers(self) -> None: with self._lock: active = list(self.active_containers) @@ -156,8 +159,10 @@ class Builder(object): self.active_containers.remove(c) -def run_builds(builder, dists, jobs=1, skip_tests=False): - def sig(signum, _frame): +def run_builds( + builder: Builder, dists: Collection[str], jobs: int = 1, skip_tests: bool = False +) -> None: + def sig(signum: int, _frame: Optional[FrameType]) -> None: print("Caught SIGINT") builder.kill_containers() diff --git a/scripts-dev/federation_client.py b/scripts-dev/federation_client.py index 079d2f5ed0..763dd02c47 100755 --- a/scripts-dev/federation_client.py +++ b/scripts-dev/federation_client.py @@ -38,7 +38,7 @@ import argparse import base64 import json import sys -from typing import Any, Optional +from typing import Any, Dict, Optional, Tuple from urllib import parse as urlparse import requests @@ -47,13 +47,14 @@ import signedjson.types import srvlookup import yaml from requests.adapters import HTTPAdapter +from urllib3 import HTTPConnectionPool # uncomment the following to enable debug logging of http requests # from httplib import HTTPConnection # HTTPConnection.debuglevel = 1 -def encode_base64(input_bytes): +def encode_base64(input_bytes: bytes) -> str: """Encode bytes as a base64 string without any padding.""" input_len = len(input_bytes) @@ -63,7 +64,7 @@ def encode_base64(input_bytes): return output_string -def encode_canonical_json(value): +def encode_canonical_json(value: object) -> bytes: return json.dumps( value, # Encode code-points outside of ASCII as UTF-8 rather than \u escapes @@ -130,7 +131,7 @@ def request( sig, destination, ) - authorization_headers.append(header.encode("ascii")) + authorization_headers.append(header) print("Authorization: %s" % header, file=sys.stderr) dest = "matrix://%s%s" % (destination, path) @@ -139,7 +140,10 @@ def request( s = requests.Session() s.mount("matrix://", MatrixConnectionAdapter()) - headers = {"Host": destination, "Authorization": authorization_headers[0]} + headers: Dict[str, str] = { + "Host": destination, + "Authorization": authorization_headers[0], + } if method == "POST": headers["Content-Type"] = "application/json" @@ -154,7 +158,7 @@ def request( ) -def main(): +def main() -> None: parser = argparse.ArgumentParser( description="Signs and sends a federation request to a matrix homeserver" ) @@ -212,6 +216,7 @@ def main(): if not args.server_name or not args.signing_key: read_args_from_config(args) + assert isinstance(args.signing_key, str) algorithm, version, key_base64 = args.signing_key.split() key = signedjson.key.decode_signing_key_base64(algorithm, version, key_base64) @@ -233,7 +238,7 @@ def main(): print("") -def read_args_from_config(args): +def read_args_from_config(args: argparse.Namespace) -> None: with open(args.config, "r") as fh: config = yaml.safe_load(fh) @@ -250,7 +255,7 @@ def read_args_from_config(args): class MatrixConnectionAdapter(HTTPAdapter): @staticmethod - def lookup(s, skip_well_known=False): + def lookup(s: str, skip_well_known: bool = False) -> Tuple[str, int]: if s[-1] == "]": # ipv6 literal (with no port) return s, 8448 @@ -276,7 +281,7 @@ class MatrixConnectionAdapter(HTTPAdapter): return s, 8448 @staticmethod - def get_well_known(server_name): + def get_well_known(server_name: str) -> Optional[str]: uri = "https://%s/.well-known/matrix/server" % (server_name,) print("fetching %s" % (uri,), file=sys.stderr) @@ -299,7 +304,9 @@ class MatrixConnectionAdapter(HTTPAdapter): print("Invalid response from %s: %s" % (uri, e), file=sys.stderr) return None - def get_connection(self, url, proxies=None): + def get_connection( + self, url: str, proxies: Optional[Dict[str, str]] = None + ) -> HTTPConnectionPool: parsed = urlparse.urlparse(url) (host, port) = self.lookup(parsed.netloc) diff --git a/scripts-dev/mypy_synapse_plugin.py b/scripts-dev/mypy_synapse_plugin.py index 1217e14874..c775865212 100644 --- a/scripts-dev/mypy_synapse_plugin.py +++ b/scripts-dev/mypy_synapse_plugin.py @@ -16,7 +16,7 @@ can crop up, e.g the cache descriptors. """ -from typing import Callable, Optional +from typing import Callable, Optional, Type from mypy.nodes import ARG_NAMED_OPT from mypy.plugin import MethodSigContext, Plugin @@ -94,7 +94,7 @@ def cached_function_method_signature(ctx: MethodSigContext) -> CallableType: return signature -def plugin(version: str): +def plugin(version: str) -> Type[SynapsePlugin]: # This is the entry point of the plugin, and let's us deal with the fact # that the mypy plugin interface is *not* stable by looking at the version # string. diff --git a/scripts-dev/release.py b/scripts-dev/release.py index 9d7c7c445f..14f3f3a45d 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -25,7 +25,7 @@ import sys import urllib.request from os import path from tempfile import TemporaryDirectory -from typing import List, Optional +from typing import Any, List, Optional, cast import attr import click @@ -36,7 +36,9 @@ from github import Github from packaging import version -def run_until_successful(command, *args, **kwargs): +def run_until_successful( + command: str, *args: Any, **kwargs: Any +) -> subprocess.CompletedProcess: while True: completed_process = subprocess.run(command, *args, **kwargs) exit_code = completed_process.returncode @@ -50,7 +52,7 @@ def run_until_successful(command, *args, **kwargs): @click.group() -def cli(): +def cli() -> None: """An interactive script to walk through the parts of creating a release. Requires the dev dependencies be installed, which can be done via: @@ -81,7 +83,7 @@ def cli(): @cli.command() -def prepare(): +def prepare() -> None: """Do the initial stages of creating a release, including creating release branch, updating changelog and pushing to GitHub. """ @@ -161,7 +163,9 @@ def prepare(): click.get_current_context().abort() # Switch to the release branch. - parsed_new_version: version.Version = version.parse(new_version) + # Cast safety: parse() won't return a version.LegacyVersion from our + # version string format. + parsed_new_version = cast(version.Version, version.parse(new_version)) # We assume for debian changelogs that we only do RCs or full releases. assert not parsed_new_version.is_devrelease @@ -176,7 +180,6 @@ def prepare(): # If the release branch only exists on the remote we check it out # locally. repo.git.checkout(release_branch_name) - release_branch = repo.active_branch else: # If a branch doesn't exist we create one. We ask which one branch it # should be based off, defaulting to sensible values depending on the @@ -198,13 +201,15 @@ def prepare(): click.get_current_context().abort() # Check out the base branch and ensure it's up to date - repo.head.reference = base_branch + repo.head.set_reference(base_branch, "check out the base branch") repo.head.reset(index=True, working_tree=True) if not base_branch.is_remote(): update_branch(repo) # Create the new release branch - release_branch = repo.create_head(release_branch_name, commit=base_branch) + # Type ignore will no longer be needed after GitPython 3.1.28. + # See https://github.com/gitpython-developers/GitPython/pull/1419 + repo.create_head(release_branch_name, commit=base_branch) # type: ignore[arg-type] # Switch to the release branch and ensure it's up to date. repo.git.checkout(release_branch_name) @@ -265,7 +270,7 @@ def prepare(): @cli.command() @click.option("--gh-token", envvar=["GH_TOKEN", "GITHUB_TOKEN"]) -def tag(gh_token: Optional[str]): +def tag(gh_token: Optional[str]) -> None: """Tags the release and generates a draft GitHub release""" # Make sure we're in a git repo. @@ -293,7 +298,12 @@ def tag(gh_token: Optional[str]): click.echo_via_pager(changes) if click.confirm("Edit text?", default=False): - changes = click.edit(changes, require_save=False) + edited_changes = click.edit(changes, require_save=False) + # This assert is for mypy's benefit. click's docs are a little unclear, but + # when `require_save=False`, not saving the temp file in the editor returns + # the original string. + assert edited_changes is not None + changes = edited_changes repo.create_tag(tag_name, message=changes, sign=True) @@ -347,7 +357,7 @@ def tag(gh_token: Optional[str]): @cli.command() @click.option("--gh-token", envvar=["GH_TOKEN", "GITHUB_TOKEN"], required=True) -def publish(gh_token: str): +def publish(gh_token: str) -> None: """Publish release.""" # Make sure we're in a git repo. @@ -390,7 +400,7 @@ def publish(gh_token: str): @cli.command() -def upload(): +def upload() -> None: """Upload release to pypi.""" current_version = get_package_version() @@ -418,7 +428,7 @@ def upload(): @cli.command() -def announce(): +def announce() -> None: """Generate markdown to announce the release.""" current_version = get_package_version() @@ -461,18 +471,19 @@ def get_package_version() -> version.Version: def find_ref(repo: git.Repo, ref_name: str) -> Optional[git.HEAD]: """Find the branch/ref, looking first locally then in the remote.""" - if ref_name in repo.refs: - return repo.refs[ref_name] + if ref_name in repo.references: + return repo.references[ref_name] elif ref_name in repo.remote().refs: return repo.remote().refs[ref_name] else: return None -def update_branch(repo: git.Repo): +def update_branch(repo: git.Repo) -> None: """Ensure branch is up to date if it has a remote""" - if repo.active_branch.tracking_branch(): - repo.git.merge(repo.active_branch.tracking_branch().name) + tracking_branch = repo.active_branch.tracking_branch() + if tracking_branch: + repo.git.merge(tracking_branch.name) def get_changes_for_version(wanted_version: version.Version) -> str: @@ -536,7 +547,9 @@ def get_changes_for_version(wanted_version: version.Version) -> str: return "\n".join(version_changelog) -def generate_and_write_changelog(current_version: version.Version, new_version: str): +def generate_and_write_changelog( + current_version: version.Version, new_version: str +) -> None: # We do this by getting a draft so that we can edit it before writing to the # changelog. result = run_until_successful( @@ -558,8 +571,8 @@ def generate_and_write_changelog(current_version: version.Version, new_version: f.write(existing_content) # Remove all the news fragments - for f in glob.iglob("changelog.d/*.*"): - os.remove(f) + for filename in glob.iglob("changelog.d/*.*"): + os.remove(filename) if __name__ == "__main__": diff --git a/scripts-dev/sign_json.py b/scripts-dev/sign_json.py index 9459543106..bb217799fb 100755 --- a/scripts-dev/sign_json.py +++ b/scripts-dev/sign_json.py @@ -27,7 +27,7 @@ from synapse.crypto.event_signing import add_hashes_and_signatures from synapse.util import json_encoder -def main(): +def main() -> None: parser = argparse.ArgumentParser( description="""Adds a signature to a JSON object.