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

CalcJob: do not pause when exception thrown in the presubmit #3699

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Dec 20, 2019

Fixes #3687

The presubmit call of the CalcJob has recently been moved to the
aiida.engine.processes.calcjobs.tasks.task_upload_job which is wrapped
in the exponential backoff mechanism. The latter was introduced to
recover from transient problems such as connection problems during the
actual upload to a remote machine. However it should not catch exception
from the presubmit call which are not actually transient and thus not
automatically recoverable. In this case the process should simply
except.

Here we test this by mocking the presubmit to raise an exception and
check that it is bubbled up and the process does not end up in a paused
state. To prevent the test from blocking in the case the process gets
put in the paused state erroneously, we put a timeout on the test for
which we need the pytest-timeout plugin.

@sphuber sphuber force-pushed the fix_3687_prepare_for_submission_pause branch from ea4c8f3 to 0def833 Compare December 20, 2019 15:38
The `presubmit` call of the `CalcJob` has recently been moved to the
`aiida.engine.processes.calcjobs.tasks.task_upload_job` which is wrapped
in the exponential backoff mechanism. The latter was introduced to
recover from transient problems such as connection problems during the
actual upload to a remote machine. However it should not catch exception
from the `presubmit` call which are not actually transient and thus not
automatically recoverable. In this case the process should simply
except.

Here we test this by mocking the presubmit to raise an exception and
check that it is bubbled up and the process does not end up in a paused
state. To prevent the test from blocking in the case the process gets
put in the paused state erroneously, we put a timeout on the test for
which we need the `pytest-timeout` plugin.
@sphuber sphuber force-pushed the fix_3687_prepare_for_submission_pause branch from 0def833 to 157059a Compare December 20, 2019 15:58
@sphuber sphuber requested review from ltalirz and greschd December 20, 2019 16:18
@sphuber
Copy link
Contributor Author

sphuber commented Dec 20, 2019

I snuck in a change to the .pylintrc here...

Copy link
Member

@greschd greschd left a comment

Choose a reason for hiding this comment

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

Good stuff. Approve of the sneaky .pylintrc update as well. Prepare for the ultimate table flip when it still generates pre-commit errors.

@sphuber
Copy link
Contributor Author

sphuber commented Dec 20, 2019

Good stuff. Approve of the sneaky .pylintrc update as well. Prepare for the ultimate table flip when it still generates pre-commit errors.

I am sitting cross-legged on the floor as I type this, all out of tables to flip

@sphuber sphuber merged commit c3c9aaf into aiidateam:develop Dec 20, 2019
@sphuber sphuber deleted the fix_3687_prepare_for_submission_pause branch December 20, 2019 16:30
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.

Error in 'prepare_for_submission' causes CalcJob to pause
2 participants