Mark Bergsma has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/405284 )

Change subject: Improve and clarify error handling of DNS lookups
......................................................................

Improve and clarify error handling of DNS lookups

Add a test for a nonexistent DNS name.

Rename _hostnameResolved to _allLookupsCompleted to better express its
function.

Consume all errors in the DeferredList because they're not useful and
error handling is done by _allLookupsCompleted.

Change-Id: I125951079cd07ef71964c5e576c3168d55c17d37
---
M pybal/server.py
M pybal/test/test_server.py
2 files changed, 20 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/debs/pybal 
refs/changes/84/405284/1

diff --git a/pybal/server.py b/pybal/server.py
index 48b35fc..f1ba983 100644
--- a/pybal/server.py
+++ b/pybal/server.py
@@ -11,6 +11,7 @@
 
 from twisted.internet import defer, reactor
 from twisted.names import client, dns
+from twisted.names.error import AuthoritativeDomainError
 from twisted.python import failure
 
 from pybal import util
@@ -89,7 +90,8 @@
         lookups.append(client.lookupIPV6Address(self.host, timeout
             ).addCallback(self._lookupFinished, socket.AF_INET6, query))
 
-        return defer.DeferredList(lookups).addBoth(self._hostnameResolved)
+        return defer.DeferredList(lookups, consumeErrors=True
+            ).addCallback(self._allLookupsCompleted)
 
     def _lookupFinished(self, (answers, authority, additional), addressFamily, 
query):
         ips = set([socket.inet_ntop(addressFamily, r.payload.address)
@@ -108,7 +110,7 @@
 
         return ips
 
-    def _hostnameResolved(self, result):
+    def _allLookupsCompleted(self, results):
         # Pick *1* main ip address to use. Prefer any existing one
         # if still available.
 
@@ -127,11 +129,12 @@
         try:
             if not self.ip or self.ip not in ip_addresses:
                 self.ip = random.choice(list(ip_addresses))
-                # TODO: (re)pool
         except IndexError:
-            return failure.Failure() # TODO: be more specific?
+            errmsg = "Could not resolve {} to IP addresses for AF {})".format(
+                self.host, self.addressFamily)
+            raise AuthoritativeDomainError(errmsg)
         else:
-            return True
+            return self.ip
 
     def destroy(self):
         self.enabled = False
diff --git a/pybal/test/test_server.py b/pybal/test/test_server.py
index e6d6bc1..7ff0a31 100644
--- a/pybal/test/test_server.py
+++ b/pybal/test/test_server.py
@@ -72,12 +72,23 @@
 
     def testResolveHostname(self):
         def callback(result):
-            self.assertTrue((result == True or isinstance(result, 
failure.Failure)))
+            self.assertIsNotNone(result)
 
         deferred = self.server.resolveHostname()
         deferred.addCallback(callback)
         return deferred
 
+    def testResolveNonexistentHostname(self):
+        def errback(err):
+            self.assertTrue(isinstance(err, failure.Failure))
+            return True
+
+        nonexistentServer = pybal.server.Server(
+            'nonexistent.example.com', self.lvsservice)
+        deferred = nonexistentServer.resolveHostname()
+        deferred.addErrback(errback)
+        return deferred
+
     def testDestroy(self):
         self.server.destroy()
         self.assertFalse(self.server.enabled)

-- 
To view, visit https://gerrit.wikimedia.org/r/405284
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I125951079cd07ef71964c5e576c3168d55c17d37
Gerrit-PatchSet: 1
Gerrit-Project: operations/debs/pybal
Gerrit-Branch: master
Gerrit-Owner: Mark Bergsma <m...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to