Skip to content

Commit

Permalink
More on failure tracking.
Browse files Browse the repository at this point in the history
Begin generating the event that we need.
  • Loading branch information
jamadden committed Jul 28, 2020
1 parent f7a083a commit 3de5eae
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 5 deletions.
12 changes: 12 additions & 0 deletions docs/delivery_attempts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ per-principal.

.. doctest::

>>> from nti.webhooks.interfaces import ILimitedAttemptWebhookSubscription
>>> from zope.interface import verify
>>> verify.verifyObject(ILimitedAttemptWebhookSubscription, subscription)
True
>>> subscription.attempt_limit
50

Expand Down Expand Up @@ -311,6 +315,14 @@ and as the newer attempts complete, they will replace them.
>>> attempt_50 in all_attempts
True


.. todo:: We need to test this case for failures, both
at HTTP delivery time, and at DNS domain validation time.
I suspect the DNS domain validation time won't work, because
there's no parent set yet when the event goes out.

This should result in deactivating the subscription.

.. testcleanup::

from zope.testing import cleanup
Expand Down
46 changes: 45 additions & 1 deletion src/nti/webhooks/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from zope.interface import taggedValue
from zope.interface.interfaces import IInterface
from zope.interface.interfaces import IObjectEvent
from zope.interface.interfaces import ObjectEvent

from zope.container.interfaces import IContainerNamesContainer
from zope.container.interfaces import IContained
Expand Down Expand Up @@ -528,7 +529,6 @@ class IWebhookSubscription(IContainerNamesContainer):
vocabulary=UtilityNames(IWebhookDialect),
)

netloc = Attribute("The network host name portion of the URL.")
dialect = Attribute("The resolved dialect to use for this subscription.")

def isApplicable(data):
Expand Down Expand Up @@ -564,6 +564,50 @@ def createDeliveryAttempt(payload_data):
)


class ILimitedAttemptWebhookSubscription(IWebhookSubscription):
"""
A webhook subscription that should limit the number of
delivery attempts it stores.
"""

attempt_limit = Attribute(
# Note that this is not a schema field, it's intended to be configured
# on a class, or rarely, through direct intervention on a particular
# subscription.
u'An integer giving approximately the number of delivery attempts this object will store.'
)

class ILimitedApplicabilityPreconditionFailureWebhookSubscription(IWebhookSubscription):
"""
A webhook subscription that supports a limit on the number
of times checking applicability can be allowed to fail.
When this number is exceeded, an event implementing
`IWebhookSubscriptionApplicabilityPreconditionFailureLimitReached`
is notified.
"""

applicable_precondition_failure_limit = Attribute(
# As for attempt_limit.
u'An integer giving the number of times applicability checks can fail '
u'before the event is generated.'
)

class IWebhookSubscriptionApplicabilityPreconditionFailureLimitReached(IObjectEvent):

failures = Attribute(
u"An instance of :class:`nti.zodb.interfaces.INumericValue`. "
u'You may set its ``value`` property to zero if you want to start the count '
u'over. Other actions would be to make this subscription inactive.'
)


class WebhookSubscriptionApplicabilityPreconditionFailureLimitReached(ObjectEvent):

def __init__(self, subscription, failures):
ObjectEvent.__init__(self, subscription)
self.failures = failures

class IWebhookSubscriptionRegistry(Interface):

def activeSubscriptions(data, event):
Expand Down
31 changes: 27 additions & 4 deletions src/nti/webhooks/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import time

from zope import component
from zope.event import notify
from zope.interface import Interface
from zope.interface import implementer
from zope.interface import providedBy
Expand Down Expand Up @@ -40,14 +41,20 @@
from zope.cachedescriptors.property import CachedProperty

from nti.zodb.containers import time_to_64bit_int
from nti.zodb.minmax import NumericPropertyDefaultingToZero
from nti.zodb.minmax import NumericMinimum

from nti.schema.fieldproperty import createDirectFieldProperties
from nti.schema.schema import SchemaConfigured

from nti.webhooks.interfaces import IWebhookDialect
from nti.webhooks.interfaces import IWebhookSubscription
from nti.webhooks.interfaces import ILimitedAttemptWebhookSubscription
from nti.webhooks.interfaces import ILimitedApplicabilityPreconditionFailureWebhookSubscription
from nti.webhooks.interfaces import IWebhookSubscriptionManager
from nti.webhooks.interfaces import IWebhookDestinationValidator
from nti.webhooks.interfaces import IWebhookDeliveryAttemptResolvedEvent
from nti.webhooks.interfaces import WebhookSubscriptionApplicabilityPreconditionFailureLimitReached

from nti.webhooks.attempts import WebhookDeliveryAttempt
from nti.webhooks.attempts import PersistentWebhookDeliveryAttempt
Expand Down Expand Up @@ -82,7 +89,10 @@ def __call__(data, event): # pylint:disable=no-self-argument,signature-differs
"""


@implementer(IWebhookSubscription, IAttributeAnnotatable, IApplicableSubscriptionFactory)
@implementer(ILimitedAttemptWebhookSubscription,
ILimitedApplicabilityPreconditionFailureWebhookSubscription,
IAttributeAnnotatable,
IApplicableSubscriptionFactory)
class Subscription(SchemaConfigured, _CheckObjectOnSetBTreeContainer):
"""
Default, non-persistent implementation of `IWebhookSubscription`.
Expand All @@ -91,10 +101,12 @@ class Subscription(SchemaConfigured, _CheckObjectOnSetBTreeContainer):
to = u''
active = None
createDirectFieldProperties(IWebhookSubscription)
createDirectFieldProperties(ILimitedAttemptWebhookSubscription)

__parent__ = None

attempt_limit = 50
applicable_precondition_failure_limit = 50

def __init__(self, **kwargs):
SchemaConfigured.__init__(self, **kwargs)
Expand Down Expand Up @@ -192,14 +204,23 @@ def __checkSecurity(self, data):
else:
endInteraction()

_delivery_applicable_precondition_failed = NumericPropertyDefaultingToZero(
'_delivery_applicable_precondition_failed',
# We have to use NumericMinimum instead of NumericMaximum or
# MergingCounter because we periodically reset to 0. And MergingCounter
# has a bug when that happens. (https://github.com/NextThought/nti.zodb/issues/6)
NumericMinimum,
)

def __call__(self, data, event):
# We're assumed applicable for the data and event, no need to double
# check that.
assert self.active
if self.__checkSecurity(data):
return self
# TODO: Here's where the cleanup would go, or at least incrementing a
# counter and sending events when its over a certain value.
failures = self._delivery_applicable_precondition_failed.increment()
if failures.value >= self.applicable_precondition_failure_limit:
notify(WebhookSubscriptionApplicabilityPreconditionFailureLimitReached(self, failures))
return None

@CachedProperty('dialect_id')
Expand Down Expand Up @@ -269,7 +290,9 @@ def prune_subscription_when_resolved(event):
# type: (IWebhookDeliveryAttemptResolvedEvent) -> None
attempt = event.object # type: WebhookDeliveryAttempt
subscription = attempt.__parent__
if subscription is None or len(subscription) <= subscription.attempt_limit:
if not ILimitedAttemptWebhookSubscription.providedBy(subscription):
return
if len(subscription) <= subscription.attempt_limit:
return

# Copy to avoid concurrent modification. On PyPy, we've seen this
Expand Down

0 comments on commit 3de5eae

Please sign in to comment.