Fix confused handling of replication ports in the DB (#420)

It's niche, but if no replication base url was configured and a peer was
configured with a NULL port in the db, then we would try to format None
as an integer and get a TypeError for our troubles.

Spotted by work in progress elsewhere to make Sydent pass mypy --strict.

Fixes #419.

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>

changelog.d/420.bugfix

@@ -0,0 +1 @@
+Fix misleading logging and potential TypeErrors related to replication ports in Sydent's database.

sydent/replication/peer.py

@@ -16,7 +16,7 @@
 import binascii
 import json
 import logging
-from typing import TYPE_CHECKING, Any, Dict
+from typing import TYPE_CHECKING, Any, Dict, Optional
 
 import signedjson.key
 import signedjson.sign
@@ -120,20 +120,19 @@ def __init__(
         self,
         sydent: "Sydent",
         server_name: str,
-        port: int,
+        port: Optional[int],
         pubkeys: Dict[str, str],
         lastSentVersion: int,
     ) -> None:
         """
         :param sydent: The current Sydent instance.
         :param server_name: The peer's server name.
-        :param port: The peer's port.
+        :param port: The peer's port. Only used if no replication url is configured.
         :param pubkeys: The peer's public keys in a dict[key_id, key_b64]
         :param lastSentVersion: The ID of the last association sent to the peer.
         """
         super().__init__(server_name, pubkeys)
         self.sydent = sydent
-        self.port = port
         self.lastSentVersion = lastSentVersion
 
         # look up or build the replication URL
@@ -147,6 +146,8 @@ def __init__(
         if replication_url[-1:] != "/":
             replication_url += "/"
 
+        # Capture the interesting bit of the url for logging.
+        self.replication_url_origin = replication_url
         replication_url += "_matrix/identity/replicate/v1/push"
         self.replication_url = replication_url
 

sydent/replication/pusher.py

@@ -89,7 +89,9 @@ async def _push_to_peer(self, p: "RemotePeer") -> None:
 
         # Check if a push operation is already active. If so, don't start another
         if p.is_being_pushed_to:
-            logger.debug("Waiting for %s:%d to finish pushing...", p.servername, p.port)
+            logger.debug(
+                "Waiting for %s to finish pushing...", p.replication_url_origin
+            )
             return
 
         p.is_being_pushed_to = True
@@ -108,7 +110,7 @@ async def _push_to_peer(self, p: "RemotePeer") -> None:
                 return
 
             logger.info(
-                "Pushing %d updates to %s:%d", len(assocs), p.servername, p.port
+                "Pushing %d updates to %s", len(assocs), p.replication_url_origin
             )
             result = await p.pushUpdates(assocs)
 
@@ -117,14 +119,13 @@ async def _push_to_peer(self, p: "RemotePeer") -> None:
             )
 
             logger.info(
-                "Pushed updates to %s:%d with result %d %s",
-                p.servername,
-                p.port,
+                "Pushed updates to %s with result %d %s",
+                p.replication_url_origin,
                 result.code,
                 result.phrase,
             )
         except Exception:
-            logger.exception("Error pushing updates to %s:%d", p.servername, p.port)
+            logger.exception("Error pushing updates to %s", p.replication_url_origin)
         finally:
             # Whether pushing completed or an error occurred, signal that pushing has finished
             p.is_being_pushed_to = False