diff --git a/acme/acme/client.py b/acme/acme/client.py index 478536ecc..49c6bcb21 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -1,5 +1,4 @@ """ACME client API.""" -import collections import datetime import heapq import logging @@ -335,9 +334,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes :param authzrs: `list` of `.AuthorizationResource` :param int mintime: Minimum time before next attempt, used if ``Retry-After`` is not present in the response. - :param int max_attempts: Maximum number of attempts (per - authorization) before `PollError` with non-empty ``waiting`` - is raised. + :param int max_attempts: Maximum number of attempts before + `PollError` with non-empty ``waiting`` is raised. :returns: ``(cert, updated_authzrs)`` `tuple` where ``cert`` is the issued certificate (`.messages.CertificateResource`), @@ -351,11 +349,6 @@ class Client(object): # pylint: disable=too-many-instance-attributes was marked by the CA as invalid """ - # pylint: disable=too-many-locals - assert max_attempts > 0 - attempts = collections.defaultdict(int) - exhausted = set() - # priority queue with datetime (based on Retry-After) as key, # and original Authorization Resource as value waiting = [(datetime.datetime.now(), authzr) for authzr in authzrs] @@ -363,7 +356,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes # recently updated one updated = dict((authzr, authzr) for authzr in authzrs) - while waiting: + while waiting and max_attempts: + max_attempts -= 1 # find the smallest Retry-After, and sleep if necessary when, authzr = heapq.heappop(waiting) now = datetime.datetime.now() @@ -377,20 +371,16 @@ class Client(object): # pylint: disable=too-many-instance-attributes updated_authzr, response = self.poll(updated[authzr]) updated[authzr] = updated_authzr - attempts[authzr] += 1 # pylint: disable=no-member if updated_authzr.body.status not in ( messages.STATUS_VALID, messages.STATUS_INVALID): - if attempts[authzr] < max_attempts: - # push back to the priority queue, with updated retry_after - heapq.heappush(waiting, (self.retry_after( - response, default=mintime), authzr)) - else: - exhausted.add(authzr) + # push back to the priority queue, with updated retry_after + heapq.heappush(waiting, (self.retry_after( + response, default=mintime), authzr)) - if exhausted or any(authzr.body.status == messages.STATUS_INVALID - for authzr in six.itervalues(updated)): - raise errors.PollError(exhausted, updated) + if not max_attempts or any(authzr.body.status == messages.STATUS_INVALID + for authzr in six.itervalues(updated)): + raise errors.PollError(waiting, updated) updated_authzrs = tuple(updated[authzr] for authzr in authzrs) return self.request_issuance(csr, updated_authzrs), updated_authzrs diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 9abc69c7c..449bd695e 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -319,10 +319,7 @@ class ClientTest(unittest.TestCase): ) cert, updated_authzrs = self.client.poll_and_request_issuance( - csr, authzrs, mintime=mintime, - # make sure that max_attempts is per-authorization, rather - # than global - max_attempts=max(len(authzrs[0].retries), len(authzrs[1].retries))) + csr, authzrs, mintime=mintime) self.assertTrue(cert[0] is csr) self.assertTrue(cert[1] is updated_authzrs) self.assertEqual(updated_authzrs[0].uri, 'a...') diff --git a/acme/acme/errors.py b/acme/acme/errors.py index 77d47c522..0385667c7 100644 --- a/acme/acme/errors.py +++ b/acme/acme/errors.py @@ -56,25 +56,26 @@ class MissingNonce(NonceError): class PollError(ClientError): """Generic error when polling for authorization fails. - This might be caused by either timeout (`exhausted` will be non-empty) + This might be caused by either timeout (`waiting` will be non-empty) or by some authorization being invalid. - :ivar exhausted: Set of `.AuthorizationResource` that didn't finish - within max allowed attempts. + :ivar waiting: Priority queue with `datetime.datatime` (based on + ``Retry-After``) as key, and original `.AuthorizationResource` + as value. :ivar updated: Mapping from original `.AuthorizationResource` to the most recently updated one """ - def __init__(self, exhausted, updated): - self.exhausted = exhausted + def __init__(self, waiting, updated): + self.waiting = waiting self.updated = updated super(PollError, self).__init__() @property def timeout(self): """Was the error caused by timeout?""" - return bool(self.exhausted) + return bool(self.waiting) def __repr__(self): - return '{0}(exhausted={1!r}, updated={2!r})'.format( - self.__class__.__name__, self.exhausted, self.updated) + return '{0}(waiting={1!r}, updated={2!r})'.format( + self.__class__.__name__, self.waiting, self.updated) diff --git a/acme/acme/errors_test.py b/acme/acme/errors_test.py index 966be8f1e..45b269a0b 100644 --- a/acme/acme/errors_test.py +++ b/acme/acme/errors_test.py @@ -1,4 +1,5 @@ """Tests for acme.errors.""" +import datetime import unittest import mock @@ -35,9 +36,9 @@ class PollErrorTest(unittest.TestCase): def setUp(self): from acme.errors import PollError self.timeout = PollError( - exhausted=set([mock.sentinel.AR]), + waiting=[(datetime.datetime(2015, 11, 29), mock.sentinel.AR)], updated={}) - self.invalid = PollError(exhausted=set(), updated={ + self.invalid = PollError(waiting=[], updated={ mock.sentinel.AR: mock.sentinel.AR2}) def test_timeout(self): @@ -45,7 +46,7 @@ class PollErrorTest(unittest.TestCase): self.assertFalse(self.invalid.timeout) def test_repr(self): - self.assertEqual('PollError(exhausted=set([]), updated={sentinel.AR: ' + self.assertEqual('PollError(waiting=[], updated={sentinel.AR: ' 'sentinel.AR2})', repr(self.invalid))