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 <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
