Skip to content

Commit

Permalink
Stricten handlers' defaults, require values/Nones explicitly
Browse files Browse the repository at this point in the history
In order to not get caught in a situation when an arg is forgotten,
which can cause misbehaviour or errors. The handlers are hidden
from the users, so we have no need to keep their interfaces simple.
  • Loading branch information
nolar committed Feb 20, 2020
1 parent 197856e commit 1ad5f29
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 9 deletions.
10 changes: 8 additions & 2 deletions kopf/on.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
reason=None, initial=True, deleted=deleted,
initial=True, deleted=deleted, requires_finalizer=None,
reason=None,
)
real_registry.resource_changing_handlers[real_resource].append(handler)
return fn
Expand Down Expand Up @@ -176,6 +177,7 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=None,
reason=causation.Reason.CREATE,
)
real_registry.resource_changing_handlers[real_resource].append(handler)
Expand Down Expand Up @@ -206,6 +208,7 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=None,
reason=causation.Reason.UPDATE,
)
real_registry.resource_changing_handlers[real_resource].append(handler)
Expand Down Expand Up @@ -237,8 +240,8 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=bool(not optional),
reason=causation.Reason.DELETE,
requires_finalizer=bool(not optional),
)
real_registry.resource_changing_handlers[real_resource].append(handler)
return fn
Expand Down Expand Up @@ -270,6 +273,7 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=real_field,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=None,
reason=None,
)
real_registry.resource_changing_handlers[real_resource].append(handler)
Expand All @@ -295,6 +299,7 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=None, timeout=None, retries=None, backoff=None, cooldown=None,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=None,
reason=None,
)
real_registry.resource_watching_handlers[real_resource].append(handler)
Expand Down Expand Up @@ -355,6 +360,7 @@ def decorator(fn: callbacks.ResourceHandlerFn) -> callbacks.ResourceHandlerFn:
fn=fn, id=real_id, field=None,
errors=errors, timeout=timeout, retries=retries, backoff=backoff, cooldown=cooldown,
labels=labels, annotations=annotations, when=when,
initial=None, deleted=None, requires_finalizer=None,
reason=None,
)
real_registry.append(handler)
Expand Down
14 changes: 7 additions & 7 deletions kopf/reactor/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def __getattribute__(self, name: str) -> Any:
@dataclasses.dataclass
class ActivityHandler(BaseHandler):
fn: callbacks.ActivityHandlerFn # type clarification
activity: Optional[causation.Activity] = None
activity: Optional[causation.Activity]
_fallback: bool = False # non-public!


Expand All @@ -55,12 +55,12 @@ class ResourceHandler(BaseHandler):
fn: callbacks.ResourceHandlerFn # type clarification
reason: Optional[causation.Reason]
field: Optional[dicts.FieldPath]
initial: Optional[bool] = None
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[callbacks.WhenHandlerFn] = None
requires_finalizer: Optional[bool] = None
initial: Optional[bool]
deleted: Optional[bool] # used for mixed-in (initial==True) @on.resume handlers only.
labels: Optional[bodies.Labels]
annotations: Optional[bodies.Annotations]
when: Optional[callbacks.WhenHandlerFn]
requires_finalizer: Optional[bool]

@property
def event(self) -> Optional[causation.Reason]:
Expand Down
4 changes: 4 additions & 0 deletions kopf/reactor/handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ async def execute(
handler = handlers_.ResourceHandler(
fn=fn, id=real_id,
errors=None, timeout=None, retries=None, backoff=None, cooldown=None,
labels=None, annotations=None, when=None,
initial=None, deleted=None, requires_finalizer=None,
reason=None, field=None,
)
subregistry.append(handler)
Expand All @@ -121,6 +123,8 @@ async def execute(
handler = handlers_.ResourceHandler(
fn=fn, id=real_id,
errors=None, timeout=None, retries=None, backoff=None, cooldown=None,
labels=None, annotations=None, when=None,
initial=None, deleted=None, requires_finalizer=None,
reason=None, field=None,
)
subregistry.append(handler)
Expand Down
6 changes: 6 additions & 0 deletions tests/basic-structs/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ def test_resource_handler_with_all_args(mocker):
retries = mocker.Mock()
backoff = mocker.Mock()
initial = mocker.Mock()
deleted = mocker.Mock()
labels = mocker.Mock()
annotations = mocker.Mock()
when = mocker.Mock()
requires_finalizer = mocker.Mock()
handler = ResourceHandler(
fn=fn,
Expand All @@ -60,8 +62,10 @@ def test_resource_handler_with_all_args(mocker):
backoff=backoff,
cooldown=None, # deprecated, but still required
initial=initial,
deleted=deleted,
labels=labels,
annotations=annotations,
when=when,
requires_finalizer=requires_finalizer,
)
assert handler.fn is fn
Expand All @@ -73,8 +77,10 @@ def test_resource_handler_with_all_args(mocker):
assert handler.retries is retries
assert handler.backoff is backoff
assert handler.initial is initial
assert handler.deleted is deleted
assert handler.labels is labels
assert handler.annotations is annotations
assert handler.when is when
assert handler.requires_finalizer is requires_finalizer

with pytest.deprecated_call(match=r"use handler.reason"):
Expand Down
6 changes: 6 additions & 0 deletions tests/basic-structs/test_handlers_deprecated_cooldown.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ def test_resource_handler_with_deprecated_cooldown_instead_of_backoff(mocker):
retries = mocker.Mock()
backoff = mocker.Mock()
initial = mocker.Mock()
deleted = mocker.Mock()
labels = mocker.Mock()
annotations = mocker.Mock()
when = mocker.Mock()
requires_finalizer = mocker.Mock()

with pytest.deprecated_call(match=r"use backoff="):
Expand All @@ -63,8 +65,10 @@ def test_resource_handler_with_deprecated_cooldown_instead_of_backoff(mocker):
backoff=None,
cooldown=backoff, # deprecated, but still required
initial=initial,
deleted=deleted,
labels=labels,
annotations=annotations,
when=when,
requires_finalizer=requires_finalizer,
)

Expand All @@ -77,6 +81,8 @@ def test_resource_handler_with_deprecated_cooldown_instead_of_backoff(mocker):
assert handler.retries is retries
assert handler.backoff is backoff
assert handler.initial is initial
assert handler.deleted is deleted
assert handler.labels is labels
assert handler.annotations is annotations
assert handler.when is when
assert handler.requires_finalizer is requires_finalizer
2 changes: 2 additions & 0 deletions tests/registries/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,7 @@ def parent_fn(**_):
return ResourceHandler(
fn=parent_fn, id=HandlerId('parent_fn'),
errors=None, retries=None, timeout=None, backoff=None, cooldown=None,
labels=None, annotations=None, when=None,
initial=None, deleted=None, requires_finalizer=None,
reason=None, field=None,
)

0 comments on commit 1ad5f29

Please sign in to comment.