From b716839b212df5f80d79a7a59d2b798ea3b99219 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 22 Apr 2022 14:15:38 +0100 Subject: [PATCH 1/7] disallow-untyped-defs in sortedcontainers stubs I've mentioned my changes on the PR these stubs come from: https://github.com/grantjenks/python-sortedcontainers/pull/107 --- stubs/sortedcontainers/sorteddict.pyi | 2 +- stubs/sortedcontainers/sortedlist.pyi | 6 +++--- stubs/sortedcontainers/sortedset.pyi | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/stubs/sortedcontainers/sorteddict.pyi b/stubs/sortedcontainers/sorteddict.pyi index 344d55cce118..e18d617281ab 100644 --- a/stubs/sortedcontainers/sorteddict.pyi +++ b/stubs/sortedcontainers/sorteddict.pyi @@ -103,7 +103,7 @@ class SortedDict(Dict[_KT, _VT]): self, start: Optional[int] = ..., stop: Optional[int] = ..., - reverse=bool, + reverse: bool = ..., ) -> Iterator[_KT]: ... def bisect_left(self, value: _KT) -> int: ... def bisect_right(self, value: _KT) -> int: ... diff --git a/stubs/sortedcontainers/sortedlist.pyi b/stubs/sortedcontainers/sortedlist.pyi index f80a3a72ce04..403897e3919e 100644 --- a/stubs/sortedcontainers/sortedlist.pyi +++ b/stubs/sortedcontainers/sortedlist.pyi @@ -81,7 +81,7 @@ class SortedList(MutableSequence[_T]): self, start: Optional[int] = ..., stop: Optional[int] = ..., - reverse=bool, + reverse: bool = ..., ) -> Iterator[_T]: ... def _islice( self, @@ -153,14 +153,14 @@ class SortedKeyList(SortedList[_T]): maximum: Optional[int] = ..., inclusive: Tuple[bool, bool] = ..., reverse: bool = ..., - ): ... + ) -> Iterator[_T]: ... def irange_key( self, min_key: Optional[Any] = ..., max_key: Optional[Any] = ..., inclusive: Tuple[bool, bool] = ..., reserve: bool = ..., - ): ... + ) -> Iterator[_T]: ... def bisect_left(self, value: _T) -> int: ... def bisect_right(self, value: _T) -> int: ... def bisect(self, value: _T) -> int: ... diff --git a/stubs/sortedcontainers/sortedset.pyi b/stubs/sortedcontainers/sortedset.pyi index f9c290838678..43c860f4221e 100644 --- a/stubs/sortedcontainers/sortedset.pyi +++ b/stubs/sortedcontainers/sortedset.pyi @@ -103,7 +103,7 @@ class SortedSet(MutableSet[_T], Sequence[_T]): self, start: Optional[int] = ..., stop: Optional[int] = ..., - reverse=bool, + reverse: bool = ..., ) -> Iterator[_T]: ... def irange( self, From 2395aad92bdc066cc7cb7e6b52f7d409836b405b Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 22 Apr 2022 14:31:38 +0100 Subject: [PATCH 2/7] disallow-untyped-defs in txredisapi stubs --- stubs/txredisapi.pyi | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/stubs/txredisapi.pyi b/stubs/txredisapi.pyi index 2d8ca018fbfc..d79ef2354c12 100644 --- a/stubs/txredisapi.pyi +++ b/stubs/txredisapi.pyi @@ -18,6 +18,8 @@ from typing import Any, List, Optional, Type, Union from twisted.internet import protocol from twisted.internet.defer import Deferred +from twisted.internet.interfaces import IAddress +from twisted.python.failure import Failure class RedisProtocol(protocol.Protocol): def publish(self, channel: str, message: bytes) -> "Deferred[None]": ... @@ -34,11 +36,11 @@ class RedisProtocol(protocol.Protocol): def get(self, key: str) -> "Deferred[Any]": ... class SubscriberProtocol(RedisProtocol): - def __init__(self, *args, **kwargs): ... + def __init__(self, *args: object, **kwargs: object): ... password: Optional[str] - def subscribe(self, channels: Union[str, List[str]]): ... - def connectionMade(self): ... - def connectionLost(self, reason): ... + def subscribe(self, channels: Union[str, List[str]]) -> "Deferred[None]": ... + def connectionMade(self) -> None: ... + def connectionLost(self, reason: Failure = ...) -> None: ... def lazyConnection( host: str = ..., @@ -74,7 +76,7 @@ class RedisFactory(protocol.ReconnectingClientFactory): replyTimeout: Optional[int] = None, convertNumbers: Optional[int] = True, ): ... - def buildProtocol(self, addr) -> RedisProtocol: ... + def buildProtocol(self, addr: IAddress) -> RedisProtocol: ... class SubscriberFactory(RedisFactory): def __init__(self) -> None: ... From c1e61718b3a594257364e17d4bf2391b1ceb04cb Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 22 Apr 2022 15:03:06 +0100 Subject: [PATCH 3/7] disallow-untyped-defs in docker start script --- docker/start.py | 42 ++++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/docker/start.py b/docker/start.py index ac62bbc8baf9..d67854b9ed92 100755 --- a/docker/start.py +++ b/docker/start.py @@ -6,27 +6,28 @@ import platform import subprocess import sys +from typing import Any, Dict, List, Mapping, MutableMapping, NoReturn, Optional import jinja2 # Utility functions -def log(txt): +def log(txt: str) -> None: print(txt, file=sys.stderr) -def error(txt): +def error(txt: str) -> NoReturn: log(txt) sys.exit(2) -def convert(src, dst, environ): +def convert(src: str, dst: str, environ: Mapping[str, object]) -> None: """Generate a file from a template Args: - src (str): path to input file - dst (str): path to file to write - environ (dict): environment dictionary, for replacement mappings. + src: path to input file + dst: path to file to write + environ: environment dictionary, for replacement mappings. """ with open(src) as infile: template = infile.read() @@ -35,25 +36,30 @@ def convert(src, dst, environ): outfile.write(rendered) -def generate_config_from_template(config_dir, config_path, environ, ownership): +def generate_config_from_template( + config_dir: str, + config_path: str, + os_environ: Mapping[str, str], + ownership: Optional[str], +) -> None: """Generate a homeserver.yaml from environment variables Args: - config_dir (str): where to put generated config files - config_path (str): where to put the main config file - environ (dict): environment dictionary - ownership (str|None): ":" string which will be used to set + config_dir: where to put generated config files + config_path: where to put the main config file + os_environ: environment mapping + ownership: ":" string which will be used to set ownership of the generated configs. If None, ownership will not change. """ for v in ("SYNAPSE_SERVER_NAME", "SYNAPSE_REPORT_STATS"): - if v not in environ: + if v not in os_environ: error( "Environment variable '%s' is mandatory when generating a config file." % (v,) ) # populate some params from data files (if they exist, else create new ones) - environ = environ.copy() + environ: Dict[str, Any] = dict(os_environ) secrets = { "registration": "SYNAPSE_REGISTRATION_SHARED_SECRET", "macaroon": "SYNAPSE_MACAROON_SECRET_KEY", @@ -83,7 +89,7 @@ def generate_config_from_template(config_dir, config_path, environ, ownership): # Convert SYNAPSE_NO_TLS to boolean if exists if "SYNAPSE_NO_TLS" in environ: - tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"]) + tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"]) # type if tlsanswerstring in ("true", "on", "1", "yes"): environ["SYNAPSE_NO_TLS"] = True else: @@ -127,12 +133,12 @@ def generate_config_from_template(config_dir, config_path, environ, ownership): subprocess.check_output(args) -def run_generate_config(environ, ownership): +def run_generate_config(environ: Mapping[str, str], ownership: Optional[str]) -> None: """Run synapse with a --generate-config param to generate a template config file Args: - environ (dict): env var dict - ownership (str|None): "userid:groupid" arg for chmod. If None, ownership will not change. + environ: env vars from `os.enrivon`. + ownership: "userid:groupid" arg for chmod. If None, ownership will not change. Never returns. """ @@ -178,7 +184,7 @@ def run_generate_config(environ, ownership): os.execv(sys.executable, args) -def main(args, environ): +def main(args: List[str], environ: MutableMapping[str, str]) -> None: mode = args[1] if len(args) > 1 else "run" # if we were given an explicit user to switch to, do so From 242a4b468a3c39f24e189583e79c55f24a50b998 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 22 Apr 2022 15:16:23 +0100 Subject: [PATCH 4/7] disallow-untyped-defs in docker worker script --- docker/configure_workers_and_start.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/docker/configure_workers_and_start.py b/docker/configure_workers_and_start.py index 23cac18e8dbd..3bda6c300ba8 100755 --- a/docker/configure_workers_and_start.py +++ b/docker/configure_workers_and_start.py @@ -29,7 +29,7 @@ import os import subprocess import sys -from typing import Any, Dict, Mapping, Set +from typing import Any, Dict, List, Mapping, MutableMapping, NoReturn, Set import jinja2 import yaml @@ -201,7 +201,7 @@ # Utility functions -def log(txt: str): +def log(txt: str) -> None: """Log something to the stdout. Args: @@ -210,7 +210,7 @@ def log(txt: str): print(txt) -def error(txt: str): +def error(txt: str) -> NoReturn: """Log something and exit with an error code. Args: @@ -220,7 +220,7 @@ def error(txt: str): sys.exit(2) -def convert(src: str, dst: str, **template_vars): +def convert(src: str, dst: str, **template_vars: object) -> None: """Generate a file from a template Args: @@ -290,7 +290,7 @@ def add_sharding_to_shared_config( shared_config.setdefault("media_instance_running_background_jobs", worker_name) -def generate_base_homeserver_config(): +def generate_base_homeserver_config() -> None: """Starts Synapse and generates a basic homeserver config, which will later be modified for worker support. @@ -302,12 +302,14 @@ def generate_base_homeserver_config(): subprocess.check_output(["/usr/local/bin/python", "/start.py", "migrate_config"]) -def generate_worker_files(environ, config_path: str, data_dir: str): +def generate_worker_files( + environ: Mapping[str, str], config_path: str, data_dir: str +) -> None: """Read the desired list of workers from environment variables and generate shared homeserver, nginx and supervisord configs. Args: - environ: _Environ[str] + environ: os.environ instance. config_path: The location of the generated Synapse main worker config file. data_dir: The location of the synapse data directory. Where log and user-facing config files live. @@ -369,13 +371,13 @@ def generate_worker_files(environ, config_path: str, data_dir: str): nginx_locations = {} # Read the desired worker configuration from the environment - worker_types = environ.get("SYNAPSE_WORKER_TYPES") - if worker_types is None: + worker_types_env = environ.get("SYNAPSE_WORKER_TYPES") + if worker_types_env is None: # No workers, just the main process worker_types = [] else: # Split type names by comma - worker_types = worker_types.split(",") + worker_types = worker_types_env.split(",") # Create the worker configuration directory if it doesn't already exist os.makedirs("/conf/workers", exist_ok=True) @@ -547,7 +549,7 @@ def generate_worker_log_config( return log_config_filepath -def main(args, environ): +def main(args: List[str], environ: MutableMapping[str, str]) -> None: config_dir = environ.get("SYNAPSE_CONFIG_DIR", "/data") config_path = environ.get("SYNAPSE_CONFIG_PATH", config_dir + "/homeserver.yaml") data_dir = environ.get("SYNAPSE_DATA_DIR", "/data") From 018fac7bfe6620890f54e999b1646e8ec1fb633f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 22 Apr 2022 18:20:14 +0100 Subject: [PATCH 5/7] Changelog --- changelog.d/12528.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12528.misc diff --git a/changelog.d/12528.misc b/changelog.d/12528.misc new file mode 100644 index 000000000000..f64b5d24b005 --- /dev/null +++ b/changelog.d/12528.misc @@ -0,0 +1 @@ +Add type hints so `docker` and `stubs` directories pass `mypy --disallow-untyped-defs`. From 283d9614018a154f0570a43d2b2dde4dfac52b4c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 25 Apr 2022 13:02:34 +0100 Subject: [PATCH 6/7] Remove unintentional `# type` comment --- docker/start.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/start.py b/docker/start.py index d67854b9ed92..4ac8f034777f 100755 --- a/docker/start.py +++ b/docker/start.py @@ -89,7 +89,7 @@ def generate_config_from_template( # Convert SYNAPSE_NO_TLS to boolean if exists if "SYNAPSE_NO_TLS" in environ: - tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"]) # type + tlsanswerstring = str.lower(environ["SYNAPSE_NO_TLS"]) if tlsanswerstring in ("true", "on", "1", "yes"): environ["SYNAPSE_NO_TLS"] = True else: From b4198640f0fad24af70a43c982ecfe7ed6dad460 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 25 Apr 2022 13:08:15 +0100 Subject: [PATCH 7/7] Correct `connectionLost` stub. It looks to me like twisted's `Protocol` isn't compliant with `IProtocol`! --- stubs/txredisapi.pyi | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stubs/txredisapi.pyi b/stubs/txredisapi.pyi index d79ef2354c12..695a2307c2c5 100644 --- a/stubs/txredisapi.pyi +++ b/stubs/txredisapi.pyi @@ -40,7 +40,10 @@ class SubscriberProtocol(RedisProtocol): password: Optional[str] def subscribe(self, channels: Union[str, List[str]]) -> "Deferred[None]": ... def connectionMade(self) -> None: ... - def connectionLost(self, reason: Failure = ...) -> None: ... + # type-ignore: twisted.internet.protocol.Protocol provides a default argument for + # `reason`. txredisapi's LineReceiver Protocol doesn't. But that's fine: it's what's + # actually specified in twisted.internet.interfaces.IProtocol. + def connectionLost(self, reason: Failure) -> None: ... # type: ignore[override] def lazyConnection( host: str = ...,