From ffe42c60f57e628549ffefbf25c536784fbb3252 Mon Sep 17 00:00:00 2001 From: John Carr Date: Mon, 6 Jan 2020 11:17:44 +0000 Subject: [PATCH 1/2] Add support for when callback, towards #98 --- docs/handlers.rst | 6 ++ examples/11-filtering-handlers/example.py | 10 +++ .../11-filtering-handlers/test_example_11.py | 2 + kopf/on.py | 18 +++-- kopf/reactor/handling.py | 2 +- kopf/reactor/invocation.py | 59 ++++++++++------- kopf/reactor/registries.py | 61 +++++++++++++---- kopf/toolkits/legacy_registries.py | 5 +- .../legacy/test_legacy_decorators.py | 29 +++++++-- .../legacy/test_legacy_requires_finalizer.py | 22 +++++-- tests/registries/test_decorators.py | 35 ++++++++-- tests/registries/test_handler_matching.py | 65 +++++++++++++++++++ tests/registries/test_requires_finalizer.py | 22 +++++-- 13 files changed, 266 insertions(+), 70 deletions(-) diff --git a/docs/handlers.rst b/docs/handlers.rst index ce95ce68..c51786ae 100644 --- a/docs/handlers.rst +++ b/docs/handlers.rst @@ -353,6 +353,12 @@ The following filters are available for all event, cause, and field handlers: def my_handler(spec, **_): pass +* Check on any field on the body with a when callback. The filter callback takes the same args as a handler:: + + @kopf.on.create('zalando.org', 'v1', 'kopfexamples', when=lambda body, **_: body.get('spec', {}).get('myfield', '') == 'somevalue') + def my_handler(spec, **_): + pass + Startup handlers ================ diff --git a/examples/11-filtering-handlers/example.py b/examples/11-filtering-handlers/example.py index c2f07304..120dccb0 100644 --- a/examples/11-filtering-handlers/example.py +++ b/examples/11-filtering-handlers/example.py @@ -29,3 +29,13 @@ def create_with_annotations_exist(logger, **kwargs): @kopf.on.create('zalando.org', 'v1', 'kopfexamples', annotations={'someannotation': 'othervalue'}) def create_with_annotations_not_satisfied(logger, **kwargs): logger.info("Annotation not satisfied.") + + +@kopf.on.create('zalando.org', 'v1', 'kopfexamples', when=lambda body, **_: True) +def create_with_filter_satisfied(logger, **kwargs): + logger.info("Filter satisfied.") + + +@kopf.on.create('zalando.org', 'v1', 'kopfexamples', when=lambda body, **_: False) +def create_with_filter_not_satisfied(logger, **kwargs): + logger.info("Filter not satisfied.") \ No newline at end of file diff --git a/examples/11-filtering-handlers/test_example_11.py b/examples/11-filtering-handlers/test_example_11.py index e94fae48..46b08a00 100644 --- a/examples/11-filtering-handlers/test_example_11.py +++ b/examples/11-filtering-handlers/test_example_11.py @@ -51,3 +51,5 @@ def test_handler_filtering(mocker): assert '[default/kopf-example-1] Annotation satisfied.' in runner.stdout assert '[default/kopf-example-1] Annotation exists.' in runner.stdout assert '[default/kopf-example-1] Annotation not satisfied.' not in runner.stdout + assert '[default/kopf-example-1] Filter satisfied.' in runner.stdout + assert '[default/kopf-example-1] Filter not satisfied.' not in runner.stdout diff --git a/kopf/on.py b/kopf/on.py index 52a09dc8..9645ca94 100644 --- a/kopf/on.py +++ b/kopf/on.py @@ -118,6 +118,7 @@ def resume( deleted: Optional[bool] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.resume()`` handler for the object resuming on operator (re)start. """ actual_registry = registry if registry is not None else registries.get_default_registry() @@ -126,7 +127,7 @@ def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: group=group, version=version, plural=plural, reason=None, initial=True, deleted=deleted, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, - fn=fn, labels=labels, annotations=annotations, + fn=fn, labels=labels, annotations=annotations, when=when, ) return decorator @@ -143,6 +144,7 @@ def create( registry: Optional[registries.OperatorRegistry] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.create()`` handler for the object creation. """ actual_registry = registry if registry is not None else registries.get_default_registry() @@ -151,7 +153,7 @@ def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: group=group, version=version, plural=plural, reason=causation.Reason.CREATE, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, - fn=fn, labels=labels, annotations=annotations, + fn=fn, labels=labels, annotations=annotations, when=when, ) return decorator @@ -168,6 +170,7 @@ def update( registry: Optional[registries.OperatorRegistry] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.update()`` handler for the object update or change. """ actual_registry = registry if registry is not None else registries.get_default_registry() @@ -176,7 +179,7 @@ def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: group=group, version=version, plural=plural, reason=causation.Reason.UPDATE, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, - fn=fn, labels=labels, annotations=annotations, + fn=fn, labels=labels, annotations=annotations, when=when, ) return decorator @@ -194,6 +197,7 @@ def delete( optional: Optional[bool] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.delete()`` handler for the object deletion. """ actual_registry = registry if registry is not None else registries.get_default_registry() @@ -203,7 +207,7 @@ def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: reason=causation.Reason.DELETE, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, fn=fn, requires_finalizer=bool(not optional), - labels=labels, annotations=annotations, + labels=labels, annotations=annotations, when=when, ) return decorator @@ -221,6 +225,7 @@ def field( registry: Optional[registries.OperatorRegistry] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.field()`` handler for the individual field changes. """ actual_registry = registry if registry is not None else registries.get_default_registry() @@ -229,7 +234,7 @@ def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: group=group, version=version, plural=plural, reason=None, field=field, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, - fn=fn, labels=labels, annotations=annotations, + fn=fn, labels=labels, annotations=annotations, when=when, ) return decorator @@ -241,13 +246,14 @@ def event( registry: Optional[registries.OperatorRegistry] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[registries.WhenHandlerFn] = None, ) -> ResourceHandlerDecorator: """ ``@kopf.on.event()`` handler for the silent spies on the events. """ actual_registry = registry if registry is not None else registries.get_default_registry() def decorator(fn: registries.ResourceHandlerFn) -> registries.ResourceHandlerFn: return actual_registry.register_resource_watching_handler( group=group, version=version, plural=plural, - id=id, fn=fn, labels=labels, annotations=annotations, + id=id, fn=fn, labels=labels, annotations=annotations, when=when, ) return decorator diff --git a/kopf/reactor/handling.py b/kopf/reactor/handling.py index 9b1fb36c..3ad20925 100644 --- a/kopf/reactor/handling.py +++ b/kopf/reactor/handling.py @@ -292,7 +292,7 @@ async def handle_resource_changing_cause( done = None skip = None - requires_finalizer = registry.requires_finalizer(resource=cause.resource, body=cause.body) + requires_finalizer = registry.requires_finalizer(resource=cause.resource, cause=cause) has_finalizer = finalizers.has_finalizers(body=cause.body) if requires_finalizer and not has_finalizer: diff --git a/kopf/reactor/invocation.py b/kopf/reactor/invocation.py index 83d83107..736a8533 100644 --- a/kopf/reactor/invocation.py +++ b/kopf/reactor/invocation.py @@ -9,7 +9,7 @@ import contextlib import contextvars import functools -from typing import Optional, Any, Union, List, Iterable, Iterator, Tuple +from typing import Optional, Any, Union, List, Iterable, Iterator, Tuple, Dict from kopf import config from kopf.reactor import causation @@ -41,39 +41,25 @@ def context( for var, token in reversed(tokens): var.reset(token) - -async def invoke( - fn: Invokable, - *args: Any, - cause: Optional[causation.BaseCause] = None, - **kwargs: Any, -) -> Any: +def get_invoke_arguments(*args: Any, cause: Optional[causation.BaseCause] = None, **kwargs: Any) -> Dict[str, Any]: """ - Invoke a single function, but safely for the main asyncio process. - - Used both for the handler functions and for the lifecycle callbacks. - - A full set of the arguments is provided, expanding the cause to some easily - usable aliases. The function is expected to accept ``**kwargs`` for the args - that it does not use -- for forward compatibility with the new features. - - The synchronous methods are executed in the executor (threads or processes), - thus making it non-blocking for the main event loop of the operator. - See: https://pymotw.com/3/asyncio/executors.html + Expand kwargs dict with fields from the causation. """ + new_kwargs = {} + new_kwargs.update(kwargs) # Add aliases for the kwargs, directly linked to the body, or to the assumed defaults. if isinstance(cause, causation.BaseCause): - kwargs.update( + new_kwargs.update( cause=cause, logger=cause.logger, ) if isinstance(cause, causation.ActivityCause): - kwargs.update( + new_kwargs.update( activity=cause.activity, ) if isinstance(cause, causation.ResourceCause): - kwargs.update( + new_kwargs.update( patch=cause.patch, memo=cause.memo, body=cause.body, @@ -85,12 +71,12 @@ async def invoke( namespace=cause.body.get('metadata', {}).get('namespace'), ) if isinstance(cause, causation.ResourceWatchingCause): - kwargs.update( + new_kwargs.update( event=cause.raw, type=cause.type, ) if isinstance(cause, causation.ResourceChangingCause): - kwargs.update( + new_kwargs.update( event=cause.reason, # deprecated; kept for backward-compatibility reason=cause.reason, diff=cause.diff, @@ -98,6 +84,31 @@ async def invoke( new=cause.new, ) + return new_kwargs + + +async def invoke( + fn: Invokable, + *args: Any, + cause: Optional[causation.BaseCause] = None, + **kwargs: Any, +) -> Any: + """ + Invoke a single function, but safely for the main asyncio process. + + Used both for the handler functions and for the lifecycle callbacks. + + A full set of the arguments is provided, expanding the cause to some easily + usable aliases. The function is expected to accept ``**kwargs`` for the args + that it does not use -- for forward compatibility with the new features. + + The synchronous methods are executed in the executor (threads or processes), + thus making it non-blocking for the main event loop of the operator. + See: https://pymotw.com/3/asyncio/executors.html + """ + + kwargs = get_invoke_arguments(*args, cause=cause, **kwargs) + if is_async_fn(fn): result = await fn(*args, **kwargs) # type: ignore else: diff --git a/kopf/reactor/registries.py b/kopf/reactor/registries.py index 92218130..5070f7c9 100644 --- a/kopf/reactor/registries.py +++ b/kopf/reactor/registries.py @@ -25,7 +25,7 @@ from typing_extensions import Protocol -from kopf.reactor import causation +from kopf.reactor import causation, invocation from kopf.structs import bodies from kopf.structs import dicts from kopf.structs import diffs @@ -79,6 +79,27 @@ def __call__( **kwargs: Any, ) -> Optional[HandlerResult]: ... +class WhenHandlerFn(Protocol): + def __call__( + self, + *args: Any, + type: str, + event: Union[str, bodies.Event], + body: bodies.Body, + meta: bodies.Meta, + spec: bodies.Spec, + status: bodies.Status, + uid: str, + name: str, + namespace: Optional[str], + patch: patches.Patch, + logger: Union[logging.Logger, logging.LoggerAdapter], + diff: diffs.Diff, + old: Optional[Union[bodies.BodyEssence, Any]], # "Any" is for field-handlers. + new: Optional[Union[bodies.BodyEssence, Any]], # "Any" is for field-handlers. + **kwargs: Any, + ) -> bool: ... + # A registered handler (function + meta info). # FIXME: Must be frozen, but mypy fails in _call_handler() with a cryptic error: @@ -126,6 +147,7 @@ class ResourceHandler(BaseHandler): deleted: Optional[bool] = None # used for mixed-in (initial==True) @on.resume handlers only. labels: Optional[bodies.Labels] = None annotations: Optional[bodies.Annotations] = None + when: Optional[WhenHandlerFn] = None requires_finalizer: Optional[bool] = None @property @@ -227,6 +249,7 @@ def register( requires_finalizer: bool = False, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[WhenHandlerFn] = None, ) -> ResourceHandlerFn: if reason is None and event is not None: reason = causation.Reason(event) @@ -237,7 +260,7 @@ def register( id=real_id, fn=fn, reason=reason, field=real_field, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, initial=initial, deleted=deleted, requires_finalizer=requires_finalizer, - labels=labels, annotations=annotations, + labels=labels, annotations=annotations, when=when, ) self.append(handler) @@ -270,11 +293,11 @@ def iter_extra_fields( def requires_finalizer( self, - body: bodies.Body, + cause: causation.ResourceCause, ) -> bool: # check whether the body matches a deletion handler for handler in self._handlers: - if handler.requires_finalizer and match(handler=handler, body=body): + if handler.requires_finalizer and match(handler=handler, cause=cause): return True return False @@ -287,7 +310,7 @@ def iter_handlers( cause: causation.ResourceWatchingCause, ) -> Iterator[ResourceHandler]: for handler in self._handlers: - if match(handler=handler, body=cause.body, ignore_fields=True): + if match(handler=handler, cause=cause, ignore_fields=True): yield handler @@ -304,7 +327,7 @@ def iter_handlers( pass # ignore initial handlers in non-initial causes. elif handler.initial and cause.deleted and not handler.deleted: pass # ignore initial handlers on deletion, unless explicitly marked as usable. - elif match(handler=handler, body=cause.body, changed_fields=changed_fields): + elif match(handler=handler, cause=cause, changed_fields=changed_fields): yield handler @@ -358,6 +381,7 @@ def register_resource_watching_handler( id: Optional[str] = None, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[WhenHandlerFn] = None, ) -> ResourceHandlerFn: """ Register an additional handler function for low-level events. @@ -365,7 +389,7 @@ def register_resource_watching_handler( resource = resources_.Resource(group, version, plural) return self._resource_watching_handlers[resource].register( fn=fn, id=id, - labels=labels, annotations=annotations, + labels=labels, annotations=annotations, when=when, ) def register_resource_changing_handler( @@ -388,6 +412,7 @@ def register_resource_changing_handler( requires_finalizer: bool = False, labels: Optional[bodies.Labels] = None, annotations: Optional[bodies.Annotations] = None, + when: Optional[WhenHandlerFn] = None, ) -> ResourceHandlerFn: """ Register an additional handler function for the specific resource and specific reason. @@ -397,7 +422,7 @@ def register_resource_changing_handler( reason=reason, event=event, field=field, fn=fn, id=id, errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown, initial=initial, deleted=deleted, requires_finalizer=requires_finalizer, - labels=labels, annotations=annotations, + labels=labels, annotations=annotations, when=when, ) def has_activity_handlers( @@ -481,13 +506,13 @@ def iter_extra_fields( def requires_finalizer( self, resource: resources_.Resource, - body: bodies.Body, + cause: causation.ResourceCause, ) -> bool: """ Check whether a finalizer should be added to the given resource or not. """ return (resource in self._resource_changing_handlers and - self._resource_changing_handlers[resource].requires_finalizer(body=body)) + self._resource_changing_handlers[resource].requires_finalizer(cause=cause)) class SmartOperatorRegistry(OperatorRegistry): @@ -589,14 +614,15 @@ def fn(**kwargs): pass def match( handler: ResourceHandler, - body: bodies.Body, + cause: causation.ResourceCause, changed_fields: Collection[dicts.FieldPath] = frozenset(), ignore_fields: bool = False, ) -> bool: return all([ _matches_field(handler, changed_fields or {}, ignore_fields), - _matches_labels(handler, body), - _matches_annotations(handler, body), + _matches_labels(handler, cause.body), + _matches_annotations(handler, cause.body), + _matches_filter_callback(handler, cause), ]) @@ -643,6 +669,15 @@ def _matches_metadata( return True +def _matches_filter_callback( + handler: ResourceHandler, + cause: causation.ResourceCause, +) -> bool: + if not handler.when: + return True + return handler.when(**invocation.get_invoke_arguments(cause=cause)) + + _default_registry: Optional[OperatorRegistry] = None diff --git a/kopf/toolkits/legacy_registries.py b/kopf/toolkits/legacy_registries.py index b170b4c0..e780a9bb 100644 --- a/kopf/toolkits/legacy_registries.py +++ b/kopf/toolkits/legacy_registries.py @@ -97,8 +97,9 @@ def iter_event_handlers( warnings.warn("SimpleRegistry.iter_event_handlers() is deprecated; use " "ResourceWatchingRegistry.iter_handlers().", DeprecationWarning) + cause = _create_watching_cause(resource, event) for handler in self._handlers: - if registries.match(handler=handler, body=event['object'], ignore_fields=True): + if registries.match(handler=handler, cause=cause, ignore_fields=True): yield handler def iter_cause_handlers( @@ -113,7 +114,7 @@ def iter_cause_handlers( if handler.reason is None or handler.reason == cause.reason: if handler.initial and not cause.initial: pass # ignore initial handlers in non-initial causes. - elif registries.match(handler=handler, body=cause.body, + elif registries.match(handler=handler, cause=cause, changed_fields=changed_fields): yield handler diff --git a/tests/registries/legacy/test_legacy_decorators.py b/tests/registries/legacy/test_legacy_decorators.py index 3c711137..e0b2a24c 100644 --- a/tests/registries/legacy/test_legacy_decorators.py +++ b/tests/registries/legacy/test_legacy_decorators.py @@ -24,6 +24,7 @@ def fn(**_): assert handlers[0].timeout is None assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_update_minimal(mocker): @@ -43,6 +44,7 @@ def fn(**_): assert handlers[0].timeout is None assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_delete_minimal(mocker): @@ -62,6 +64,7 @@ def fn(**_): assert handlers[0].timeout is None assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_field_minimal(mocker): @@ -82,6 +85,7 @@ def fn(**_): assert handlers[0].timeout is None assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_field_fails_without_field(): @@ -97,10 +101,13 @@ def test_on_create_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.CREATE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda **_: False + @kopf.on.create('group', 'version', 'plural', id='id', timeout=123, registry=registry, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -113,7 +120,7 @@ def fn(**_): assert handlers[0].timeout == 123 assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} - + assert handlers[0].when == when def test_on_update_with_all_kwargs(mocker): registry = GlobalRegistry() @@ -121,10 +128,13 @@ def test_on_update_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda **_: False + @kopf.on.update('group', 'version', 'plural', id='id', timeout=123, registry=registry, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -137,6 +147,7 @@ def fn(**_): assert handlers[0].timeout == 123 assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when @pytest.mark.parametrize('optional', [ @@ -149,10 +160,13 @@ def test_on_delete_with_all_kwargs(mocker, optional): cause = mocker.MagicMock(resource=resource, reason=Reason.DELETE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda **_: False + @kopf.on.delete('group', 'version', 'plural', id='id', timeout=123, registry=registry, optional=optional, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -165,6 +179,7 @@ def fn(**_): assert handlers[0].timeout == 123 assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_on_field_with_all_kwargs(mocker): @@ -174,10 +189,13 @@ def test_on_field_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE, diff=diff) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.field('group', 'version', 'plural', 'field.subfield', id='id', timeout=123, registry=registry, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -190,6 +208,7 @@ def fn(**_): assert handlers[0].timeout == 123 assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_subhandler_declaratively(mocker): diff --git a/tests/registries/legacy/test_legacy_requires_finalizer.py b/tests/registries/legacy/test_legacy_requires_finalizer.py index 0af11baf..bd15167c 100644 --- a/tests/registries/legacy/test_legacy_requires_finalizer.py +++ b/tests/registries/legacy/test_legacy_requires_finalizer.py @@ -3,6 +3,7 @@ import kopf from kopf import GlobalRegistry from kopf.structs.resources import Resource +from kopf.reactor.causation import ResourceCause OBJECT_BODY = { 'apiVersion': 'group/version', @@ -18,6 +19,13 @@ } } +CAUSE = ResourceCause( + logger=None, + resource=None, + patch=None, + body=OBJECT_BODY, + memo=None +) @pytest.mark.parametrize('optional, expected', [ pytest.param(True, False, id='optional'), @@ -32,7 +40,7 @@ def test_requires_finalizer_deletion_handler(optional, expected): def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -54,7 +62,7 @@ def fn1(**_): def fn2(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -67,7 +75,7 @@ def test_requires_finalizer_no_deletion_handler(): def fn1(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer is False @@ -89,7 +97,7 @@ def test_requires_finalizer_deletion_handler_matches_labels(labels, optional, ex def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -111,7 +119,7 @@ def test_requires_finalizer_deletion_handler_mismatches_labels(labels, optional, def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -133,7 +141,7 @@ def test_requires_finalizer_deletion_handler_matches_annotations(annotations, op def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -155,5 +163,5 @@ def test_requires_finalizer_deletion_handler_mismatches_annotations(annotations, def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected diff --git a/tests/registries/test_decorators.py b/tests/registries/test_decorators.py index 6ac13771..be8c143e 100644 --- a/tests/registries/test_decorators.py +++ b/tests/registries/test_decorators.py @@ -84,6 +84,7 @@ def fn(**_): assert handlers[0].cooldown is None # deprecated alias assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_create_minimal(mocker): @@ -107,6 +108,7 @@ def fn(**_): assert handlers[0].cooldown is None # deprecated alias assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_update_minimal(mocker): @@ -130,6 +132,7 @@ def fn(**_): assert handlers[0].cooldown is None # deprecated alias assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_delete_minimal(mocker): @@ -153,6 +156,7 @@ def fn(**_): assert handlers[0].cooldown is None # deprecated alias assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_field_minimal(mocker): @@ -177,6 +181,7 @@ def fn(**_): assert handlers[0].cooldown is None # deprecated alias assert handlers[0].labels is None assert handlers[0].annotations is None + assert handlers[0].when is None def test_on_field_fails_without_field(): @@ -257,12 +262,15 @@ def test_on_resume_with_all_kwargs(mocker, reason): cause = mocker.MagicMock(resource=resource, reason=reason, initial=True, deleted=False) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.resume('group', 'version', 'plural', id='id', registry=registry, errors=ErrorsMode.PERMANENT, timeout=123, retries=456, backoff=78, deleted=True, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -280,6 +288,7 @@ def fn(**_): assert handlers[0].deleted == True assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_on_create_with_all_kwargs(mocker): @@ -288,11 +297,14 @@ def test_on_create_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.CREATE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.create('group', 'version', 'plural', id='id', registry=registry, errors=ErrorsMode.PERMANENT, timeout=123, retries=456, backoff=78, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -309,6 +321,7 @@ def fn(**_): assert handlers[0].cooldown == 78 # deprecated alias assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_on_update_with_all_kwargs(mocker): @@ -317,11 +330,14 @@ def test_on_update_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.update('group', 'version', 'plural', id='id', registry=registry, errors=ErrorsMode.PERMANENT, timeout=123, retries=456, backoff=78, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -338,6 +354,7 @@ def fn(**_): assert handlers[0].cooldown == 78 # deprecated alias assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when @pytest.mark.parametrize('optional', [ @@ -350,12 +367,15 @@ def test_on_delete_with_all_kwargs(mocker, optional): cause = mocker.MagicMock(resource=resource, reason=Reason.DELETE) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.delete('group', 'version', 'plural', id='id', registry=registry, errors=ErrorsMode.PERMANENT, timeout=123, retries=456, backoff=78, optional=optional, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -372,6 +392,7 @@ def fn(**_): assert handlers[0].cooldown == 78 # deprecated alias assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_on_field_with_all_kwargs(mocker): @@ -381,11 +402,14 @@ def test_on_field_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE, diff=diff) mocker.patch('kopf.reactor.registries.match', return_value=True) + when = lambda body: False + @kopf.on.field('group', 'version', 'plural', 'field.subfield', id='id', registry=registry, errors=ErrorsMode.PERMANENT, timeout=123, retries=456, backoff=78, labels={'somelabel': 'somevalue'}, - annotations={'someanno': 'somevalue'}) + annotations={'someanno': 'somevalue'}, + when=when) def fn(**_): pass @@ -402,6 +426,7 @@ def fn(**_): assert handlers[0].cooldown == 78 # deprecated alias assert handlers[0].labels == {'somelabel': 'somevalue'} assert handlers[0].annotations == {'someanno': 'somevalue'} + assert handlers[0].when == when def test_subhandler_declaratively(mocker): diff --git a/tests/registries/test_handler_matching.py b/tests/registries/test_handler_matching.py index 775b0d54..f66e9817 100644 --- a/tests/registries/test_handler_matching.py +++ b/tests/registries/test_handler_matching.py @@ -4,6 +4,7 @@ import pytest from kopf import ResourceRegistry, OperatorRegistry +from kopf.reactor.causation import ResourceChangingCause # Used in the tests. Must be global-scoped, or its qualname will be affected. @@ -223,6 +224,47 @@ def test_catchall_handlers_with_labels_and_annotations_not_satisfied(registry, r assert not handlers +@pytest.mark.parametrize('when', [ + pytest.param(None, id='without-when'), + pytest.param(lambda body=None, **_: body['spec']['name'] == 'test', id='with-when'), + pytest.param(lambda **_: True, id='with-other-when'), +]) +def test_catchall_handlers_with_when_match(registry, register_fn, resource, when): + cause = ResourceChangingCause( + resource=resource, + reason='some-reason', + diff=None, + body={'spec': {'name': 'test'}}, + logger=None, + patch=None, + memo=None, + initial=None + ) + register_fn(some_fn, reason=None, field=None, when=when) + handlers = registry.get_resource_changing_handlers(cause) + assert handlers + + +@pytest.mark.parametrize('when', [ + pytest.param(lambda body=None, **_: body['spec']['name'] != "test", id='with-when'), + pytest.param(lambda **_: False, id='with-other-when'), +]) +def test_catchall_handlers_with_when_not_match(registry, register_fn, resource, when): + cause = ResourceChangingCause( + resource=resource, + reason='some-reason', + diff=None, + body={'spec': {'name': 'test'}}, + logger=None, + patch=None, + memo=None, + initial=None + ) + register_fn(some_fn, reason=None, field=None, when=when) + handlers = registry.get_resource_changing_handlers(cause) + assert not handlers + + # # Relevant handlers are those with event == 'some-reason' (but not 'another-reason'). # In the per-field handlers, also with field == 'some-field' (not 'another-field'). @@ -272,6 +314,18 @@ def test_relevant_handlers_with_annotations_not_satisfied(cause_any_diff, regist assert not handlers +def test_relevant_handlers_with_filter_satisfied(cause_any_diff, registry, register_fn): + register_fn(some_fn, reason='some-reason', when=lambda *_: True) + handlers = registry.get_resource_changing_handlers(cause_any_diff) + assert handlers + + +def test_relevant_handlers_with_filter_not_satisfied(cause_any_diff, registry, register_fn): + register_fn(some_fn, reason='some-reason', when=lambda *_: False) + handlers = registry.get_resource_changing_handlers(cause_any_diff) + assert not handlers + + def test_irrelevant_handlers_without_field_ignored(cause_any_diff, registry, register_fn): register_fn(some_fn, reason='another-reason') handlers = registry.get_resource_changing_handlers(cause_any_diff) @@ -307,6 +361,17 @@ def test_irrelevant_handlers_with_annotations_not_satisfied(cause_any_diff, regi assert not handlers +def test_irrelevant_handlers_with_when_satisfied(cause_any_diff, registry, register_fn): + register_fn(some_fn, reason='another-reason', when=lambda *_: True) + handlers = registry.get_resource_changing_handlers(cause_any_diff) + assert not handlers + + +def test_irrelevant_handlers_with_when_not_satisfied(cause_any_diff, registry, register_fn): + register_fn(some_fn, reason='another-reason', when=lambda *_: False) + handlers = registry.get_resource_changing_handlers(cause_any_diff) + assert not handlers + # # The handlers must be returned in order of registration, # even if they are mixed with-/without- * -event/-field handlers. diff --git a/tests/registries/test_requires_finalizer.py b/tests/registries/test_requires_finalizer.py index 7067de7a..10d77a24 100644 --- a/tests/registries/test_requires_finalizer.py +++ b/tests/registries/test_requires_finalizer.py @@ -3,6 +3,7 @@ import kopf from kopf.reactor.registries import OperatorRegistry from kopf.structs.resources import Resource +from kopf.reactor.causation import ResourceCause OBJECT_BODY = { 'apiVersion': 'group/version', @@ -18,6 +19,13 @@ } } +CAUSE = ResourceCause( + logger=None, + resource=None, + patch=None, + body=OBJECT_BODY, + memo=None +) @pytest.mark.parametrize('optional, expected', [ pytest.param(True, False, id='optional'), @@ -32,7 +40,7 @@ def test_requires_finalizer_deletion_handler(optional, expected): def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -54,7 +62,7 @@ def fn1(**_): def fn2(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -67,7 +75,7 @@ def test_requires_finalizer_no_deletion_handler(): def fn1(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer is False @@ -89,7 +97,7 @@ def test_requires_finalizer_deletion_handler_matches_labels(labels, optional, ex def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -111,7 +119,7 @@ def test_requires_finalizer_deletion_handler_mismatches_labels(labels, optional, def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -133,7 +141,7 @@ def test_requires_finalizer_deletion_handler_matches_annotations(annotations, op def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected @@ -155,5 +163,5 @@ def test_requires_finalizer_deletion_handler_mismatches_annotations(annotations, def fn(**_): pass - requires_finalizer = registry.requires_finalizer(resource=resource, body=OBJECT_BODY) + requires_finalizer = registry.requires_finalizer(resource=resource, cause=CAUSE) assert requires_finalizer == expected From a8c15e24d30441f7f9b3a0056647c1d5fb006711 Mon Sep 17 00:00:00 2001 From: John Carr Date: Wed, 15 Jan 2020 13:42:07 +0000 Subject: [PATCH 2/2] Feedback from review --- kopf/reactor/invocation.py | 6 +++++- tests/registries/legacy/test_legacy_decorators.py | 2 +- tests/registries/test_decorators.py | 10 +++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/kopf/reactor/invocation.py b/kopf/reactor/invocation.py index 736a8533..602f1210 100644 --- a/kopf/reactor/invocation.py +++ b/kopf/reactor/invocation.py @@ -41,7 +41,11 @@ def context( for var, token in reversed(tokens): var.reset(token) -def get_invoke_arguments(*args: Any, cause: Optional[causation.BaseCause] = None, **kwargs: Any) -> Dict[str, Any]: +def get_invoke_arguments( + *args: Any, + cause: Optional[causation.BaseCause] = None, + **kwargs: Any +) -> Dict[str, Any]: """ Expand kwargs dict with fields from the causation. """ diff --git a/tests/registries/legacy/test_legacy_decorators.py b/tests/registries/legacy/test_legacy_decorators.py index e0b2a24c..2baa749a 100644 --- a/tests/registries/legacy/test_legacy_decorators.py +++ b/tests/registries/legacy/test_legacy_decorators.py @@ -189,7 +189,7 @@ def test_on_field_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE, diff=diff) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.field('group', 'version', 'plural', 'field.subfield', id='id', timeout=123, registry=registry, diff --git a/tests/registries/test_decorators.py b/tests/registries/test_decorators.py index be8c143e..b8cc8f60 100644 --- a/tests/registries/test_decorators.py +++ b/tests/registries/test_decorators.py @@ -262,7 +262,7 @@ def test_on_resume_with_all_kwargs(mocker, reason): cause = mocker.MagicMock(resource=resource, reason=reason, initial=True, deleted=False) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.resume('group', 'version', 'plural', id='id', registry=registry, @@ -297,7 +297,7 @@ def test_on_create_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.CREATE) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.create('group', 'version', 'plural', id='id', registry=registry, @@ -330,7 +330,7 @@ def test_on_update_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.update('group', 'version', 'plural', id='id', registry=registry, @@ -367,7 +367,7 @@ def test_on_delete_with_all_kwargs(mocker, optional): cause = mocker.MagicMock(resource=resource, reason=Reason.DELETE) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.delete('group', 'version', 'plural', id='id', registry=registry, @@ -402,7 +402,7 @@ def test_on_field_with_all_kwargs(mocker): cause = mocker.MagicMock(resource=resource, reason=Reason.UPDATE, diff=diff) mocker.patch('kopf.reactor.registries.match', return_value=True) - when = lambda body: False + when = lambda **_: False @kopf.on.field('group', 'version', 'plural', 'field.subfield', id='id', registry=registry,