From 55cc0420d700f35ab863feaac8aea48d9f03eaf8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:03:42 -0500 Subject: [PATCH 01/13] Add types to gai_resolver. --- synapse/util/gai_resolver.py | 73 ++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/synapse/util/gai_resolver.py b/synapse/util/gai_resolver.py index a447ce4e5595..9e7480419b79 100644 --- a/synapse/util/gai_resolver.py +++ b/synapse/util/gai_resolver.py @@ -3,23 +3,48 @@ # We copy it here as we need to instantiate `GAIResolver` manually, but it is a # private class. - from socket import ( AF_INET, AF_INET6, AF_UNSPEC, SOCK_DGRAM, SOCK_STREAM, + AddressFamily, + SocketKind, gaierror, getaddrinfo, ) +from typing import ( + TYPE_CHECKING, + Callable, + List, + NoReturn, + Optional, + Sequence, + Tuple, + Union, +) from zope.interface import implementer from twisted.internet.address import IPv4Address, IPv6Address -from twisted.internet.interfaces import IHostnameResolver, IHostResolution +from twisted.internet.interfaces import ( + IAddress, + IHostnameResolver, + IHostResolution, + IReactorThreads, + IResolutionReceiver, +) from twisted.internet.threads import deferToThreadPool +if TYPE_CHECKING: + from twisted.python.runtime import platform + + if platform.supportsThreads(): + from twisted.python.threadpool import ThreadPool + else: + ThreadPool = object # type: ignore[misc, assignment] + @implementer(IHostResolution) class HostResolution: @@ -27,13 +52,13 @@ class HostResolution: The in-progress resolution of a given hostname. """ - def __init__(self, name): + def __init__(self, name: str): """ Create a L{HostResolution} with the given name. """ self.name = name - def cancel(self): + def cancel(self) -> NoReturn: # IHostResolution.cancel raise NotImplementedError() @@ -62,6 +87,17 @@ def cancel(self): } +_GETADDRINFO_RESULT = List[ + Tuple[ + AddressFamily, + SocketKind, + int, + str, + Union[Tuple[str, int], Tuple[str, int, int, int]], + ] +] + + @implementer(IHostnameResolver) class GAIResolver: """ @@ -69,7 +105,12 @@ class GAIResolver: L{getaddrinfo} in a thread. """ - def __init__(self, reactor, getThreadPool=None, getaddrinfo=getaddrinfo): + def __init__( + self, + reactor: IReactorThreads, + getThreadPool: Optional[Callable[[], "ThreadPool"]] = None, + getaddrinfo: Callable[[str, int, int, int], _GETADDRINFO_RESULT] = getaddrinfo, + ): """ Create a L{GAIResolver}. @param reactor: the reactor to schedule result-delivery on @@ -91,12 +132,12 @@ def __init__(self, reactor, getThreadPool=None, getaddrinfo=getaddrinfo): def resolveHostName( self, - resolutionReceiver, - hostName, - portNumber=0, - addressTypes=None, - transportSemantics="TCP", - ): + resolutionReceiver: IResolutionReceiver, + hostName: str, + portNumber: int = 0, + addressTypes: Optional[Sequence[IAddress]] = None, + transportSemantics: str = "TCP", + ) -> IResolutionReceiver: """ See L{IHostnameResolver.resolveHostName} @param resolutionReceiver: see interface @@ -108,11 +149,11 @@ def resolveHostName( """ pool = self._getThreadPool() addressFamily = _typesToAF[ - _any if addressTypes is None else frozenset(addressTypes) + _any if addressTypes is None else frozenset(addressTypes) # type: ignore[arg-type] ] socketType = _transportToSocket[transportSemantics] - def get(): + def get() -> _GETADDRINFO_RESULT: try: return self._getaddrinfo( hostName, portNumber, addressFamily, socketType @@ -125,7 +166,7 @@ def get(): resolutionReceiver.resolutionBegan(resolution) @d.addCallback - def deliverResults(result): + def deliverResults(result: _GETADDRINFO_RESULT) -> None: for family, socktype, _proto, _cannoname, sockaddr in result: addrType = _afToType[family] resolutionReceiver.addressResolved( @@ -133,4 +174,6 @@ def deliverResults(result): ) resolutionReceiver.resolutionComplete() - return resolution + # IHostnameResolver declares that resolverHostName returns IResolutionReceiver, + # but the implementations return IHostResolution. + return resolution # type: ignore[return-value] From 24bf70f8cddd91b2ddbcdcc2837b91d7fd9cc5c6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:17:05 -0500 Subject: [PATCH 02/13] Add missing type hints to async_helpers. --- synapse/util/async_helpers.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index 561b962e146a..e7eeed13508c 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -27,6 +27,7 @@ Generic, Hashable, Iterable, + Iterator, Optional, Set, TypeVar, @@ -40,7 +41,6 @@ from twisted.internet import defer from twisted.internet.defer import CancelledError from twisted.internet.interfaces import IReactorTime -from twisted.python import failure from twisted.python.failure import Failure from synapse.logging.context import ( @@ -78,7 +78,7 @@ def __init__(self, deferred: "defer.Deferred[_T]", consumeErrors: bool = False): object.__setattr__(self, "_result", None) object.__setattr__(self, "_observers", []) - def callback(r): + def callback(r: _T) -> _T: object.__setattr__(self, "_result", (True, r)) # once we have set _result, no more entries will be added to _observers, @@ -98,7 +98,7 @@ def callback(r): ) return r - def errback(f): + def errback(f: Failure) -> Optional[Failure]: object.__setattr__(self, "_result", (False, f)) # once we have set _result, no more entries will be added to _observers, @@ -109,7 +109,7 @@ def errback(f): for observer in observers: # This is a little bit of magic to correctly propagate stack # traces when we `await` on one of the observer deferreds. - f.value.__failure__ = f + f.value.__failure__ = f # type: ignore[union-attr] try: observer.errback(f) except Exception as e: @@ -314,7 +314,7 @@ def queue(self, key: Hashable) -> defer.Deferred: # will release the lock. @contextmanager - def _ctx_manager(_): + def _ctx_manager(_: None) -> Iterator[None]: try: yield finally: @@ -355,7 +355,7 @@ def _await_lock(self, key: Hashable) -> defer.Deferred: new_defer = make_deferred_yieldable(defer.Deferred()) entry.deferreds[new_defer] = 1 - def cb(_r): + def cb(_r: None) -> defer.Deferred: logger.debug("Acquired linearizer lock %r for key %r", self.name, key) entry.count += 1 @@ -371,7 +371,7 @@ def cb(_r): # code must be synchronous, so this is the only sensible place.) return self._clock.sleep(0) - def eb(e): + def eb(e: Failure) -> Failure: logger.info("defer %r got err %r", new_defer, e) if isinstance(e, CancelledError): logger.debug( @@ -435,7 +435,7 @@ async def read(self, key: str) -> ContextManager: await make_deferred_yieldable(curr_writer) @contextmanager - def _ctx_manager(): + def _ctx_manager() -> Iterator[None]: try: yield finally: @@ -464,7 +464,7 @@ async def write(self, key: str) -> ContextManager: await make_deferred_yieldable(defer.gatherResults(to_wait_on)) @contextmanager - def _ctx_manager(): + def _ctx_manager() -> Iterator[None]: try: yield finally: @@ -524,7 +524,7 @@ def time_it_out() -> None: delayed_call = reactor.callLater(timeout, time_it_out) - def convert_cancelled(value: failure.Failure): + def convert_cancelled(value: Failure) -> Failure: # if the original deferred was cancelled, and our timeout has fired, then # the reason it was cancelled was due to our timeout. Turn the CancelledError # into a TimeoutError. @@ -534,7 +534,7 @@ def convert_cancelled(value: failure.Failure): deferred.addErrback(convert_cancelled) - def cancel_timeout(result): + def cancel_timeout(result: _T) -> _T: # stop the pending call to cancel the deferred if it's been fired if delayed_call.active(): delayed_call.cancel() @@ -542,11 +542,11 @@ def cancel_timeout(result): deferred.addBoth(cancel_timeout) - def success_cb(val): + def success_cb(val: Any) -> None: if not new_d.called: new_d.callback(val) - def failure_cb(val): + def failure_cb(val: Failure) -> None: if not new_d.called: new_d.errback(val) @@ -557,13 +557,13 @@ def failure_cb(val): # This class can't be generic because it uses slots with attrs. # See: https://github.com/python-attrs/attrs/issues/313 -@attr.s(slots=True, frozen=True) +@attr.s(slots=True, frozen=True, auto_attribs=True) class DoneAwaitable: # should be: Generic[R] """Simple awaitable that returns the provided value.""" - value = attr.ib(type=Any) # should be: R + value: Any # should be: R - def __await__(self): + def __await__(self) -> Any: return self def __iter__(self) -> "DoneAwaitable": From f3529a122527fe165d5e5a84a12c93d36387ae54 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:25:20 -0500 Subject: [PATCH 03/13] Add type hints for distributor. --- synapse/util/distributor.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/util/distributor.py b/synapse/util/distributor.py index 31097d64398e..91837655f8fa 100644 --- a/synapse/util/distributor.py +++ b/synapse/util/distributor.py @@ -18,12 +18,13 @@ from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.types import UserID from synapse.util.async_helpers import maybe_awaitable logger = logging.getLogger(__name__) -def user_left_room(distributor, user, room_id): +def user_left_room(distributor: "Distributor", user: UserID, room_id: str) -> None: distributor.fire("user_left_room", user=user, room_id=room_id) @@ -63,7 +64,7 @@ def observe(self, name: str, observer: Callable) -> None: self.pre_registration[name] = [] self.pre_registration[name].append(observer) - def fire(self, name: str, *args, **kwargs) -> None: + def fire(self, name: str, *args: Any, **kwargs: Any) -> None: """Dispatches the given signal to the registered observers. Runs the observers as a background process. Does not return a deferred. @@ -95,7 +96,7 @@ def observe(self, observer: Callable) -> None: Each observer callable may return a Deferred.""" self.observers.append(observer) - def fire(self, *args, **kwargs) -> "defer.Deferred[List[Any]]": + def fire(self, *args: Any, **kwargs: Any) -> "defer.Deferred[List[Any]]": """Invokes every callable in the observer list, passing in the args and kwargs. Exceptions thrown by observers are logged but ignored. It is not an error to fire a signal with no observers. @@ -103,7 +104,7 @@ def fire(self, *args, **kwargs) -> "defer.Deferred[List[Any]]": Returns a Deferred that will complete when all the observers have completed.""" - async def do(observer): + async def do(observer: Callable[..., Any]) -> Any: try: return await maybe_awaitable(observer(*args, **kwargs)) except Exception as e: @@ -120,5 +121,5 @@ async def do(observer): defer.gatherResults(deferreds, consumeErrors=True) ) - def __repr__(self): + def __repr__(self) -> str: return "" % (self.name,) From 9e76291a32f3bd5a979a6474ce451f6b464c2fc9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:26:28 -0500 Subject: [PATCH 04/13] Add a missing type hint to DeferredCache. --- synapse/util/caches/deferred_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/util/caches/deferred_cache.py b/synapse/util/caches/deferred_cache.py index da502aec114d..3c4cc093aff3 100644 --- a/synapse/util/caches/deferred_cache.py +++ b/synapse/util/caches/deferred_cache.py @@ -289,7 +289,7 @@ def prefill( callbacks = [callback] if callback else [] self.cache.set(key, value, callbacks=callbacks) - def invalidate(self, key) -> None: + def invalidate(self, key: KT) -> None: """Delete a key, or tree of entries If the cache is backed by a regular dict, then "key" must be of From 5cba073c848b193e5ff93e6aaea18ca3299f863f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:30:41 -0500 Subject: [PATCH 05/13] Add a missing type hint to ExpiringCache. --- synapse/util/caches/expiringcache.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/util/caches/expiringcache.py b/synapse/util/caches/expiringcache.py index c3f72aa06de6..6a7e5345766d 100644 --- a/synapse/util/caches/expiringcache.py +++ b/synapse/util/caches/expiringcache.py @@ -19,6 +19,8 @@ import attr from typing_extensions import Literal +from twisted.internet import defer + from synapse.config import cache as cache_config from synapse.metrics.background_process_metrics import run_as_background_process from synapse.util import Clock @@ -81,7 +83,7 @@ def __init__( # Don't bother starting the loop if things never expire return - def f(): + def f() -> "defer.Deferred[None]": return run_as_background_process( "prune_cache_%s" % self._cache_name, self._prune_cache ) @@ -210,7 +212,7 @@ def set_cache_factor(self, factor: float) -> bool: return False -@attr.s(slots=True) +@attr.s(slots=True, auto_attribs=True) class _CacheEntry: - time = attr.ib(type=int) - value = attr.ib() + time: int + value: Any From 609d9267d070926ba7d92dd92bd78ed604a336d3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 10:36:45 -0500 Subject: [PATCH 06/13] Add a missing type hint to metrics. --- synapse/util/metrics.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/synapse/util/metrics.py b/synapse/util/metrics.py index 1e784b3f1f8d..949b5e23932c 100644 --- a/synapse/util/metrics.py +++ b/synapse/util/metrics.py @@ -64,6 +64,12 @@ sub_metrics=["real_time_max", "real_time_sum"], ) +# This is dynamically created in InFlightGauge.__init__. +class _InFlightMetric(Protocol): + real_time_max: float + real_time_sum: float + + T = TypeVar("T", bound=Callable[..., Any]) @@ -180,7 +186,7 @@ def get_resource_usage(self) -> ContextResourceUsage: """ return self._logging_context.get_resource_usage() - def _update_in_flight(self, metrics) -> None: + def _update_in_flight(self, metrics: _InFlightMetric) -> None: """Gets called when processing in flight metrics""" assert self.start is not None duration = self.clock.time() - self.start From c6ed2a8fc146ca54a3a839d91e805b76181721c4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 11:29:42 -0500 Subject: [PATCH 07/13] Add a missing type hint to cache descriptors. --- synapse/util/caches/descriptors.py | 67 +++++++++++++++++++----------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/synapse/util/caches/descriptors.py b/synapse/util/caches/descriptors.py index b9dcca17f1a6..a3c0a6ca56ed 100644 --- a/synapse/util/caches/descriptors.py +++ b/synapse/util/caches/descriptors.py @@ -19,12 +19,15 @@ from typing import ( Any, Callable, + Dict, Generic, Iterable, + Hashable, Mapping, Optional, Sequence, Tuple, + Type, TypeVar, Union, cast, @@ -32,6 +35,7 @@ from weakref import WeakValueDictionary from twisted.internet import defer +from twisted.python.failure import Failure from synapse.logging.context import make_deferred_yieldable, preserve_fn from synapse.util import unwrapFirstError @@ -60,7 +64,12 @@ class _CachedFunction(Generic[F]): class _CacheDescriptorBase: - def __init__(self, orig: Callable[..., Any], num_args, cache_context=False): + def __init__( + self, + orig: Callable[..., Any], + num_args: Optional[int], + cache_context: bool = False, + ): self.orig = orig arg_spec = inspect.getfullargspec(orig) @@ -172,14 +181,14 @@ class _Sentinel(enum.Enum): def __init__( self, - orig, + orig: Callable[..., Any], max_entries: int = 1000, cache_context: bool = False, ): super().__init__(orig, num_args=None, cache_context=cache_context) self.max_entries = max_entries - def __get__(self, obj, owner): + def __get__(self, obj: Optional[Any], owner: Optional[Type]) -> Callable[..., Any]: cache: LruCache[CacheKey, Any] = LruCache( cache_name=self.orig.__name__, max_size=self.max_entries, @@ -189,7 +198,7 @@ def __get__(self, obj, owner): sentinel = LruCacheDescriptor._Sentinel.sentinel @functools.wraps(self.orig) - def _wrapped(*args, **kwargs): + def _wrapped(*args: Any, **kwargs: Any) -> Any: invalidate_callback = kwargs.pop("on_invalidate", None) callbacks = (invalidate_callback,) if invalidate_callback else () @@ -245,19 +254,19 @@ def foo(self, key, cache_context): return r1 + r2 Args: - num_args (int): number of positional arguments (excluding ``self`` and + num_args: number of positional arguments (excluding ``self`` and ``cache_context``) to use as cache keys. Defaults to all named args of the function. """ def __init__( self, - orig, - max_entries=1000, - num_args=None, - tree=False, - cache_context=False, - iterable=False, + orig: Callable[..., Any], + max_entries: int = 1000, + num_args: Optional[int] = None, + tree: bool = False, + cache_context: bool = False, + iterable: bool = False, prune_unread_entries: bool = True, ): super().__init__(orig, num_args=num_args, cache_context=cache_context) @@ -272,7 +281,7 @@ def __init__( self.iterable = iterable self.prune_unread_entries = prune_unread_entries - def __get__(self, obj, owner): + def __get__(self, obj: Optional[Any], owner: Optional[Type]) -> Callable[..., Any]: cache: DeferredCache[CacheKey, Any] = DeferredCache( name=self.orig.__name__, max_entries=self.max_entries, @@ -284,7 +293,7 @@ def __get__(self, obj, owner): get_cache_key = self.cache_key_builder @functools.wraps(self.orig) - def _wrapped(*args, **kwargs): + def _wrapped(*args: Any, **kwargs: Any) -> Any: # If we're passed a cache_context then we'll want to call its invalidate() # whenever we are invalidated invalidate_callback = kwargs.pop("on_invalidate", None) @@ -335,13 +344,19 @@ class DeferredCacheListDescriptor(_CacheDescriptorBase): of results. """ - def __init__(self, orig, cached_method_name, list_name, num_args=None): + def __init__( + self, + orig: Callable[..., Any], + cached_method_name: str, + list_name: str, + num_args: Optional[int] = None, + ): """ Args: - orig (function) - cached_method_name (str): The name of the cached method. - list_name (str): Name of the argument which is the bulk lookup list - num_args (int): number of positional arguments (excluding ``self``, + orig + cached_method_name: The name of the cached method. + list_name: Name of the argument which is the bulk lookup list + num_args: number of positional arguments (excluding ``self``, but including list_name) to use as cache keys. Defaults to all named args of the function. """ @@ -360,13 +375,15 @@ def __init__(self, orig, cached_method_name, list_name, num_args=None): % (self.list_name, cached_method_name) ) - def __get__(self, obj, objtype=None): + def __get__( + self, obj: Optional[Any], objtype: Optional[Type] = None + ) -> Callable[..., Any]: cached_method = getattr(obj, self.cached_method_name) cache: DeferredCache[CacheKey, Any] = cached_method.cache num_args = cached_method.num_args @functools.wraps(self.orig) - def wrapped(*args, **kwargs): + def wrapped(*args: Any, **kwargs: Any) -> Any: # If we're passed a cache_context then we'll want to call its # invalidate() whenever we are invalidated invalidate_callback = kwargs.pop("on_invalidate", None) @@ -377,7 +394,7 @@ def wrapped(*args, **kwargs): results = {} - def update_results_dict(res, arg): + def update_results_dict(res: Any, arg: Hashable) -> None: results[arg] = res # list of deferreds to wait for @@ -389,13 +406,13 @@ def update_results_dict(res, arg): # otherwise a tuple is used. if num_args == 1: - def arg_to_cache_key(arg): + def arg_to_cache_key(arg: Hashable) -> Hashable: return arg else: keylist = list(keyargs) - def arg_to_cache_key(arg): + def arg_to_cache_key(arg: Hashable) -> Hashable: keylist[self.list_pos] = arg return tuple(keylist) @@ -421,7 +438,7 @@ def arg_to_cache_key(arg): key = arg_to_cache_key(arg) cache.set(key, deferred, callback=invalidate_callback) - def complete_all(res): + def complete_all(res: Dict[Hashable, Any]) -> None: # the wrapped function has completed. It returns a # a dict. We can now resolve the observable deferreds in # the cache and update our own result map. @@ -430,7 +447,7 @@ def complete_all(res): deferreds_map[e].callback(val) results[e] = val - def errback(f): + def errback(f: Failure) -> Failure: # the wrapped function has failed. Invalidate any cache # entries we're supposed to be populating, and fail # their deferreds. From 9ef6a81fee6a82da1e085e2db9c56095fff1736e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 11:39:55 -0500 Subject: [PATCH 08/13] Add remaining type hints, minus TreeCache. --- mypy.ini | 87 ++------------------------------- synapse/util/caches/__init__.py | 32 ++++++------ 2 files changed, 20 insertions(+), 99 deletions(-) diff --git a/mypy.ini b/mypy.ini index 56a62bb9b795..08a3bff7706f 100644 --- a/mypy.ini +++ b/mypy.ini @@ -181,92 +181,11 @@ disallow_untyped_defs = True [mypy-synapse.streams.*] disallow_untyped_defs = True -[mypy-synapse.util.batching_queue] +[mypy-synapse.util.*] disallow_untyped_defs = True -[mypy-synapse.util.caches.cached_call] -disallow_untyped_defs = True - -[mypy-synapse.util.caches.dictionary_cache] -disallow_untyped_defs = True - -[mypy-synapse.util.caches.lrucache] -disallow_untyped_defs = True - -[mypy-synapse.util.caches.response_cache] -disallow_untyped_defs = True - -[mypy-synapse.util.caches.stream_change_cache] -disallow_untyped_defs = True - -[mypy-synapse.util.caches.ttl_cache] -disallow_untyped_defs = True - -[mypy-synapse.util.daemonize] -disallow_untyped_defs = True - -[mypy-synapse.util.file_consumer] -disallow_untyped_defs = True - -[mypy-synapse.util.frozenutils] -disallow_untyped_defs = True - -[mypy-synapse.util.hash] -disallow_untyped_defs = True - -[mypy-synapse.util.httpresourcetree] -disallow_untyped_defs = True - -[mypy-synapse.util.iterutils] -disallow_untyped_defs = True - -[mypy-synapse.util.linked_list] -disallow_untyped_defs = True - -[mypy-synapse.util.logcontext] -disallow_untyped_defs = True - -[mypy-synapse.util.logformatter] -disallow_untyped_defs = True - -[mypy-synapse.util.macaroons] -disallow_untyped_defs = True - -[mypy-synapse.util.manhole] -disallow_untyped_defs = True - -[mypy-synapse.util.module_loader] -disallow_untyped_defs = True - -[mypy-synapse.util.msisdn] -disallow_untyped_defs = True - -[mypy-synapse.util.patch_inline_callbacks] -disallow_untyped_defs = True - -[mypy-synapse.util.ratelimitutils] -disallow_untyped_defs = True - -[mypy-synapse.util.retryutils] -disallow_untyped_defs = True - -[mypy-synapse.util.rlimit] -disallow_untyped_defs = True - -[mypy-synapse.util.stringutils] -disallow_untyped_defs = True - -[mypy-synapse.util.templates] -disallow_untyped_defs = True - -[mypy-synapse.util.threepids] -disallow_untyped_defs = True - -[mypy-synapse.util.wheel_timer] -disallow_untyped_defs = True - -[mypy-synapse.util.versionstring] -disallow_untyped_defs = True +[mypy-synapse.util.caches.treecache] +disallow_untyped_defs = False [mypy-tests] disallow_untyped_defs = True diff --git a/synapse/util/caches/__init__.py b/synapse/util/caches/__init__.py index df4d61e4b642..5265524ed179 100644 --- a/synapse/util/caches/__init__.py +++ b/synapse/util/caches/__init__.py @@ -17,7 +17,7 @@ import typing from enum import Enum, auto from sys import intern -from typing import Callable, Dict, Optional, Sized +from typing import Any, Callable, Dict, List, Optional, Sized, TypeVar import attr from prometheus_client.core import Gauge @@ -58,20 +58,20 @@ class EvictionReason(Enum): time = auto() -@attr.s(slots=True) +@attr.s(slots=True, auto_attribs=True) class CacheMetric: - _cache = attr.ib() - _cache_type = attr.ib(type=str) - _cache_name = attr.ib(type=str) - _collect_callback = attr.ib(type=Optional[Callable]) + _cache: Sized + _cache_type: str + _cache_name: str + _collect_callback: Optional[Callable] - hits = attr.ib(default=0) - misses = attr.ib(default=0) + hits: int = 0 + misses: int = 0 eviction_size_by_reason: typing.Counter[EvictionReason] = attr.ib( factory=collections.Counter ) - memory_usage = attr.ib(default=None) + memory_usage: Optional[int] = None def inc_hits(self) -> None: self.hits += 1 @@ -89,13 +89,14 @@ def inc_memory_usage(self, memory: int) -> None: self.memory_usage += memory def dec_memory_usage(self, memory: int) -> None: + assert self.memory_usage is not None self.memory_usage -= memory def clear_memory_usage(self) -> None: if self.memory_usage is not None: self.memory_usage = 0 - def describe(self): + def describe(self) -> List[str]: return [] def collect(self) -> None: @@ -118,8 +119,9 @@ def collect(self) -> None: self.eviction_size_by_reason[reason] ) cache_total.labels(self._cache_name).set(self.hits + self.misses) - if getattr(self._cache, "max_size", None): - cache_max_size.labels(self._cache_name).set(self._cache.max_size) + max_size = getattr(self._cache, "max_size", None) + if max_size: + cache_max_size.labels(self._cache_name).set(max_size) if TRACK_MEMORY_USAGE: # self.memory_usage can be None if nothing has been inserted @@ -193,7 +195,7 @@ def register_cache( } -def intern_string(string): +def intern_string(string: Optional[str]) -> Optional[str]: """Takes a (potentially) unicode string and interns it if it's ascii""" if string is None: return None @@ -204,7 +206,7 @@ def intern_string(string): return string -def intern_dict(dictionary): +def intern_dict(dictionary: Dict[str, Any]) -> Dict[str, Any]: """Takes a dictionary and interns well known keys and their values""" return { KNOWN_KEYS.get(key, key): _intern_known_values(key, value) @@ -212,7 +214,7 @@ def intern_dict(dictionary): } -def _intern_known_values(key, value): +def _intern_known_values(key: str, value: Any) -> Any: intern_keys = ("event_id", "room_id", "sender", "user_id", "type", "state_key") if key in intern_keys: From 0bb57520d17823f4a0da9cfc9bad6db6c7393d59 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 11:42:17 -0500 Subject: [PATCH 09/13] Newsfragment --- changelog.d/11328.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11328.misc diff --git a/changelog.d/11328.misc b/changelog.d/11328.misc new file mode 100644 index 000000000000..7c377813e888 --- /dev/null +++ b/changelog.d/11328.misc @@ -0,0 +1 @@ +Add type hints to `synapse.util`. From f310b05f0a4ed990f9f2c185b44e5cd543ad36c4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 12 Nov 2021 12:21:13 -0500 Subject: [PATCH 10/13] Lint --- synapse/util/caches/__init__.py | 2 +- synapse/util/caches/descriptors.py | 2 +- synapse/util/metrics.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/util/caches/__init__.py b/synapse/util/caches/__init__.py index 5265524ed179..15debd6c460f 100644 --- a/synapse/util/caches/__init__.py +++ b/synapse/util/caches/__init__.py @@ -17,7 +17,7 @@ import typing from enum import Enum, auto from sys import intern -from typing import Any, Callable, Dict, List, Optional, Sized, TypeVar +from typing import Any, Callable, Dict, List, Optional, Sized import attr from prometheus_client.core import Gauge diff --git a/synapse/util/caches/descriptors.py b/synapse/util/caches/descriptors.py index a3c0a6ca56ed..375cd443f14e 100644 --- a/synapse/util/caches/descriptors.py +++ b/synapse/util/caches/descriptors.py @@ -21,8 +21,8 @@ Callable, Dict, Generic, - Iterable, Hashable, + Iterable, Mapping, Optional, Sequence, diff --git a/synapse/util/metrics.py b/synapse/util/metrics.py index 949b5e23932c..ad775dfc7d79 100644 --- a/synapse/util/metrics.py +++ b/synapse/util/metrics.py @@ -64,6 +64,7 @@ sub_metrics=["real_time_max", "real_time_sum"], ) + # This is dynamically created in InFlightGauge.__init__. class _InFlightMetric(Protocol): real_time_max: float From c4feb6bdcb6912284451fa282f614f57303d085f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 15 Nov 2021 12:58:24 -0500 Subject: [PATCH 11/13] Handle review comments. --- synapse/util/async_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index e7eeed13508c..20ce294209ad 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -355,7 +355,7 @@ def _await_lock(self, key: Hashable) -> defer.Deferred: new_defer = make_deferred_yieldable(defer.Deferred()) entry.deferreds[new_defer] = 1 - def cb(_r: None) -> defer.Deferred: + def cb(_r: None) -> "defer.Deferred[None]": logger.debug("Acquired linearizer lock %r for key %r", self.name, key) entry.count += 1 @@ -542,7 +542,7 @@ def cancel_timeout(result: _T) -> _T: deferred.addBoth(cancel_timeout) - def success_cb(val: Any) -> None: + def success_cb(val: _T) -> None: if not new_d.called: new_d.callback(val) From e4e13ad7ff669af4b8822798c506a7d87743ac92 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 07:46:35 -0500 Subject: [PATCH 12/13] Add a note about type hints. --- synapse/util/gai_resolver.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/util/gai_resolver.py b/synapse/util/gai_resolver.py index 9e7480419b79..cb79f33ba720 100644 --- a/synapse/util/gai_resolver.py +++ b/synapse/util/gai_resolver.py @@ -38,6 +38,9 @@ from twisted.internet.threads import deferToThreadPool if TYPE_CHECKING: + # The types below are copied from + # https://github.com/twisted/twisted/blob/release-21.2.0-10091/src/twisted/internet/interfaces.py + # so that the type hints can match the interfaces. from twisted.python.runtime import platform if platform.supportsThreads(): From 3f533729ff17fdaf912ccdf719eed59a1ac0e9c5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 16 Nov 2021 08:06:36 -0500 Subject: [PATCH 13/13] Reference the upstream issue with type hints. --- synapse/util/gai_resolver.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/synapse/util/gai_resolver.py b/synapse/util/gai_resolver.py index cb79f33ba720..214eb17fbccb 100644 --- a/synapse/util/gai_resolver.py +++ b/synapse/util/gai_resolver.py @@ -22,6 +22,7 @@ Optional, Sequence, Tuple, + Type, Union, ) @@ -133,14 +134,16 @@ def __init__( ) self._getaddrinfo = getaddrinfo - def resolveHostName( + # The types on IHostnameResolver is incorrect in Twisted, see + # https://twistedmatrix.com/trac/ticket/10276 + def resolveHostName( # type: ignore[override] self, resolutionReceiver: IResolutionReceiver, hostName: str, portNumber: int = 0, - addressTypes: Optional[Sequence[IAddress]] = None, + addressTypes: Optional[Sequence[Type[IAddress]]] = None, transportSemantics: str = "TCP", - ) -> IResolutionReceiver: + ) -> IHostResolution: """ See L{IHostnameResolver.resolveHostName} @param resolutionReceiver: see interface @@ -152,7 +155,7 @@ def resolveHostName( """ pool = self._getThreadPool() addressFamily = _typesToAF[ - _any if addressTypes is None else frozenset(addressTypes) # type: ignore[arg-type] + _any if addressTypes is None else frozenset(addressTypes) ] socketType = _transportToSocket[transportSemantics] @@ -177,6 +180,4 @@ def deliverResults(result: _GETADDRINFO_RESULT) -> None: ) resolutionReceiver.resolutionComplete() - # IHostnameResolver declares that resolverHostName returns IResolutionReceiver, - # but the implementations return IHostResolution. - return resolution # type: ignore[return-value] + return resolution