Ensure WorkChain does not exit unless stepper returns non-zero value #1945
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1945 +/- ##
========================================
Coverage 67.55% 67.55%
========================================
Files 321 321
Lines 33275 33275
========================================
Hits 22479 22479
Misses 10796 10796Continue to review full report at Codecov.
|
aiida/work/workchain.py
Outdated
| @@ -156,13 +156,18 @@ def _do_step(self): | |||
| except _PropagateReturn as exception: | |||
| finished, result = True, exception.exit_code | |||
| else: | |||
| if isinstance(stepper_result, (int, ExitCode)): | |||
| # Set result to None unless stepper_result was non-zero integer or ExitCode instance with non-zero status | |||
| if isinstance(stepper_result, int) and stepper_result != 0: | |||
There was a problem hiding this comment.
Is this the right way to check also in py2/py3? (e.g. long in py2, ...)
There was a problem hiding this comment.
No clue to be honest. Surely @dev-zero would be able to tell us? :)
There was a problem hiding this comment.
if you suspect that stepper_result could be a long in Python 2, the future-proof ways to check are:
import six
if isinstance(stepper_result, six.integer_types) and stepper_result != 0:or
import numbers # Python built-in since 2.6
if isinstance(stepper_result, numbers.Integral) and stepper_result != 0:but for stepper_result to be a long in Python 2, you either have to set it explicitly using long(123), the ...L literal suffix or that it grows bigger than sys.maxint (which is in Python 2: 9223372036854775807) by itself.
There was a problem hiding this comment.
Since it can be a value that is returned by a user (return value from a WorkChain step function) and the spec is that it should allow positive integers greater than zero (which therefore technically should include longs), I will go with the six variant. Thanks @dev-zero
The engine exposes the possibility to the user of aborting a running workchain from within its outline step functions, by simply returning a non-zero integer or `ExitCode` instance with a non-zero status. Any other return value should be ignored in terms of aborting the running of the `WorkChain`. Due to a logical flaw in the `_do_step` implementation of the `WorkChain` this heuristic was not being respected and returning an `ExitCode` instance with zero status, would cause the `WorkChain` to exit the control flow.
sphuber
force-pushed
the
fix_1944_workchain_exits_when_it_should_not
branch
from
941e238 to
bb3b4ba
Compare
Fixes #1944
The engine exposes the possibility to the user of aborting a running
workchain from within its outline step functions, by simply returning
a non-zero integer or
ExitCodeinstance with a non-zero status.Any other return value should be ignored in terms of aborting the
running of the
WorkChain. Due to a logical flaw in the_do_stepimplementation of the
WorkChainthis heuristic was not beingrespected and returning an
ExitCodeinstance with zero status, wouldcause the
WorkChainto exit the control flow.