Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure the type of exit codes is None, int or ExitCode #1681

Merged
merged 1 commit into from
Jun 22, 2018

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Jun 22, 2018

Fixes #1679

There was a bug in the execute method of the Waiting state of the
JobProcess, that in the case of a TransportException, which would
occur in the case of a SUBMISSIONFAILED or RETRIEVALFAILED, the
returned exit code was a JobCalculationExitStatus enum instance
and not the required integer or ExitCode. This would except the
process due to the check in the Process.on_finish call.

A similar problem existed in the execmanager.parse_results method
which also did not guarantee the return of an exit code of a valid
type. We now make sure to convert the bool, integer or ExitCode
which can be returned by parse_results is converted to an ExitCode
before it is returned.

@sphuber sphuber requested a review from muhrin June 22, 2018 13:51
@codecov-io
Copy link

codecov-io commented Jun 22, 2018

Codecov Report

Merging #1681 into develop will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1681      +/-   ##
===========================================
- Coverage    57.16%   57.15%   -0.02%     
===========================================
  Files          275      275              
  Lines        33911    33917       +6     
===========================================
  Hits         19385    19385              
- Misses       14526    14532       +6
Impacted Files Coverage Δ
aiida/daemon/execmanager.py 8.01% <0%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adefb22...f658c4b. Read the comment docs.

elif isinstance(exit_code, ExitCode):
pass
else:
raise ValueError("parse_from_calc returned an 'exit_code' of invalid_type: {}. It should"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This string shouldhave a space after shouldotherwise it will print shouldreturn when what it shouldsay is 'should return'...shouldn'tit...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it should

There was a bug in the execute method of the Waiting state of the
JobProcess, that in the case of a TransportException, which would
occur in the case of a SUBMISSIONFAILED or RETRIEVALFAILED, the
returned exit code was a JobCalculationExitStatus enum instance
and not the required integer or ExitCode. This would except the
process due to the check in the Process.on_finish call.

A similar problem existed in the execmanager.parse_results method
which also did not guarantee the return of an exit code of a valid
type. We now make sure to convert the bool, integer or ExitCode
which can be returned by parse_results is converted to an ExitCode
before it is returned.
@sphuber sphuber force-pushed the fix_1679_ensure_type_exit_codes branch from dd5d34f to f658c4b Compare June 22, 2018 14:45
@sphuber sphuber merged commit 695b548 into aiidateam:develop Jun 22, 2018
@sphuber sphuber deleted the fix_1679_ensure_type_exit_codes branch June 22, 2018 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants