From e19bdce2b01c352d37254acadc055517bd9b94a2 Mon Sep 17 00:00:00 2001 From: Sergey Vasilyev Date: Sat, 27 Mar 2021 10:14:41 +0100 Subject: [PATCH 1/4] Unquote callbacks as unspecific callables at runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a recent commit, callbacks were properly defined, but this required MyPy extensions (to be released in Python & typing_extensions on Python 3.10 release, presumably Oct'2021). The existence of these extensions required a conditional to avoid installing them at runtime (where they are not needed). Thus, in turn, required quoting (ForwardRef'ing) all references to callbacks. As a result, this broke IDE navigation, as PyCharm does not understand "quoted" types. To fix that, a little improvement is added: for runtime, define the same callback protocols, but with no arguments specified — make them just callables. This allows us to unquote the callback types and to use them both at runtime and type-checking time naturally. Signed-off-by: Sergey Vasilyev --- kopf/on.py | 94 +++++++++++++++++++------------------- kopf/reactor/handling.py | 2 +- kopf/reactor/registries.py | 32 ++++++------- kopf/structs/callbacks.py | 43 ++++++----------- kopf/structs/filters.py | 9 ++-- kopf/structs/handlers.py | 14 +++--- 6 files changed, 86 insertions(+), 108 deletions(-) diff --git a/kopf/on.py b/kopf/on.py index aadaeebc..24ee0931 100644 --- a/kopf/on.py +++ b/kopf/on.py @@ -16,12 +16,12 @@ def creation_handler(**kwargs): from kopf.reactor import handling, registries from kopf.structs import callbacks, dicts, filters, handlers, references -ActivityDecorator = Callable[["callbacks.ActivityFn"], "callbacks.ActivityFn"] -ResourceIndexingDecorator = Callable[["callbacks.ResourceIndexingFn"], "callbacks.ResourceIndexingFn"] -ResourceWatchingDecorator = Callable[["callbacks.ResourceWatchingFn"], "callbacks.ResourceWatchingFn"] -ResourceChangingDecorator = Callable[["callbacks.ResourceChangingFn"], "callbacks.ResourceChangingFn"] -ResourceDaemonDecorator = Callable[["callbacks.ResourceDaemonFn"], "callbacks.ResourceDaemonFn"] -ResourceTimerDecorator = Callable[["callbacks.ResourceTimerFn"], "callbacks.ResourceTimerFn"] +ActivityDecorator = Callable[[callbacks.ActivityFn], callbacks.ActivityFn] +ResourceIndexingDecorator = Callable[[callbacks.ResourceIndexingFn], callbacks.ResourceIndexingFn] +ResourceWatchingDecorator = Callable[[callbacks.ResourceWatchingFn], callbacks.ResourceWatchingFn] +ResourceChangingDecorator = Callable[[callbacks.ResourceChangingFn], callbacks.ResourceChangingFn] +ResourceDaemonDecorator = Callable[[callbacks.ResourceDaemonFn], callbacks.ResourceDaemonFn] +ResourceTimerDecorator = Callable[[callbacks.ResourceTimerFn], callbacks.ResourceTimerFn] def startup( # lgtm[py/similar-function] @@ -37,8 +37,8 @@ def startup( # lgtm[py/similar-function] registry: Optional[registries.OperatorRegistry] = None, ) -> ActivityDecorator: def decorator( # lgtm[py/similar-function] - fn: "callbacks.ActivityFn", - ) -> "callbacks.ActivityFn": + fn: callbacks.ActivityFn, + ) -> callbacks.ActivityFn: real_registry = registry if registry is not None else registries.get_default_registry() real_id = registries.generate_id(fn=fn, id=id) handler = handlers.ActivityHandler( @@ -64,8 +64,8 @@ def cleanup( # lgtm[py/similar-function] registry: Optional[registries.OperatorRegistry] = None, ) -> ActivityDecorator: def decorator( # lgtm[py/similar-function] - fn: "callbacks.ActivityFn", - ) -> "callbacks.ActivityFn": + fn: callbacks.ActivityFn, + ) -> callbacks.ActivityFn: real_registry = registry if registry is not None else registries.get_default_registry() real_id = registries.generate_id(fn=fn, id=id) handler = handlers.ActivityHandler( @@ -92,8 +92,8 @@ def login( # lgtm[py/similar-function] ) -> ActivityDecorator: """ ``@kopf.on.login()`` handler for custom (re-)authentication. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ActivityFn", - ) -> "callbacks.ActivityFn": + fn: callbacks.ActivityFn, + ) -> callbacks.ActivityFn: real_registry = registry if registry is not None else registries.get_default_registry() real_id = registries.generate_id(fn=fn, id=id) handler = handlers.ActivityHandler( @@ -120,8 +120,8 @@ def probe( # lgtm[py/similar-function] ) -> ActivityDecorator: """ ``@kopf.on.probe()`` handler for arbitrary liveness metrics. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ActivityFn", - ) -> "callbacks.ActivityFn": + fn: callbacks.ActivityFn, + ) -> callbacks.ActivityFn: real_registry = registry if registry is not None else registries.get_default_registry() real_id = registries.generate_id(fn=fn, id=id) handler = handlers.ActivityHandler( @@ -158,7 +158,7 @@ def resume( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -166,8 +166,8 @@ def resume( # lgtm[py/similar-function] ) -> ResourceChangingDecorator: """ ``@kopf.on.resume()`` handler for the object resuming on operator (re)start. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -214,7 +214,7 @@ def create( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -222,8 +222,8 @@ def create( # lgtm[py/similar-function] ) -> ResourceChangingDecorator: """ ``@kopf.on.create()`` handler for the object creation. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -270,7 +270,7 @@ def update( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, old: Optional[filters.ValueFilter] = None, @@ -280,8 +280,8 @@ def update( # lgtm[py/similar-function] ) -> ResourceChangingDecorator: """ ``@kopf.on.update()`` handler for the object update or change. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: _warn_conflicting_values(field, value, old, new) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -329,7 +329,7 @@ def delete( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -337,8 +337,8 @@ def delete( # lgtm[py/similar-function] ) -> ResourceChangingDecorator: """ ``@kopf.on.delete()`` handler for the object deletion. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -385,7 +385,7 @@ def field( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: dicts.FieldSpec, value: Optional[filters.ValueFilter] = None, old: Optional[filters.ValueFilter] = None, @@ -395,8 +395,8 @@ def field( # lgtm[py/similar-function] ) -> ResourceChangingDecorator: """ ``@kopf.on.field()`` handler for the individual field changes. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: _warn_conflicting_values(field, value, old, new) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -443,7 +443,7 @@ def index( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -451,8 +451,8 @@ def index( # lgtm[py/similar-function] ) -> ResourceIndexingDecorator: """ ``@kopf.index()`` handler for the indexing callbacks. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceIndexingFn", - ) -> "callbacks.ResourceIndexingFn": + fn: callbacks.ResourceIndexingFn, + ) -> callbacks.ResourceIndexingFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -493,7 +493,7 @@ def event( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -501,8 +501,8 @@ def event( # lgtm[py/similar-function] ) -> ResourceWatchingDecorator: """ ``@kopf.on.event()`` handler for the silent spies on the events. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceWatchingFn", - ) -> "callbacks.ResourceWatchingFn": + fn: callbacks.ResourceWatchingFn, + ) -> callbacks.ResourceWatchingFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -551,7 +551,7 @@ def daemon( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -559,8 +559,8 @@ def daemon( # lgtm[py/similar-function] ) -> ResourceDaemonDecorator: """ ``@kopf.daemon()`` decorator for the background threads/tasks. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceDaemonFn", - ) -> "callbacks.ResourceDaemonFn": + fn: callbacks.ResourceDaemonFn, + ) -> callbacks.ResourceDaemonFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -613,7 +613,7 @@ def timer( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, # Operator specification: @@ -621,8 +621,8 @@ def timer( # lgtm[py/similar-function] ) -> ResourceTimerDecorator: """ ``@kopf.timer()`` handler for the regular events. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceTimerFn", - ) -> "callbacks.ResourceTimerFn": + fn: callbacks.ResourceTimerFn, + ) -> callbacks.ResourceTimerFn: _warn_conflicting_values(field, value) _verify_filters(labels, annotations) real_registry = registry if registry is not None else registries.get_default_registry() @@ -658,7 +658,7 @@ def subhandler( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, + when: Optional[callbacks.WhenFilterFn] = None, field: Optional[dicts.FieldSpec] = None, value: Optional[filters.ValueFilter] = None, old: Optional[filters.ValueFilter] = None, # only for on.update's subhandlers @@ -693,8 +693,8 @@ def create_task(*, spec, task=task, **kwargs): create function will have its own value, not the latest in the for-cycle. """ def decorator( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", - ) -> "callbacks.ResourceChangingFn": + fn: callbacks.ResourceChangingFn, + ) -> callbacks.ResourceChangingFn: parent_handler = handling.handler_var.get() if not isinstance(parent_handler, handlers.ResourceChangingHandler): raise TypeError("Sub-handlers are only supported for resource-changing handlers.") @@ -720,7 +720,7 @@ def decorator( # lgtm[py/similar-function] def register( # lgtm[py/similar-function] - fn: "callbacks.ResourceChangingFn", + fn: callbacks.ResourceChangingFn, *, # Handler's behaviour specification: id: Optional[str] = None, @@ -732,8 +732,8 @@ def register( # lgtm[py/similar-function] # Resource object specification: labels: Optional[filters.MetaFilter] = None, annotations: Optional[filters.MetaFilter] = None, - when: Optional["callbacks.WhenFilterFn"] = None, -) -> "callbacks.ResourceChangingFn": + when: Optional[callbacks.WhenFilterFn] = None, +) -> callbacks.ResourceChangingFn: """ Register a function as a sub-handler of the currently executed handler. diff --git a/kopf/reactor/handling.py b/kopf/reactor/handling.py index f9cb8ca1..1f556f31 100644 --- a/kopf/reactor/handling.py +++ b/kopf/reactor/handling.py @@ -62,7 +62,7 @@ class HandlerChildrenRetry(TemporaryError): async def execute( *, - fns: Optional[Iterable["callbacks.ResourceChangingFn"]] = None, + fns: Optional[Iterable[callbacks.ResourceChangingFn]] = None, handlers: Optional[Iterable[handlers_.ResourceChangingHandler]] = None, registry: Optional[registries.ResourceChangingRegistry] = None, lifecycle: Optional[lifecycles.LifeCycleFn] = None, diff --git a/kopf/reactor/registries.py b/kopf/reactor/registries.py index 01db3f9d..e2cc767f 100644 --- a/kopf/reactor/registries.py +++ b/kopf/reactor/registries.py @@ -16,26 +16,22 @@ import functools from types import FunctionType, MethodType from typing import Any, Callable, Collection, Container, Generic, Iterable, Iterator, List, \ - Mapping, MutableMapping, Optional, Sequence, Set, Tuple, TypeVar, cast, \ - TYPE_CHECKING + Mapping, MutableMapping, Optional, Sequence, Set, Tuple, TypeVar, cast from kopf.reactor import causation, invocation -from kopf.structs import dicts, filters, handlers, references +from kopf.structs import callbacks, dicts, filters, handlers, references from kopf.utilities import piggybacking -if TYPE_CHECKING: # pragma: nocover - from kopf.structs import callbacks - # We only type-check for known classes of handlers/callbacks, and ignore any custom subclasses. CauseT = TypeVar('CauseT', bound=causation.BaseCause) HandlerT = TypeVar('HandlerT', bound=handlers.BaseHandler) ResourceHandlerT = TypeVar('ResourceHandlerT', bound=handlers.ResourceHandler) HandlerFnT = TypeVar('HandlerFnT', - "callbacks.ActivityFn", - "callbacks.ResourceIndexingFn", - "callbacks.ResourceWatchingFn", - "callbacks.ResourceSpawningFn", - "callbacks.ResourceChangingFn") + callbacks.ActivityFn, + callbacks.ResourceIndexingFn, + callbacks.ResourceWatchingFn, + callbacks.ResourceSpawningFn, + callbacks.ResourceChangingFn) class GenericRegistry(Generic[HandlerFnT, HandlerT]): @@ -54,7 +50,7 @@ def get_all_handlers(self) -> Collection[HandlerT]: class ActivityRegistry(GenericRegistry[ - "callbacks.ActivityFn", + callbacks.ActivityFn, handlers.ActivityHandler]): def get_handlers( @@ -128,7 +124,7 @@ def iter_extra_fields( class ResourceIndexingRegistry(ResourceRegistry[ causation.ResourceIndexingCause, - "callbacks.ResourceIndexingFn", + callbacks.ResourceIndexingFn, handlers.ResourceIndexingHandler]): def iter_handlers( @@ -144,7 +140,7 @@ def iter_handlers( class ResourceWatchingRegistry(ResourceRegistry[ causation.ResourceWatchingCause, - "callbacks.ResourceWatchingFn", + callbacks.ResourceWatchingFn, handlers.ResourceWatchingHandler]): def iter_handlers( @@ -160,7 +156,7 @@ def iter_handlers( class ResourceSpawningRegistry(ResourceRegistry[ causation.ResourceSpawningCause, - "callbacks.ResourceSpawningFn", + callbacks.ResourceSpawningFn, handlers.ResourceSpawningHandler]): def iter_handlers( @@ -191,7 +187,7 @@ def requires_finalizer( class ResourceChangingRegistry(ResourceRegistry[ causation.ResourceChangingCause, - "callbacks.ResourceChangingFn", + callbacks.ResourceChangingFn, handlers.ResourceChangingHandler]): def iter_handlers( @@ -270,7 +266,7 @@ def __init__(self) -> None: else: self._activities.append(handlers.ActivityHandler( id=handlers.HandlerId('login_via_pykube'), - fn=cast("callbacks.ActivityFn", piggybacking.login_via_pykube), + fn=cast(callbacks.ActivityFn, piggybacking.login_via_pykube), activity=handlers.Activity.AUTHENTICATION, errors=handlers.ErrorsMode.IGNORED, param=None, timeout=None, retries=None, backoff=None, @@ -283,7 +279,7 @@ def __init__(self) -> None: else: self._activities.append(handlers.ActivityHandler( id=handlers.HandlerId('login_via_client'), - fn=cast("callbacks.ActivityFn", piggybacking.login_via_client), + fn=cast(callbacks.ActivityFn, piggybacking.login_via_client), activity=handlers.Activity.AUTHENTICATION, errors=handlers.ErrorsMode.IGNORED, param=None, timeout=None, retries=None, backoff=None, diff --git a/kopf/structs/callbacks.py b/kopf/structs/callbacks.py index 4d4ca9b6..024f7389 100644 --- a/kopf/structs/callbacks.py +++ b/kopf/structs/callbacks.py @@ -26,8 +26,17 @@ LoggerType = Union[logging.Logger, logging.LoggerAdapter] - -if TYPE_CHECKING: # pragma: nocover +if not TYPE_CHECKING: # pragma: nocover + # Define unspecified protocols for the runtime annotations -- to avoid "quoting". + ActivityFn = Callable[..., _SyncOrAsyncResult] + ResourceIndexingFn = Callable[..., _SyncOrAsyncResult] + ResourceWatchingFn = Callable[..., _SyncOrAsyncResult] + ResourceChangingFn = Callable[..., _SyncOrAsyncResult] + ResourceDaemonFn = Callable[..., _SyncOrAsyncResult] + ResourceTimerFn = Callable[..., _SyncOrAsyncResult] + WhenFilterFn = Callable[..., bool] + MetaFilterFn = Callable[..., bool] +else: from mypy_extensions import Arg, DefaultNamedArg, KwArg, NamedArg, VarArg # TODO: Try using ParamSpec to support index type checking in callbacks @@ -38,7 +47,6 @@ NamedArg(int, "retry"), NamedArg(datetime, "started"), NamedArg(timedelta, "runtime"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -47,7 +55,6 @@ _SyncOrAsyncResult ] - ResourceIndexingFn = Callable[ [ NamedArg(Dict[str, str], "labels"), @@ -61,7 +68,6 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -70,12 +76,10 @@ _SyncOrAsyncResult ] - ResourceWatchingFn = Callable[ [ NamedArg(str, "type"), NamedArg(bodies.RawEvent, "event"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -87,7 +91,6 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -96,13 +99,11 @@ _SyncOrAsyncResult ] - ResourceChangingFn = Callable[ [ NamedArg(int, "retry"), NamedArg(datetime, "started"), NamedArg(timedelta, "runtime"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -114,12 +115,10 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(str, "reason"), NamedArg(diffs.Diff, "diff"), NamedArg(Optional[Union[bodies.BodyEssence, Any]], "old"), NamedArg(Optional[Union[bodies.BodyEssence, Any]], "new"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -128,15 +127,12 @@ _SyncOrAsyncResult ] - ResourceDaemonFn = Callable[ [ NamedArg(primitives.SyncDaemonStopperChecker, "stopped"), - NamedArg(int, "retry"), NamedArg(datetime, "started"), NamedArg(timedelta, "runtime"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -148,7 +144,6 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -157,11 +152,9 @@ _SyncOrAsyncResult ] - ResourceTimerFn = Callable[ [ NamedArg(ephemera.Index, "*"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -173,7 +166,6 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -182,14 +174,10 @@ _SyncOrAsyncResult ] - - ResourceSpawningFn = Union[ResourceDaemonFn, ResourceTimerFn] - WhenFilterFn = Callable[ [ NamedArg(str, "type"), NamedArg(bodies.RawEvent, "event"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -201,11 +189,9 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(diffs.Diff, "diff"), NamedArg(Optional[Union[bodies.BodyEssence, Any]], "old"), NamedArg(Optional[Union[bodies.BodyEssence, Any]], "new"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -214,12 +200,10 @@ bool ] - MetaFilterFn = Callable[ [ Arg(Any, "value"), NamedArg(str, "type"), - NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -231,7 +215,6 @@ NamedArg(Optional[str], "name"), NamedArg(Optional[str], "namespace"), NamedArg(patches.Patch, "patch"), - NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -240,8 +223,8 @@ bool ] - -_FnT = TypeVar('_FnT', "WhenFilterFn", "MetaFilterFn") +ResourceSpawningFn = Union[ResourceDaemonFn, ResourceTimerFn] +_FnT = TypeVar('_FnT', WhenFilterFn, MetaFilterFn) def not_(fn: _FnT) -> _FnT: diff --git a/kopf/structs/filters.py b/kopf/structs/filters.py index e6ed067a..71d30db9 100644 --- a/kopf/structs/filters.py +++ b/kopf/structs/filters.py @@ -1,8 +1,7 @@ import enum -from typing import Any, Mapping, Union, TYPE_CHECKING +from typing import Any, Mapping, Union -if TYPE_CHECKING: # pragma: nocover - from kopf.structs import callbacks +from kopf.structs import callbacks class MetaFilterToken(enum.Enum): @@ -16,8 +15,8 @@ class MetaFilterToken(enum.Enum): PRESENT = MetaFilterToken.PRESENT # Filters for handler specifications (not the same as the object's values). -MetaFilter = Mapping[str, Union[str, MetaFilterToken, "callbacks.MetaFilterFn"]] +MetaFilter = Mapping[str, Union[str, MetaFilterToken, callbacks.MetaFilterFn]] # Filters for old/new values of a field. # NB: `Any` covers all other values, but we want to highlight that they are specially treated. -ValueFilter = Union[None, Any, MetaFilterToken, "callbacks.MetaFilterFn"] +ValueFilter = Union[None, Any, MetaFilterToken, callbacks.MetaFilterFn] diff --git a/kopf/structs/handlers.py b/kopf/structs/handlers.py index 27666218..8527ce9d 100644 --- a/kopf/structs/handlers.py +++ b/kopf/structs/handlers.py @@ -83,7 +83,7 @@ def __str__(self) -> str: @dataclasses.dataclass class ActivityHandler(BaseHandler): - fn: "callbacks.ActivityFn" # type clarification + fn: callbacks.ActivityFn # type clarification activity: Optional[Activity] _fallback: bool = False # non-public! @@ -96,7 +96,7 @@ class ResourceHandler(BaseHandler): selector: Optional[references.Selector] # None is used only in sub-handlers labels: Optional[filters.MetaFilter] annotations: Optional[filters.MetaFilter] - when: Optional["callbacks.WhenFilterFn"] + when: Optional[callbacks.WhenFilterFn] field: Optional[dicts.FieldPath] value: Optional[filters.ValueFilter] @@ -107,7 +107,7 @@ def requires_patching(self) -> bool: @dataclasses.dataclass class ResourceIndexingHandler(ResourceHandler): - fn: "callbacks.ResourceIndexingFn" # type clarification + fn: callbacks.ResourceIndexingFn # type clarification def __str__(self) -> str: return f"Indexer {self.id!r}" @@ -119,7 +119,7 @@ def requires_patching(self) -> bool: @dataclasses.dataclass class ResourceWatchingHandler(ResourceHandler): - fn: "callbacks.ResourceWatchingFn" # type clarification + fn: callbacks.ResourceWatchingFn # type clarification @property def requires_patching(self) -> bool: @@ -128,7 +128,7 @@ def requires_patching(self) -> bool: @dataclasses.dataclass class ResourceChangingHandler(ResourceHandler): - fn: "callbacks.ResourceChangingFn" # type clarification + fn: callbacks.ResourceChangingFn # type clarification reason: Optional[Reason] initial: Optional[bool] deleted: Optional[bool] # used for mixed-in (initial==True) @on.resume handlers only. @@ -146,7 +146,7 @@ class ResourceSpawningHandler(ResourceHandler): @dataclasses.dataclass class ResourceDaemonHandler(ResourceSpawningHandler): - fn: "callbacks.ResourceDaemonFn" # type clarification + fn: callbacks.ResourceDaemonFn # type clarification cancellation_backoff: Optional[float] # how long to wait before actual cancellation. cancellation_timeout: Optional[float] # how long to wait before giving up on cancellation. cancellation_polling: Optional[float] # how often to check for cancellation status. @@ -157,7 +157,7 @@ def __str__(self) -> str: @dataclasses.dataclass class ResourceTimerHandler(ResourceSpawningHandler): - fn: "callbacks.ResourceTimerFn" # type clarification + fn: callbacks.ResourceTimerFn # type clarification sharp: Optional[bool] idle: Optional[float] interval: Optional[float] From 3cd12eee90390b922b82fa1cb2afaacfb7541b50 Mon Sep 17 00:00:00 2001 From: Sergey Vasilyev Date: Sat, 27 Mar 2021 10:19:00 +0100 Subject: [PATCH 2/4] Apply Google Style Guide's importing notation Signed-off-by: Sergey Vasilyev --- kopf/structs/callbacks.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/kopf/structs/callbacks.py b/kopf/structs/callbacks.py index 024f7389..a3144995 100644 --- a/kopf/structs/callbacks.py +++ b/kopf/structs/callbacks.py @@ -4,8 +4,8 @@ Since these signatures contain a lot of copy-pasted kwargs and are not so important for the codebase, they are moved to this separate module. """ +import datetime import logging -from datetime import datetime, timedelta from typing import TYPE_CHECKING, Any, Callable, Collection, \ Coroutine, Dict, NewType, Optional, TypeVar, Union @@ -45,8 +45,8 @@ [ NamedArg(ephemera.Index, "*"), NamedArg(int, "retry"), - NamedArg(datetime, "started"), - NamedArg(timedelta, "runtime"), + NamedArg(datetime.datetime, "started"), + NamedArg(datetime.timedelta, "runtime"), NamedArg(LoggerType, "logger"), NamedArg(ephemera.AnyMemo, "memo"), DefaultNamedArg(Any, "param"), @@ -102,8 +102,8 @@ ResourceChangingFn = Callable[ [ NamedArg(int, "retry"), - NamedArg(datetime, "started"), - NamedArg(timedelta, "runtime"), + NamedArg(datetime.datetime, "started"), + NamedArg(datetime.timedelta, "runtime"), NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), @@ -131,8 +131,8 @@ [ NamedArg(primitives.SyncDaemonStopperChecker, "stopped"), NamedArg(int, "retry"), - NamedArg(datetime, "started"), - NamedArg(timedelta, "runtime"), + NamedArg(datetime.datetime, "started"), + NamedArg(datetime.timedelta, "runtime"), NamedArg(Dict[str, str], "labels"), NamedArg(Dict[str, str], "annotations"), NamedArg(bodies.Body, "body"), From ea4eba94c9d17cc14af9911a321b73bac160a807 Mon Sep 17 00:00:00 2001 From: Sergey Vasilyev Date: Sat, 27 Mar 2021 11:44:03 +0100 Subject: [PATCH 3/4] Export labels/annotations type declarations as readonly mappings Since annotations become readonly even in body essences, this requires stricter typing in storages. Signed-off-by: Sergey Vasilyev --- kopf/__init__.py | 4 ++++ kopf/storage/conventions.py | 29 +++++++++++++++++++++++++++++ kopf/storage/diffbase.py | 19 ++++++------------- kopf/storage/progress.py | 9 +++++---- kopf/structs/bodies.py | 21 +++++++++++++-------- kopf/structs/callbacks.py | 30 +++++++++++++++--------------- 6 files changed, 72 insertions(+), 40 deletions(-) diff --git a/kopf/__init__.py b/kopf/__init__.py index 922b8dfb..4d7a44eb 100644 --- a/kopf/__init__.py +++ b/kopf/__init__.py @@ -77,6 +77,8 @@ Meta, Body, BodyEssence, + Labels, + Annotations, OwnerReference, ObjectReference, build_object_reference, @@ -186,6 +188,8 @@ 'Meta', 'Body', 'BodyEssence', + 'Labels', + 'Annotations', 'ObjectReference', 'OwnerReference', 'Memo', 'Index', 'Store', diff --git a/kopf/storage/conventions.py b/kopf/storage/conventions.py index d0207102..71c5733d 100644 --- a/kopf/storage/conventions.py +++ b/kopf/storage/conventions.py @@ -250,3 +250,32 @@ def _store_marker( marker = f'{prefix}/kopf-managed' if marker not in body.metadata.annotations and marker not in patch.metadata.annotations: patch.metadata.annotations[marker] = value + + +class StorageStanzaCleaner: + """ + A mixin used internally to remove unwanted annotations and empty stanzas. + """ + + @staticmethod + def remove_annotations(essence: bodies.BodyEssence, keys_to_remove: Collection[str]) -> None: + """ Remove annotations (in-place). """ + current_keys = essence.get('metadata', {}).get('annotations', {}) + if frozenset(keys_to_remove) & frozenset(current_keys): + essence['metadata']['annotations'] = { + key: val + for key, val in essence.get('metadata', {}).get('annotations', {}).items() + if key not in keys_to_remove + } + + @staticmethod + def remove_empty_stanzas(essence: bodies.BodyEssence) -> None: + """ Remove (in-place) the parent structs/stanzas if they are empty. """ + if 'annotations' in essence.get('metadata', {}) and not essence['metadata']['annotations']: + del essence['metadata']['annotations'] + if 'labels' in essence.get('metadata', {}) and not essence['metadata']['labels']: + del essence['metadata']['labels'] + if 'metadata' in essence and not essence['metadata']: + del essence['metadata'] + if 'status' in essence and not essence['status']: + del essence['status'] diff --git a/kopf/storage/diffbase.py b/kopf/storage/diffbase.py index 7f1d37d7..37c35ac8 100644 --- a/kopf/storage/diffbase.py +++ b/kopf/storage/diffbase.py @@ -7,7 +7,9 @@ from kopf.structs import bodies, dicts, patches -class DiffBaseStorage(conventions.StorageKeyMarkingConvention, metaclass=abc.ABCMeta): +class DiffBaseStorage(conventions.StorageKeyMarkingConvention, + conventions.StorageStanzaCleaner, + metaclass=abc.ABCMeta): """ Store the base essence for diff calculations, i.e. last handled state. @@ -79,14 +81,7 @@ def build( # Restore all explicitly whitelisted extra-fields from the original body. dicts.cherrypick(src=body, dst=essence, fields=extra_fields, picker=copy.deepcopy) - # Cleanup the parent structs if they have become empty, for consistent essence comparison. - if 'annotations' in essence.get('metadata', {}) and not essence['metadata']['annotations']: - del essence['metadata']['annotations'] - if 'metadata' in essence and not essence['metadata']: - del essence['metadata'] - if 'status' in essence and not essence['status']: - del essence['status'] - + self.remove_empty_stanzas(cast(bodies.BodyEssence, essence)) return cast(bodies.BodyEssence, essence) @abc.abstractmethod @@ -127,10 +122,8 @@ def build( extra_fields: Optional[Iterable[dicts.FieldSpec]] = None, ) -> bodies.BodyEssence: essence = super().build(body=body, extra_fields=extra_fields) - annotations = essence.get('metadata', {}).get('annotations', {}) - for full_key in self.make_keys(self.key, body=body): - if full_key in annotations: - del annotations[full_key] + self.remove_annotations(essence, set(self.make_keys(self.key, body=body))) + self.remove_empty_stanzas(essence) return essence def fetch( diff --git a/kopf/storage/progress.py b/kopf/storage/progress.py index ec79ea5c..c4a98d2a 100644 --- a/kopf/storage/progress.py +++ b/kopf/storage/progress.py @@ -62,7 +62,7 @@ class ProgressRecord(TypedDict, total=True): subrefs: Optional[Collection[handlers.HandlerId]] -class ProgressStorage(metaclass=abc.ABCMeta): +class ProgressStorage(conventions.StorageStanzaCleaner, metaclass=abc.ABCMeta): """ Base class and an interface for all persistent states. @@ -237,9 +237,9 @@ def touch( def clear(self, *, essence: bodies.BodyEssence) -> bodies.BodyEssence: essence = super().clear(essence=essence) annotations = essence.get('metadata', {}).get('annotations', {}) - for name in list(annotations.keys()): - if self.prefix and name.startswith(f'{self.prefix}/'): - del annotations[name] + keys = {key for key in annotations if self.prefix and key.startswith(f'{self.prefix}/')} + self.remove_annotations(essence, keys) + self.remove_empty_stanzas(essence) return essence @@ -367,6 +367,7 @@ def clear(self, *, essence: bodies.BodyEssence) -> bodies.BodyEssence: essence_dict = cast(Dict[Any, Any], essence) dicts.remove(essence_dict, self.field) + self.remove_empty_stanzas(essence) return essence diff --git a/kopf/structs/bodies.py b/kopf/structs/bodies.py index ce47ec06..cb411725 100644 --- a/kopf/structs/bodies.py +++ b/kopf/structs/bodies.py @@ -41,12 +41,16 @@ def create_fn(*args, meta: kopf.Meta, **kwargs): and object-processing functions. The internal dicts will remain the same. """ -from typing import Any, List, Mapping, MutableMapping, Optional, Union, cast +from typing import Any, List, Mapping, Optional, Union, cast from typing_extensions import Literal, TypedDict from kopf.structs import dicts, references +# Make sure every kwarg has a corresponding same-named type in the root package. +Labels = Mapping[str, str] +Annotations = Mapping[str, str] + # # Everything marked "raw" is a plain unwrapped unprocessed data as JSON-decoded # from Kubernetes API, usually as retrieved in watching or fetching API calls. @@ -63,8 +67,8 @@ class RawMeta(TypedDict, total=False): uid: str name: str namespace: str - labels: Mapping[str, str] - annotations: Mapping[str, str] + labels: Labels + annotations: Annotations finalizers: List[str] resourceVersion: str deletionTimestamp: str @@ -111,13 +115,14 @@ class RawEvent(TypedDict, total=True): class MetaEssence(TypedDict, total=False): - labels: MutableMapping[str, str] - annotations: MutableMapping[str, str] + labels: Labels + annotations: Annotations class BodyEssence(TypedDict, total=False): metadata: MetaEssence - spec: MutableMapping[str, Any] + spec: Mapping[str, Any] + status: Mapping[str, Any] # @@ -136,11 +141,11 @@ def __init__(self, __src: "Body") -> None: self._annotations: dicts.MappingView[str, str] = dicts.MappingView(self, 'annotations') @property - def labels(self) -> dicts.MappingView[str, str]: + def labels(self) -> Labels: return self._labels @property - def annotations(self) -> dicts.MappingView[str, str]: + def annotations(self) -> Annotations: return self._annotations @property diff --git a/kopf/structs/callbacks.py b/kopf/structs/callbacks.py index a3144995..ddaf79ce 100644 --- a/kopf/structs/callbacks.py +++ b/kopf/structs/callbacks.py @@ -7,7 +7,7 @@ import datetime import logging from typing import TYPE_CHECKING, Any, Callable, Collection, \ - Coroutine, Dict, NewType, Optional, TypeVar, Union + Coroutine, NewType, Optional, TypeVar, Union from kopf.structs import bodies, diffs, ephemera, patches, primitives, references @@ -57,8 +57,8 @@ ResourceIndexingFn = Callable[ [ - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -80,8 +80,8 @@ [ NamedArg(str, "type"), NamedArg(bodies.RawEvent, "event"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -104,8 +104,8 @@ NamedArg(int, "retry"), NamedArg(datetime.datetime, "started"), NamedArg(datetime.timedelta, "runtime"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -133,8 +133,8 @@ NamedArg(int, "retry"), NamedArg(datetime.datetime, "started"), NamedArg(datetime.timedelta, "runtime"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -155,8 +155,8 @@ ResourceTimerFn = Callable[ [ NamedArg(ephemera.Index, "*"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -178,8 +178,8 @@ [ NamedArg(str, "type"), NamedArg(bodies.RawEvent, "event"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), @@ -204,8 +204,8 @@ [ Arg(Any, "value"), NamedArg(str, "type"), - NamedArg(Dict[str, str], "labels"), - NamedArg(Dict[str, str], "annotations"), + NamedArg(bodies.Annotations, "annotations"), + NamedArg(bodies.Labels, "labels"), NamedArg(bodies.Body, "body"), NamedArg(bodies.Meta, "meta"), NamedArg(bodies.Spec, "spec"), From ea05caa6eb8e66321e4d9e4c17b50eba1ff15737 Mon Sep 17 00:00:00 2001 From: Sergey Vasilyev Date: Sat, 27 Mar 2021 11:50:44 +0100 Subject: [PATCH 4/4] Accept both sync & async daemon stoppers in protocols Signed-off-by: Sergey Vasilyev --- kopf/structs/callbacks.py | 2 +- kopf/structs/primitives.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/kopf/structs/callbacks.py b/kopf/structs/callbacks.py index ddaf79ce..a0e19b22 100644 --- a/kopf/structs/callbacks.py +++ b/kopf/structs/callbacks.py @@ -129,7 +129,7 @@ ResourceDaemonFn = Callable[ [ - NamedArg(primitives.SyncDaemonStopperChecker, "stopped"), + NamedArg(primitives.SyncAsyncDaemonStopperChecker, "stopped"), NamedArg(int, "retry"), NamedArg(datetime.datetime, "started"), NamedArg(datetime.timedelta, "runtime"), diff --git a/kopf/structs/primitives.py b/kopf/structs/primitives.py index 14087273..a809dd2e 100644 --- a/kopf/structs/primitives.py +++ b/kopf/structs/primitives.py @@ -353,6 +353,11 @@ async def wait(self, timeout: Optional[float] = None) -> bool: return bool(self) +# Having this union allows both sync & async checkers in the same protocol, +# while not restricting the use of `wait()` as if the base class would be used. +SyncAsyncDaemonStopperChecker = Union[SyncDaemonStopperChecker, AsyncDaemonStopperChecker] + + async def sleep_or_wait( delays: Union[None, float, Collection[Union[None, float]]], wakeup: Optional[Union[asyncio.Event, DaemonStopper]] = None,