From b87e1b6ebb0c46e5c1e36938335803211436845d Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 23 Sep 2021 17:12:28 +0100 Subject: [PATCH 1/5] Pass str to twisted's IReactorTCP This follows a correction made in twisted/twisted#1664. Until that change is in a twisted release, we'll have to ignore the type of the `host` argument. I'll raise an issue to remind us to review the issue in a few months' time. --- synapse/handlers/send_email.py | 9 +++++++-- synapse/replication/tcp/handler.py | 8 ++++++-- synapse/replication/tcp/redis.py | 8 +++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index 25e6b012b7ca..1a062a784cd4 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -105,8 +105,13 @@ def build_sender_factory(**kwargs: Any) -> ESMTPSenderFactory: # set to enable TLS. factory = build_sender_factory(hostname=smtphost if enable_tls else None) - # the IReactorTCP interface claims host has to be a bytes, which seems to be wrong - reactor.connectTCP(smtphost, smtpport, factory, timeout=30, bindAddress=None) # type: ignore[arg-type] + reactor.connectTCP( + smtphost, # type: ignore[arg-type] + smtpport, + factory, + timeout=30, + bindAddress=None, + ) await make_deferred_yieldable(d) diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index 509ed7fb13ca..5e4cd6e88e64 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -315,7 +315,7 @@ def start_replication(self, hs): hs, outbound_redis_connection ) hs.get_reactor().connectTCP( - hs.config.redis.redis_host.encode(), + hs.config.redis.redis_host, # type: ignore[arg-type] hs.config.redis.redis_port, self._factory, ) @@ -324,7 +324,11 @@ def start_replication(self, hs): self._factory = DirectTcpReplicationClientFactory(hs, client_name, self) host = hs.config.worker_replication_host port = hs.config.worker_replication_port - hs.get_reactor().connectTCP(host.encode(), port, self._factory) + hs.get_reactor().connectTCP( + host, # type: ignore[arg-type] + port, + self._factory, + ) def get_streams(self) -> Dict[str, Stream]: """Get a map from stream name to all streams.""" diff --git a/synapse/replication/tcp/redis.py b/synapse/replication/tcp/redis.py index 8c0df627c8d0..062fe2f33e0c 100644 --- a/synapse/replication/tcp/redis.py +++ b/synapse/replication/tcp/redis.py @@ -364,6 +364,12 @@ def lazyConnection( factory.continueTrying = reconnect reactor = hs.get_reactor() - reactor.connectTCP(host.encode(), port, factory, timeout=30, bindAddress=None) + reactor.connectTCP( + host, # type: ignore[arg-type] + port, + factory, + timeout=30, + bindAddress=None, + ) return factory.handler From 78e9c738bc1d0ff5429e79e8028bff0efd7784b7 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 23 Sep 2021 17:20:58 +0100 Subject: [PATCH 2/5] Changelog --- changelog.d/10895.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10895.misc diff --git a/changelog.d/10895.misc b/changelog.d/10895.misc new file mode 100644 index 000000000000..d1c822498016 --- /dev/null +++ b/changelog.d/10895.misc @@ -0,0 +1 @@ +Fix type hints to be compatible with an upcoming change to Twisted. \ No newline at end of file From e9b8b8e057a496862924df1c2245a2608657cce9 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 23 Sep 2021 18:47:44 +0100 Subject: [PATCH 3/5] Fix unit test --- tests/replication/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index e9fd991718d9..d5a68df83209 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -424,7 +424,7 @@ def connect_any_redis_attempts(self): clients = self.reactor.tcpClients while clients: (host, port, client_factory, _timeout, _bindAddress) = clients.pop(0) - self.assertEqual(host, b"localhost") + self.assertEqual(host, "localhost") self.assertEqual(port, 6379) client_protocol = client_factory.buildProtocol(None) From e831726815b2bb58c7d7607c27fe368fc9582f98 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 27 Sep 2021 14:43:26 +0100 Subject: [PATCH 4/5] replication setup should add cb. with host: str --- tests/replication/_base.py | 2 +- tests/server.py | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index dcf828872c51..cdd6e3d3c1df 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -240,7 +240,7 @@ def setUp(self): if self.hs.config.redis.redis_enabled: # Handle attempts to connect to fake redis server. self.reactor.add_tcp_client_callback( - b"localhost", + "localhost", 6379, self.connect_any_redis_attempts, ) diff --git a/tests/server.py b/tests/server.py index 88dfa8058e62..4c7a3b12e3fe 100644 --- a/tests/server.py +++ b/tests/server.py @@ -317,7 +317,7 @@ class ThreadedMemoryReactorClock(MemoryReactorClock): def __init__(self): self.threadpool = ThreadPool(self) - self._tcp_callbacks = {} + self._tcp_callbacks: Dict[Tuple[str, int], Callable] = {} self._udp = [] self.lookups: Dict[str, str] = {} self._thread_callbacks: Deque[Callable[[], None]] = deque() @@ -355,7 +355,7 @@ def callFromThread(self, callback, *args, **kwargs): def getThreadPool(self): return self.threadpool - def add_tcp_client_callback(self, host, port, callback): + def add_tcp_client_callback(self, host: str, port: int, callback: Callable): """Add a callback that will be invoked when we receive a connection attempt to the given IP/port using `connectTCP`. @@ -364,12 +364,10 @@ def add_tcp_client_callback(self, host, port, callback): """ self._tcp_callbacks[(host, port)] = callback - def connectTCP(self, host, port, factory, timeout=30, bindAddress=None): + def connectTCP(self, host: str, port: int, factory, timeout=30, bindAddress=None): """Fake L{IReactorTCP.connectTCP}.""" - conn = super().connectTCP( - host, port, factory, timeout=timeout, bindAddress=None - ) + conn = super().connectTCP(host, port, factory, timeout=timeout, bindAddress=None) callback = self._tcp_callbacks.get((host, port)) if callback: @@ -475,7 +473,7 @@ def runInteraction(interaction, *args, **kwargs): return server -def get_clock(): +def get_clock() -> Tuple[ThreadedMemoryReactorClock, Clock]: clock = ThreadedMemoryReactorClock() hs_clock = Clock(clock) return clock, hs_clock From dfa13c2f912cb738c6ab29eb01f83ede3cd8ca91 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 27 Sep 2021 14:47:00 +0100 Subject: [PATCH 5/5] Lint whoooops --- tests/server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/server.py b/tests/server.py index 4c7a3b12e3fe..64645651ce5d 100644 --- a/tests/server.py +++ b/tests/server.py @@ -367,7 +367,9 @@ def add_tcp_client_callback(self, host: str, port: int, callback: Callable): def connectTCP(self, host: str, port: int, factory, timeout=30, bindAddress=None): """Fake L{IReactorTCP.connectTCP}.""" - conn = super().connectTCP(host, port, factory, timeout=timeout, bindAddress=None) + conn = super().connectTCP( + host, port, factory, timeout=timeout, bindAddress=None + ) callback = self._tcp_callbacks.get((host, port)) if callback: