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

Additional type hints for config module. #11465

Merged
merged 10 commits into from
Dec 1, 2021
3 changes: 2 additions & 1 deletion synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
Iterable,
List,
NoReturn,
Optional,
Tuple,
cast,
)
Expand Down Expand Up @@ -129,7 +130,7 @@ def start_worker_reactor(
def start_reactor(
appname: str,
soft_file_limit: int,
gc_thresholds: Tuple[int, int, int],
gc_thresholds: Optional[Tuple[int, int, int]],
pid_file: str,
daemonize: bool,
print_pidfile: bool,
Expand Down
15 changes: 10 additions & 5 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from twisted.conch.ssh.keys import Key

from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
from synapse.types import JsonDict
from synapse.util.module_loader import load_module
from synapse.util.stringutils import parse_and_validate_server_name

Expand Down Expand Up @@ -1275,14 +1276,16 @@ def read_gc_intervals(self, durations) -> Optional[Tuple[float, float, float]]:
)


def is_threepid_reserved(reserved_threepids, threepid):
def is_threepid_reserved(
reserved_threepids: List[JsonDict], threepid: JsonDict
) -> bool:
"""Check the threepid against the reserved threepid config
Args:
reserved_threepids([dict]) - list of reserved threepids
threepid(dict) - The threepid to test for
reserved_threepids: List of reserved threepids
threepid: The threepid to test for

Returns:
boolean Is the threepid undertest reserved_user
Is the threepid undertest reserved_user
"""

for tp in reserved_threepids:
Expand All @@ -1291,7 +1294,9 @@ def is_threepid_reserved(reserved_threepids, threepid):
return False


def read_gc_thresholds(thresholds):
def read_gc_thresholds(
thresholds: Optional[List[Any]],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe List[SupportsInt] if you're feeling fancy!

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, below on +1314, looks like listener could be Dict: [str, Any]

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. That's what is expected by these methods, but not what is necessarily passed in?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess since this does runtime checking it makes sense to be flexible with its inputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure what the best practice is here, but this is where we're taking any YAML junk and ensuring it is correct, so I think it would be a lie to say it has to accept integers?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. (The argument I had in mind was that mypy could detect type errors if we described the intended inputs. But it's not going to be able to do that when the yaml comes at runtime.)

) -> Optional[Tuple[int, int, int]]:
"""Reads the three integer thresholds for garbage collection. Ensures that
the thresholds are integers if thresholds are supplied.
"""
Expand Down