From 0cb8502bbbcbfc383214045eea4f45dc6a8b1724 Mon Sep 17 00:00:00 2001
From: Mathieu Velten <matmaul@gmail.com>
Date: Tue, 13 Jun 2023 23:48:02 +0200
Subject: [PATCH] Use parse_duration for newly introduced options

---
 synapse/config/federation.py              | 12 ++++++--
 synapse/http/matrixfederationclient.py    | 35 +++++++++++------------
 tests/http/test_matrixfederationclient.py |  6 ++--
 3 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/synapse/config/federation.py b/synapse/config/federation.py
index d21f7fd02a..e42fa80638 100644
--- a/synapse/config/federation.py
+++ b/synapse/config/federation.py
@@ -53,9 +53,15 @@ class FederationConfig(Config):
 
         # Allow for the configuration of timeout, max request retries
         # and min/max retry delays in the matrix federation client.
-        self.client_timeout = federation_config.get("client_timeout", 60)
-        self.max_long_retry_delay = federation_config.get("max_long_retry_delay", 60)
-        self.max_short_retry_delay = federation_config.get("max_short_retry_delay", 2)
+        self.client_timeout = Config.parse_duration(
+            federation_config.get("client_timeout", "60s")
+        )
+        self.max_long_retry_delay = Config.parse_duration(
+            federation_config.get("max_long_retry_delay", "60s")
+        )
+        self.max_short_retry_delay = Config.parse_duration(
+            federation_config.get("max_short_retry_delay", "2s")
+        )
         self.max_long_retries = federation_config.get("max_long_retries", 10)
         self.max_short_retries = federation_config.get("max_short_retries", 3)
 
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index ed36825b67..1ac3cbca5c 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -404,10 +404,10 @@ class MatrixFederationHttpClient:
         self.clock = hs.get_clock()
         self._store = hs.get_datastores().main
         self.version_string_bytes = hs.version_string.encode("ascii")
-        self.default_timeout = hs.config.federation.client_timeout
+        self.default_timeout = hs.config.federation.client_timeout / 1000
 
-        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay
-        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay
+        self.max_long_retry_delay = hs.config.federation.max_long_retry_delay / 1000
+        self.max_short_retry_delay = hs.config.federation.max_short_retry_delay / 1000
         self.max_long_retries = hs.config.federation.max_long_retries
         self.max_short_retries = hs.config.federation.max_short_retries
 
@@ -538,10 +538,10 @@ class MatrixFederationHttpClient:
             logger.exception(f"Invalid destination: {request.destination}.")
             raise FederationDeniedError(request.destination)
 
-        if timeout:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+
+        _sec_timeout = timeout / 1000
 
         if (
             self.hs.config.federation.federation_domain_whitelist is not None
@@ -946,10 +946,9 @@ class MatrixFederationHttpClient:
             timeout=timeout,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1135,10 +1134,9 @@ class MatrixFederationHttpClient:
             timeout=timeout,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         if parser is None:
             parser = cast(ByteParser[T], JsonParser())
@@ -1211,10 +1209,9 @@ class MatrixFederationHttpClient:
             ignore_backoff=ignore_backoff,
         )
 
-        if timeout is not None:
-            _sec_timeout = timeout / 1000
-        else:
-            _sec_timeout = self.default_timeout
+        if timeout is None:
+            timeout = int(self.default_timeout)
+        _sec_timeout = timeout / 1000
 
         body = await _handle_response(
             self.reactor, _sec_timeout, request, response, start_ms, parser=JsonParser()
diff --git a/tests/http/test_matrixfederationclient.py b/tests/http/test_matrixfederationclient.py
index 8565f8ac64..9eea7870b8 100644
--- a/tests/http/test_matrixfederationclient.py
+++ b/tests/http/test_matrixfederationclient.py
@@ -644,9 +644,9 @@ class FederationClientTests(HomeserverTestCase):
     @override_config(
         {
             "federation": {
-                "client_timeout": 180,
-                "max_long_retry_delay": 100,
-                "max_short_retry_delay": 7,
+                "client_timeout": "180s",
+                "max_long_retry_delay": "100s",
+                "max_short_retry_delay": "7s",
                 "max_long_retries": 20,
                 "max_short_retries": 5,
             }