Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use ParamSpec in type hints for synapse.logging.context #12150

Merged
merged 3 commits into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/12150.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use `ParamSpec` in type hints for `synapse.logging.context`.
5 changes: 3 additions & 2 deletions synapse/handlers/initial_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ async def _snapshot_all_rooms(

public_room_ids = await self.store.get_public_room_ids()

limit = pagin_config.limit
if limit is None:
if pagin_config.limit is not None:
limit = pagin_config.limit
else:
limit = 10

async def handle_room(event: RoomsForUser) -> None:
Expand Down
44 changes: 24 additions & 20 deletions synapse/logging/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
from types import TracebackType
from typing import (
TYPE_CHECKING,
Any,
Awaitable,
Callable,
Optional,
Expand All @@ -41,7 +40,7 @@
)

import attr
from typing_extensions import Literal
from typing_extensions import Literal, ParamSpec

from twisted.internet import defer, threads
from twisted.python.threadpool import ThreadPool
Expand Down Expand Up @@ -719,40 +718,41 @@ def nested_logging_context(suffix: str) -> LoggingContext:
)


P = ParamSpec("P")
R = TypeVar("R")


@overload
def preserve_fn( # type: ignore[misc]
f: Callable[..., Awaitable[R]],
) -> Callable[..., "defer.Deferred[R]"]:
f: Callable[P, Awaitable[R]],
) -> Callable[P, "defer.Deferred[R]"]:
# The `type: ignore[misc]` above suppresses
# "Overloaded function signatures 1 and 2 overlap with incompatible return types"
...


@overload
def preserve_fn(f: Callable[..., R]) -> Callable[..., "defer.Deferred[R]"]:
def preserve_fn(f: Callable[P, R]) -> Callable[P, "defer.Deferred[R]"]:
...


def preserve_fn(
f: Union[
Callable[..., R],
Callable[..., Awaitable[R]],
Callable[P, R],
Callable[P, Awaitable[R]],
]
) -> Callable[..., "defer.Deferred[R]"]:
) -> Callable[P, "defer.Deferred[R]"]:
"""Function decorator which wraps the function with run_in_background"""

def g(*args: Any, **kwargs: Any) -> "defer.Deferred[R]":
def g(*args: P.args, **kwargs: P.kwargs) -> "defer.Deferred[R]":
return run_in_background(f, *args, **kwargs)

return g


@overload
def run_in_background( # type: ignore[misc]
f: Callable[..., Awaitable[R]], *args: Any, **kwargs: Any
f: Callable[P, Awaitable[R]], *args: P.args, **kwargs: P.kwargs
) -> "defer.Deferred[R]":
# The `type: ignore[misc]` above suppresses
# "Overloaded function signatures 1 and 2 overlap with incompatible return types"
Expand All @@ -761,18 +761,22 @@ def run_in_background( # type: ignore[misc]

@overload
def run_in_background(
f: Callable[..., R], *args: Any, **kwargs: Any
f: Callable[P, R], *args: P.args, **kwargs: P.kwargs
) -> "defer.Deferred[R]":
...


def run_in_background(
def run_in_background( # type: ignore[misc]
# The `type: ignore[misc]` above suppresses
# "Overloaded function implementation does not accept all possible arguments of signature 1"
# "Overloaded function implementation does not accept all possible arguments of signature 2"
clokep marked this conversation as resolved.
Show resolved Hide resolved
# which seems like a bug in mypy.
f: Union[
Callable[..., R],
Callable[..., Awaitable[R]],
Callable[P, R],
Callable[P, Awaitable[R]],
],
*args: Any,
**kwargs: Any,
*args: P.args,
**kwargs: P.kwargs,
) -> "defer.Deferred[R]":
"""Calls a function, ensuring that the current context is restored after
return from the function, and that the sentinel context is set once the
Expand Down Expand Up @@ -872,7 +876,7 @@ def _set_context_cb(result: ResultT, context: LoggingContext) -> ResultT:


def defer_to_thread(
reactor: "ISynapseReactor", f: Callable[..., R], *args: Any, **kwargs: Any
reactor: "ISynapseReactor", f: Callable[P, R], *args: P.args, **kwargs: P.kwargs
) -> "defer.Deferred[R]":
"""
Calls the function `f` using a thread from the reactor's default threadpool and
Expand Down Expand Up @@ -908,9 +912,9 @@ def defer_to_thread(
def defer_to_threadpool(
reactor: "ISynapseReactor",
threadpool: ThreadPool,
f: Callable[..., R],
*args: Any,
**kwargs: Any,
f: Callable[P, R],
*args: P.args,
**kwargs: P.kwargs,
) -> "defer.Deferred[R]":
"""
A wrapper for twisted.internet.threads.deferToThreadpool, which handles
Expand Down
3 changes: 2 additions & 1 deletion synapse/python_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
"netaddr>=0.7.18",
"Jinja2>=2.9",
"bleach>=1.4.3",
"typing-extensions>=3.7.4",
# We use `ParamSpec`, which was added in `typing-extensions` 3.10.0.0.
"typing-extensions>=3.10.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any issues with bumping this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment as to why we require a given version.

Copy link
Contributor Author

@squahtx squahtx Mar 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment with words about ParamSpec

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any issues with bumping this?

Not sure I understand the question. What should I be looking out for?

Copy link
Contributor Author

@squahtx squahtx Mar 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the package maintainers: "Fedora 34 and 35 has 3.7.4.3. Only Fedora 36 will get 3.10.0.2".
Fedora 34 and 35 EOL is 17th May 2022 and 7th December 2022 respectively.

So we either have to wait until December to merge this, or see if the packagers are willing to bring 3.10.0.2 to Fedora 34/35.

...or use if TYPE_CHECKING guards everywhere, which would be annoying, as clokep points out.

# We enforce that we have a `cryptography` version that bundles an `openssl`
# with the latest security patches.
"cryptography>=3.4.7",
Expand Down
9 changes: 7 additions & 2 deletions synapse/rest/media/v1/storage_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import logging
import os
import shutil
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING, Callable, Optional

from synapse.config._base import Config
from synapse.logging.context import defer_to_thread, run_in_background
Expand Down Expand Up @@ -150,8 +150,13 @@ async def store_file(self, path: str, file_info: FileInfo) -> None:
dirname = os.path.dirname(backup_fname)
os.makedirs(dirname, exist_ok=True)

# mypy needs help inferring the type of the second parameter, which is generic
shutil_copyfile: Callable[[str, str], str] = shutil.copyfile
clokep marked this conversation as resolved.
Show resolved Hide resolved
await defer_to_thread(
self.hs.get_reactor(), shutil.copyfile, primary_fname, backup_fname
self.hs.get_reactor(),
shutil_copyfile,
primary_fname,
backup_fname,
)

async def fetch(self, path: str, file_info: FileInfo) -> Optional[Responder]:
Expand Down