Added load_plugin_safe method to return calc node if plugin is not available #1185
Conversation
aiida/common/pluginloader.py
Outdated
| @@ -125,6 +125,48 @@ def get_plugin(category, name): | |||
|
|
|||
| return plugin | |||
|
|
|||
| def load_plugin_safe(base_class, plugins_module, plugin_type): | |||
| """ | |||
| It is a copy of load_plugin function to return closely related node class | |||
There was a problem hiding this comment.
I would not say it is a copy, I would say it is a 'wrapper' and mention that it does not raise exceptions
aiida/common/pluginloader.py
Outdated
| def load_plugin_safe(base_class, plugins_module, plugin_type): | ||
| """ | ||
| It is a copy of load_plugin function to return closely related node class | ||
| if plugin is not available. We are duplicating load_plugin function to not break |
There was a problem hiding this comment.
You can remove this sentence "We are duplicating load_plugin function to not break its default behaviour." (as above, it's not a copy).
aiida/common/pluginloader.py
Outdated
| elif subNodeType == "work": | ||
| PluginClass = load_plugin(base_class, plugins_module, 'calculation.work.WorkCalculation') | ||
| else: | ||
| PluginClass = load_plugin(base_class, plugins_module, 'calculation.Calculation') |
There was a problem hiding this comment.
missing else (for baseNodeType). If there is a node of a different baseNodeType, this will crash as PluginClass is not defined. This e.g. will happen for Node (that starts with node.Node. I think).
Instead, put an else, and return Node with a warning as above:
aiidalogger.error("Unable to find plugin for type '{}' (node= {}), "
"will use base Node class".format(self.type, self.pk))
PluginClass = Node
Maybe add also the case where the baseNodeType is 'node' so you don't get a warning in that case. Can you add a test for Node as well (just to see that it returns Node and does not crash).
| @@ -162,7 +162,7 @@ def get_aiida_class(self): | |||
| "not valid: '{}'".format(self.pk, self.type)) | |||
|
|
|||
| try: | |||
| PluginClass = load_plugin(Node, 'aiida.orm', pluginclassname) | |||
| PluginClass = load_plugin_safe(Node, 'aiida.orm', pluginclassname) | |||
There was a problem hiding this comment.
You can remove it from the try/except, as the _safe version does not raise anymore
|
I have made the changes. |
aiida/common/pluginloader.py
Outdated
| @@ -125,15 +125,19 @@ def get_plugin(category, name): | |||
|
|
|||
| return plugin | |||
|
|
|||
| def load_plugin_safe(base_class, plugins_module, plugin_type): | |||
| def load_plugin_safe(base_class, plugins_module, plugin_type, nodeType, nodePk): | |||
There was a problem hiding this comment.
all good, but I just realised that you should use names_with_underscores for variables instead of camelCase, or we risk that people get confused when specifying the variable names of a function (the same inside the functions).
|
done. |
Issue #1110