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

Clean-up type hints in server config #10915

Merged
merged 5 commits into from
Sep 28, 2021
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/10915.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Clean-up configuration helper classes for the `ServerConfig` class.
100 changes: 49 additions & 51 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import os.path
import re
from textwrap import indent
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple
from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union

import attr
import yaml
Expand Down Expand Up @@ -184,49 +184,74 @@ def generate_ip_set(

@attr.s(frozen=True)
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't use slots since we splat a dictionary directly into it from the user (e.g. HttpResourceConfig(**config)).

class HttpResourceConfig:
names = attr.ib(
type=List[str],
names: List[str] = attr.ib(
factory=list,
validator=attr.validators.deep_iterable(attr.validators.in_(KNOWN_RESOURCES)), # type: ignore
)
compress = attr.ib(
type=bool,
compress: bool = attr.ib(
default=False,
validator=attr.validators.optional(attr.validators.instance_of(bool)), # type: ignore[arg-type]
Comment on lines -187 to 193
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a style preference or a functional change? (Perhaps mypy et al only recognise the annotation?)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a style preference, mypy pulls it out either way.

If auto_attribs=True is used, it is required.

)


@attr.s(frozen=True)
@attr.s(slots=True, frozen=True, auto_attribs=True)
class HttpListenerConfig:
"""Object describing the http-specific parts of the config of a listener"""

x_forwarded = attr.ib(type=bool, default=False)
resources = attr.ib(type=List[HttpResourceConfig], factory=list)
additional_resources = attr.ib(type=Dict[str, dict], factory=dict)
tag = attr.ib(type=str, default=None)
x_forwarded: bool = False
resources: List[HttpResourceConfig] = attr.ib(factory=list)
additional_resources: Dict[str, dict] = attr.ib(factory=dict)
tag: Optional[str] = None


@attr.s(frozen=True)
@attr.s(slots=True, frozen=True, auto_attribs=True)
class ListenerConfig:
"""Object describing the configuration of a single listener."""

port = attr.ib(type=int, validator=attr.validators.instance_of(int))
bind_addresses = attr.ib(type=List[str])
type = attr.ib(type=str, validator=attr.validators.in_(KNOWN_LISTENER_TYPES))
tls = attr.ib(type=bool, default=False)
port: int = attr.ib(validator=attr.validators.instance_of(int))
bind_addresses: List[str]
type: str = attr.ib(validator=attr.validators.in_(KNOWN_LISTENER_TYPES))
tls: bool = False

# http_options is only populated if type=http
http_options = attr.ib(type=Optional[HttpListenerConfig], default=None)
http_options: Optional[HttpListenerConfig] = None


@attr.s(frozen=True)
@attr.s(slots=True, frozen=True, auto_attribs=True)
class ManholeConfig:
"""Object describing the configuration of the manhole"""

username = attr.ib(type=str, validator=attr.validators.instance_of(str))
password = attr.ib(type=str, validator=attr.validators.instance_of(str))
priv_key = attr.ib(type=Optional[Key])
pub_key = attr.ib(type=Optional[Key])
username: str = attr.ib(validator=attr.validators.instance_of(str))
password: str = attr.ib(validator=attr.validators.instance_of(str))
priv_key: Optional[Key]
pub_key: Optional[Key]


@attr.s(slots=True, frozen=True, auto_attribs=True)
class RetentionConfig:
"""Object describing the configuration of the manhole"""

interval: int
shortest_max_lifetime: Optional[int]
longest_max_lifetime: Optional[int]


@attr.s(frozen=True)
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't use slots since we splat a dictionary directly into it from the user (e.g. LimitRemoteRoomsConfig(**config)).

class LimitRemoteRoomsConfig:
enabled: bool = attr.ib(validator=attr.validators.instance_of(bool), default=False)
complexity: Union[float, int] = attr.ib(
validator=attr.validators.instance_of(
(float, int) # type: ignore[arg-type] # noqa
),
default=1.0,
)
complexity_error: str = attr.ib(
validator=attr.validators.instance_of(str),
default=ROOM_COMPLEXITY_TOO_GREAT,
)
admins_can_join: bool = attr.ib(
validator=attr.validators.instance_of(bool), default=False
)


class ServerConfig(Config):
Expand Down Expand Up @@ -519,7 +544,7 @@ def read_config(self, config, **kwargs):
" greater than 'allowed_lifetime_max'"
)

self.retention_purge_jobs: List[Dict[str, Optional[int]]] = []
self.retention_purge_jobs: List[RetentionConfig] = []
for purge_job_config in retention_config.get("purge_jobs", []):
interval_config = purge_job_config.get("interval")

Expand Down Expand Up @@ -553,20 +578,12 @@ def read_config(self, config, **kwargs):
)

self.retention_purge_jobs.append(
{
"interval": interval,
"shortest_max_lifetime": shortest_max_lifetime,
"longest_max_lifetime": longest_max_lifetime,
}
RetentionConfig(interval, shortest_max_lifetime, longest_max_lifetime)
)

if not self.retention_purge_jobs:
self.retention_purge_jobs = [
{
"interval": self.parse_duration("1d"),
"shortest_max_lifetime": None,
"longest_max_lifetime": None,
}
RetentionConfig(self.parse_duration("1d"), None, None)
]

self.listeners = [parse_listener_def(x) for x in config.get("listeners", [])]
Expand All @@ -591,25 +608,6 @@ def read_config(self, config, **kwargs):
self.gc_thresholds = read_gc_thresholds(config.get("gc_thresholds", None))
self.gc_seconds = self.read_gc_intervals(config.get("gc_min_interval", None))

@attr.s
class LimitRemoteRoomsConfig:
enabled = attr.ib(
validator=attr.validators.instance_of(bool), default=False
)
complexity = attr.ib(
validator=attr.validators.instance_of(
(float, int) # type: ignore[arg-type] # noqa
),
default=1.0,
)
complexity_error = attr.ib(
validator=attr.validators.instance_of(str),
default=ROOM_COMPLEXITY_TOO_GREAT,
)
admins_can_join = attr.ib(
validator=attr.validators.instance_of(bool), default=False
)

self.limit_remote_rooms = LimitRemoteRoomsConfig(
**(config.get("limit_remote_rooms") or {})
)
Expand Down
8 changes: 4 additions & 4 deletions synapse/handlers/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,16 @@ def __init__(self, hs: "HomeServer"):

if hs.config.worker.run_background_tasks and hs.config.retention_enabled:
# Run the purge jobs described in the configuration file.
for job in hs.config.retention_purge_jobs:
for job in hs.config.server.retention_purge_jobs:
logger.info("Setting up purge job with config: %s", job)

self.clock.looping_call(
run_as_background_process,
job["interval"],
job.interval,
"purge_history_for_rooms_in_range",
self.purge_history_for_rooms_in_range,
job["shortest_max_lifetime"],
job["longest_max_lifetime"],
job.shortest_max_lifetime,
job.longest_max_lifetime,
)

async def purge_history_for_rooms_in_range(
Expand Down