Restore proper mutability implementation of Node attributes#1181
Merged
giovannipizzi merged 13 commits intoFeb 22, 2018
Merged
Conversation
…_release Enable the Trusty platform and postgres 9.5 on Travis
The check for node attribute immutability, i.e. when a node is stored its attributes are to be immutable, was moved out of the base Node class and moved to the Data class and the SealableWithUpdatableAttributes mixin, in order to allow for Calculation attributes that needed to be mutable even after storing the node, e.g. the calculation state. However, the logic was improperly implemented and not applied everywhere leaving certain nodes with all attributes mutable even after being stored. To prevent this error from happening in the future, we move the check back to the Node class. The calculation classes will have to override this behavior in the future to allow for updatable attributes
The attribues of the Node class become immutable once the node is stored. However, for some Node subclasses one needs to allow for certain attributes to be updatable even after storing. An example is the Calculation class. For it to run, it needs to be stored, however its running state is stored in an attribute which needs to be updated despite the underlying node already being stored. To solve this we introduce the Sealable mixin, which can defined a tuple of updatable attributes that are mutable even when the node is stored. It also provides the 'sealed' attribute which when set even the updatable attributes become immutable
…utes This was already done in PR aiidateam#652 that was merged into develop but needs to be done in this branch as well for consistency, which will be merged into the v0.11.1 patch release
The 'input_plugin', 'append_text' and 'prepend_text' attributes should not really be updatable as they can really alter the behavior of the code. For that reason we remove them from the updatable_attributes. The 'hidden' attribute can be turned into an extra, as it really is more meta-data that is only pertinent to the user that owns the code. That leaves no updatable attributes for this class, which means the Sealable mixin can be removed.
The -a switch for verdi code list was broken because it filtered on the hidden attribute in a hardcoded way and just checked if the hidden property was not True. However if it is not set at all, which is the default, then it should also be ignored.
sphuber
force-pushed
the
fix_1109_mutable_attributes_workflows_branch
branch
2 times, most recently
from
aaab0f1 to
fa00b5f
Compare
To make sure that it properly extends the mutable attributes from AbstractCalculation, we subclass from AbstractCalculation instead of just object
…ibutes_workflows_branch
sphuber
force-pushed
the
fix_1109_mutable_attributes_workflows_branch
branch
from
fa00b5f to
4862307
Compare
giovannipizzi
approved these changes
Feb 22, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1109
The concept of attribute immutability has been properly reinstated. The current conception of node attribute mutability is the following.
Sealablemixin are immutable when the node is sealed or when stored and the attribute is not in the_updatable_attributestupleTo enforce this behavior, the
is_storedcheck is moved back into the_set_attrand_del_attrmethods of theAbstractNodeclass. TheSealablemixin, allows theAbstractCalculationandWorkCalculationsubclasses of Node to define a tuple of_updatable_attributesthat can be mutated as long as the node is not sealed.The updatable attributes that were defined for the
Codeclass,input_plugin,prepend_text,append_textandhiddenhave been removed, allowing theSealablemixin to be removed. The first three really should be immutable as changing them can really affect the outcome of calculations run with that code. Thehiddenattribute is more a meta data attribute, specific to the user that owns the code and therefore better belongs in the extras of the node