From f984191b0475d08a65af60a00c598756f32f0401 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 4 Oct 2021 11:52:46 +0100 Subject: [PATCH 1/3] no-untyped-defs for synapse.logging.formatter --- mypy.ini | 3 +++ synapse/logging/formatter.py | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/mypy.ini b/mypy.ini index 568166db3300..7760ea3d422f 100644 --- a/mypy.ini +++ b/mypy.ini @@ -96,6 +96,9 @@ files = [mypy-synapse.handlers.*] disallow_untyped_defs = True +[mypy-synapse.logging.formatter] +disallow_untyped_defs = True + [mypy-synapse.rest.*] disallow_untyped_defs = True diff --git a/synapse/logging/formatter.py b/synapse/logging/formatter.py index c0f12ecd15b8..bbfd08b9b9d1 100644 --- a/synapse/logging/formatter.py +++ b/synapse/logging/formatter.py @@ -16,6 +16,13 @@ import logging import traceback from io import StringIO +from types import TracebackType +from typing import Optional, Tuple, Type, Union + +ExceptionInfo = Union[ + Tuple[Type[BaseException], BaseException, Optional[TracebackType]], + Tuple[None, None, None], +] class LogFormatter(logging.Formatter): @@ -28,10 +35,7 @@ class LogFormatter(logging.Formatter): where it was caught are logged). """ - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - - def formatException(self, ei): + def formatException(self, ei: ExceptionInfo) -> str: sio = StringIO() (typ, val, tb) = ei From 63c33aa4ce163cf7ec6566b4c19a4b28b2b8b37e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 4 Oct 2021 11:54:23 +0100 Subject: [PATCH 2/3] no-untyped-defs for synapse.logging.handlers --- mypy.ini | 3 +++ synapse/logging/handlers.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/mypy.ini b/mypy.ini index 7760ea3d422f..78ecf4937983 100644 --- a/mypy.ini +++ b/mypy.ini @@ -99,6 +99,9 @@ disallow_untyped_defs = True [mypy-synapse.logging.formatter] disallow_untyped_defs = True +[mypy-synapse.logging.handlers] +disallow_untyped_defs = True + [mypy-synapse.rest.*] disallow_untyped_defs = True diff --git a/synapse/logging/handlers.py b/synapse/logging/handlers.py index af5fc407a8e5..fbd6ba810291 100644 --- a/synapse/logging/handlers.py +++ b/synapse/logging/handlers.py @@ -49,7 +49,7 @@ def __init__( ) self._flushing_thread.start() - def on_reactor_running(): + def on_reactor_running() -> None: self._reactor_started = True reactor_to_use: IReactorCore @@ -74,7 +74,7 @@ def shouldFlush(self, record: LogRecord) -> bool: else: return True - def _flush_periodically(self): + def _flush_periodically(self) -> None: """ Whilst this handler is active, flush the handler periodically. """ From 36f47b37a9dad97879a0dfdc1e79fd7db1e78c19 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 4 Oct 2021 13:15:51 +0100 Subject: [PATCH 3/3] Easier fn annotations for synapse.logging.context Not yet passing `no-untyped-defs`. `make_deferred_yieldable` is tricky and needs more thought. --- synapse/logging/context.py | 83 +++++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 24 deletions(-) diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 02e5ddd2ef2a..fc3e514ec8d0 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -22,21 +22,37 @@ See doc/log_contexts.rst for details on how this works. """ +import functools import inspect import logging import threading import typing import warnings -from typing import TYPE_CHECKING, Optional, Tuple, TypeVar, Union +from types import TracebackType +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Optional, + Tuple, + Type, + TypeVar, + Union, + cast, +) import attr from typing_extensions import Literal from twisted.internet import defer, threads +from twisted.internet.interfaces import IReactorThreads, ThreadPool if TYPE_CHECKING: from synapse.logging.scopecontextmanager import _LogContextScope +T = TypeVar("T") +F = TypeVar("F", bound=Callable[..., Any]) + logger = logging.getLogger(__name__) try: @@ -66,7 +82,7 @@ def get_thread_resource_usage() -> "Optional[resource._RUsage]": # a hook which can be set during testing to assert that we aren't abusing logcontexts. -def logcontext_error(msg: str): +def logcontext_error(msg: str) -> None: logger.warning(msg) @@ -220,28 +236,28 @@ def __init__(self) -> None: self.scope = None self.tag = None - def __str__(self): + def __str__(self) -> str: return "sentinel" - def copy_to(self, record): + def copy_to(self, record: "LoggingContext") -> None: pass - def start(self, rusage: "Optional[resource._RUsage]"): + def start(self, rusage: "Optional[resource._RUsage]") -> None: pass - def stop(self, rusage: "Optional[resource._RUsage]"): + def stop(self, rusage: "Optional[resource._RUsage]") -> None: pass - def add_database_transaction(self, duration_sec): + def add_database_transaction(self, duration_sec: float) -> None: pass - def add_database_scheduled(self, sched_sec): + def add_database_scheduled(self, sched_sec: float) -> None: pass - def record_event_fetch(self, event_count): + def record_event_fetch(self, event_count: int) -> None: pass - def __bool__(self): + def __bool__(self) -> bool: return False @@ -379,7 +395,12 @@ def __enter__(self) -> "LoggingContext": ) return self - def __exit__(self, type, value, traceback) -> None: + def __exit__( + self, + type: Optional[Type[BaseException]], + value: Optional[BaseException], + traceback: Optional[TracebackType], + ) -> None: """Restore the logging context in thread local storage to the state it was before this context was entered. Returns: @@ -399,10 +420,8 @@ def __exit__(self, type, value, traceback) -> None: # recorded against the correct metrics. self.finished = True - def copy_to(self, record) -> None: - """Copy logging fields from this context to a log record or - another LoggingContext - """ + def copy_to(self, record: "LoggingContext") -> None: + """Copy logging fields from this context to another LoggingContext""" # we track the current request record.request = self.request @@ -575,7 +594,7 @@ class LoggingContextFilter(logging.Filter): record. """ - def __init__(self, request: str = ""): + def __init__(self, request: str = "") -> None: self._default_request = request def filter(self, record: logging.LogRecord) -> Literal[True]: @@ -626,7 +645,12 @@ def __init__( def __enter__(self) -> None: self._old_context = set_current_context(self._new_context) - def __exit__(self, type, value, traceback) -> None: + def __exit__( + self, + type: Optional[Type[BaseException]], + value: Optional[BaseException], + traceback: Optional[TracebackType], + ) -> None: context = set_current_context(self._old_context) if context != self._new_context: @@ -711,16 +735,19 @@ def nested_logging_context(suffix: str) -> LoggingContext: ) -def preserve_fn(f): +def preserve_fn(f: F) -> F: """Function decorator which wraps the function with run_in_background""" - def g(*args, **kwargs): + @functools.wraps(f) + def g(*args: Any, **kwargs: Any) -> Any: return run_in_background(f, *args, **kwargs) - return g + return cast(F, g) -def run_in_background(f, *args, **kwargs) -> defer.Deferred: +def run_in_background( + f: Callable[..., T], *args: Any, **kwargs: Any +) -> "defer.Deferred[T]": """Calls a function, ensuring that the current context is restored after return from the function, and that the sentinel context is set once the deferred returned by the function completes. @@ -823,7 +850,9 @@ def _set_context_cb(result: ResultT, context: LoggingContext) -> ResultT: return result -def defer_to_thread(reactor, f, *args, **kwargs): +def defer_to_thread( + reactor: IReactorThreads, f: Callable[..., T], *args: Any, **kwargs: Any +) -> "defer.Deferred[T]": """ Calls the function `f` using a thread from the reactor's default threadpool and returns the result as a Deferred. @@ -855,7 +884,13 @@ def defer_to_thread(reactor, f, *args, **kwargs): return defer_to_threadpool(reactor, reactor.getThreadPool(), f, *args, **kwargs) -def defer_to_threadpool(reactor, threadpool, f, *args, **kwargs): +def defer_to_threadpool( + reactor: IReactorThreads, + threadpool: ThreadPool, + f: Callable[..., T], + *args: Any, + **kwargs: Any, +) -> "defer.Deferred[T]": """ A wrapper for twisted.internet.threads.deferToThreadpool, which handles logcontexts correctly. @@ -897,7 +932,7 @@ def defer_to_threadpool(reactor, threadpool, f, *args, **kwargs): assert isinstance(curr_context, LoggingContext) parent_context = curr_context - def g(): + def g() -> T: with LoggingContext(str(curr_context), parent_context=parent_context): return f(*args, **kwargs)