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

Fix typechecking errors introduced in #14128 #14455

Merged
merged 3 commits into from
Nov 15, 2022
Merged

Fix typechecking errors introduced in #14128 #14455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a3e208
Fix typechecking errors introduced in #14128
Nov 15, 2022
2845688
Changelog
Nov 15, 2022
c1cc906
Correct annotations
Nov 15, 2022
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/14455.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add TLS support for generic worker endpoints.
4 changes: 2 additions & 2 deletions synapse/app/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,8 @@ def listen_http(
root_resource: Resource,
version_string: str,
max_request_body_size: int,
context_factory: IOpenSSLContextFactory,
reactor: IReactorSSL = reactor,
context_factory: Optional[IOpenSSLContextFactory],
reactor: ISynapseReactor = reactor,
) -> List[Port]:
port = listener_config.port
bind_addresses = listener_config.bind_addresses
Expand Down
5 changes: 3 additions & 2 deletions synapse/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,6 @@ class HomeServer(metaclass=abc.ABCMeta):
# instantiated during setup() for future return by get_datastores()
DATASTORE_CLASS = abc.abstractproperty()

tls_server_context_factory: Optional[IOpenSSLContextFactory]

Comment on lines -224 to -225
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with ABCMeta but I don't think this should have magically an instance attribute?

(What I'm driving at is: why didn't complement fall over on #14128 but did on an earlier version of this PR?)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with ABCMeta but I don't think this should have magically an instance attribute?

Regardless of ABCMeta I would think this would end up as an expected property? But yeah there's no guarantee it actually gets added unless we do it in __init__.

(Using a free function to set it is kind of meh...)

def __init__(
self,
hostname: str,
Expand Down Expand Up @@ -258,6 +256,9 @@ def __init__(
self._module_web_resources: Dict[str, Resource] = {}
self._module_web_resources_consumed = False

# This attribute is set by the free function `refresh_certificate`.
self.tls_server_context_factory: Optional[IOpenSSLContextFactory] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this might be redundant; I think mypy infers the type of instance attributes if they're set in the constructor like this? 🤷


def register_module_web_resource(self, path: str, resource: Resource) -> None:
"""Allows a module to register a web resource to be served at the given path.

Expand Down