Add a keyword argument to get_json to avoid retrying on DNS failures. Rather than passing MatrixHttpClient.RETRY_DNS_LOOKUP_FAILURES as a fake query string parameter

6 files changed, 16 insertions, 20 deletions

diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py
index 96b82f00cb..5f96f79998 100644
--- a/synapse/federation/replication.py
+++ b/synapse/federation/replication.py
@@ -159,7 +159,8 @@ class ReplicationLayer(object):
         return defer.succeed(None)
 
     @log_function
-    def make_query(self, destination, query_type, args):
+    def make_query(self, destination, query_type, args,
+                   retry_on_dns_fail=True):
         """Sends a federation Query to a remote homeserver of the given type
         and arguments.
 
@@ -174,7 +175,9 @@ class ReplicationLayer(object):
             a Deferred which will eventually yield a JSON object from the
             response
         """
-        return self.transport_layer.make_query(destination, query_type, args)
+        return self.transport_layer.make_query(
+            destination, query_type, args, retry_on_dns_fail=retry_on_dns_fail
+        )
 
     @defer.inlineCallbacks
     @log_function
diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py
index afc777ec9e..93296af204 100644
--- a/synapse/federation/transport.py
+++ b/synapse/federation/transport.py
@@ -193,13 +193,14 @@ class TransportLayer(object):
 
     @defer.inlineCallbacks
     @log_function
-    def make_query(self, destination, query_type, args):
+    def make_query(self, destination, query_type, args, retry_on_dns_fail):
         path = PREFIX + "/query/%s" % query_type
 
         response = yield self.client.get_json(
             destination=destination,
             path=path,
-            args=args
+            args=args,
+            retry_on_dns_fail=retry_on_dns_fail,
         )
 
         defer.returnValue(response)
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index cec7737e09..a56830d520 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -18,7 +18,6 @@ from twisted.internet import defer
 from ._base import BaseHandler
 
 from synapse.api.errors import SynapseError
-from synapse.http.client import MatrixHttpClient
 from synapse.api.events.room import RoomAliasesEvent
 
 import logging
@@ -98,8 +97,8 @@ class DirectoryHandler(BaseHandler):
                 query_type="directory",
                 args={
                     "room_alias": room_alias.to_string(),
-                    MatrixHttpClient.RETRY_DNS_LOOKUP_FAILURES: False
-                }
+                },
+                retry_on_dns_fail=False,
             )
 
             if result and "room_id" in result and "servers" in result:
diff --git a/synapse/http/client.py b/synapse/http/client.py
index e02cce5642..57b49355f2 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -183,7 +183,7 @@ class MatrixHttpClient(BaseHttpClient):
         defer.returnValue((response.code, body))
 
     @defer.inlineCallbacks
-    def get_json(self, destination, path, args={}):
+    def get_json(self, destination, path, args={}, retry_on_dns_fail=True):
         """ Get's some json from the given host homeserver and path
 
         Args:
@@ -203,13 +203,6 @@ class MatrixHttpClient(BaseHttpClient):
         """
         logger.debug("get_json args: %s", args)
 
-        retry_on_dns_fail = True
-        if HttpClient.RETRY_DNS_LOOKUP_FAILURES in args:
-            # FIXME: This isn't ideal, but the interface exposed in get_json
-            # isn't comprehensive enough to give caller's any control over
-            # their connection mechanics.
-            retry_on_dns_fail = args.pop(HttpClient.RETRY_DNS_LOOKUP_FAILURES)
-
         query_bytes = urllib.urlencode(args, True)
         logger.debug("Query bytes: %s Retry DNS: %s", args, retry_on_dns_fail)
 
diff --git a/tests/federation/test_federation.py b/tests/federation/test_federation.py
index bb17e9aafe..d95b9013a3 100644
--- a/tests/federation/test_federation.py
+++ b/tests/federation/test_federation.py
@@ -253,7 +253,7 @@ class FederationTestCase(unittest.TestCase):
         response = yield self.federation.make_query(
             destination="remote",
             query_type="a-question",
-            args={"one": "1", "two": "2"}
+            args={"one": "1", "two": "2"},
         )
 
         self.assertEquals({"your": "response"}, response)
@@ -261,7 +261,8 @@ class FederationTestCase(unittest.TestCase):
         self.mock_http_client.get_json.assert_called_with(
             destination="remote",
             path="/_matrix/federation/v1/query/a-question",
-            args={"one": "1", "two": "2"}
+            args={"one": "1", "two": "2"},
+            retry_on_dns_fail=True,
         )
 
     @defer.inlineCallbacks
diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py
index 0c31502dd4..e10a49a8ac 100644
--- a/tests/handlers/test_directory.py
+++ b/tests/handlers/test_directory.py
@@ -20,7 +20,6 @@ from twisted.internet import defer
 from mock import Mock
 
 from synapse.server import HomeServer
-from synapse.http.client import MatrixHttpClient
 from synapse.handlers.directory import DirectoryHandler
 from synapse.storage.directory import RoomAliasMapping
 
@@ -95,8 +94,8 @@ class DirectoryTestCase(unittest.TestCase):
             query_type="directory",
             args={
                 "room_alias": "#another:remote",
-                MatrixHttpClient.RETRY_DNS_LOOKUP_FAILURES: False
-            }
+            },
+            retry_on_dns_fail=False,
         )
 
     @defer.inlineCallbacks