Skip to content

Commit

Permalink
BaseRestartWorkChain: fix handler_overrides ignoring `enabled=Fal…
Browse files Browse the repository at this point in the history
…se` (#5598)

In commit `7b8c61d0869751455993891cfe5cf6c637593344`, the input that
allows overriding handlers, `handler_overrides`, was updated to include
the possibility of overriding the priority. The changes broke the
original behavior though where a handler that was disabled by default
could not be enabled by specifying `enabled=False` in the overrides.
  • Loading branch information
sphuber authored Jul 14, 2022
1 parent 974ace2 commit a0c0699
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
2 changes: 1 addition & 1 deletion aiida/engine/processes/workchains/restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def get_process_handlers_by_priority(self) -> List[Tuple[int, FunctionType]]:
enabled = overrides.pop('enabled', None)
priority = overrides.pop('priority', None)

if enabled is False or not handler.enabled: # type: ignore[attr-defined]
if enabled is False or (enabled is None and not handler.enabled): # type: ignore[attr-defined]
continue

handlers.append((priority or handler.priority, handler)) # type: ignore[attr-defined]
Expand Down
10 changes: 9 additions & 1 deletion tests/engine/processes/workchains/test_restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ def handler_a(self, node):
def handler_b(self, _):
return

@engine.process_handler(priority=0, enabled=False)
def handler_c(self, _):
return

def not_a_handler(self, _):
pass

Expand All @@ -46,12 +50,14 @@ def test_is_process_handler():

def test_get_process_handlers():
"""Test the `BaseRestartWorkChain.get_process_handlers` class method."""
assert [handler.__name__ for handler in SomeWorkChain.get_process_handlers()] == ['handler_a', 'handler_b']
assert [handler.__name__ for handler in SomeWorkChain.get_process_handlers()
] == ['handler_a', 'handler_b', 'handler_c']


# yapf: disable
@pytest.mark.parametrize('inputs, priorities', (
({}, [100, 200]),
({'handler_overrides': {'handler_c': {'enabled': True}}}, [0, 100, 200]),
({'handler_overrides': {'handler_a': {'priority': 50}}}, [50, 100]),
({'handler_overrides': {'handler_a': {'enabled': False}}}, [100]),
({'handler_overrides': {'handler_a': False}}, [100]), # This notation is deprecated
Expand All @@ -67,8 +73,10 @@ def test_get_process_handlers_by_priority(generate_work_chain, inputs, prioritie
# Verify the actual handlers on the class haven't been modified
assert getattr(SomeWorkChain, 'handler_a').priority == 200
assert getattr(SomeWorkChain, 'handler_b').priority == 100
assert getattr(SomeWorkChain, 'handler_c').priority == 0
assert getattr(SomeWorkChain, 'handler_a').enabled
assert getattr(SomeWorkChain, 'handler_b').enabled
assert not getattr(SomeWorkChain, 'handler_c').enabled


@pytest.mark.requires_rmq
Expand Down

0 comments on commit a0c0699

Please sign in to comment.