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

Prevent launching or construction of WorkChain and CalcJob base class #2564

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Mar 4, 2019

Fixes #2493 and fixes #2474

The WorkChain and CalcJob classes are base classes that are meant to
be sub classed as they miss the necessary functionality to successfully
run. However, it was still possible to launch these base classes through
the launchers or straight up constructing them. They would immediately
fail when launched, but they would create a node in the provenance graph
nonetheless. To prevent this from happening, a check is placed in the
constructors to make sure that it is actually a sub class that is being
constructed.

@coveralls
Copy link

coveralls commented Mar 4, 2019

Coverage Status

Coverage increased (+0.007%) to 70.321% when pulling cc02f2b on sphuber:fix_2493_prevent_base_process_classes_submit into 76c9037 on aiidateam:provenance_redesign.


run_and_check_success(Wf)
# def test_run(self):
Copy link
Member

Choose a reason for hiding this comment

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

Is this huge number of lines replaced with comments expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I forgot to uncomment, I will fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now

@sphuber sphuber force-pushed the fix_2493_prevent_base_process_classes_submit branch from 57beac6 to 0cefc70 Compare March 4, 2019 21:26
Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

ok for me even if I'm not sure on what's going on with the tests

@sphuber
Copy link
Contributor Author

sphuber commented Mar 4, 2019

I canceled them since I will have to rebase this anyway after the others are merged, and so I am not blocking the build queue

…lass

The `WorkChain` and `CalcJob` classes are base classes that are meant to
be sub classed as they miss the necessary functionality to successfully
run. However, it was still possible to launch these base classes through
the launchers or straight up constructing them. They would immediately
fail when launched, but they would create a node in the provenance graph
nonetheless. To prevent this from happening, a check is placed in the
constructors to make sure that it is actually a sub class that is being
constructed.
@sphuber sphuber force-pushed the fix_2493_prevent_base_process_classes_submit branch from 0cefc70 to cc02f2b Compare March 4, 2019 22:10
@sphuber sphuber merged commit 28b0c82 into aiidateam:provenance_redesign Mar 4, 2019
@sphuber sphuber deleted the fix_2493_prevent_base_process_classes_submit branch March 4, 2019 22:50
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