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