aiidateam / aiida-core

The official repository for the AiiDA code
https://aiida-core.readthedocs.io
Other
436 stars 190 forks source link

verdi data upf uploadfamily broken #1104

Closed ltalirz closed 6 years ago

ltalirz commented 6 years ago

This comes from the build of the Quantum Mobile VM. The following command used to work in 0.10.1 and doesn't anymore in 0.11.0:

$ verdi data upf uploadfamily /home/max/codes/sg15-oncv-1.1 sg15-oncv-1.1 'SG15 Optimized Norm-Conserving Vanderbilt (ONCV) pseudopotentials'
Traceback (most recent call last):
  File "/home/max/.virtualenvs/aiida/bin/verdi", line 11, in <module>
    sys.exit(run())
  File "/home/max/.virtualenvs/aiida/local/lib/python2.7/site-packages/aiida/cmdline/verdilib.py", line 1049, in run
    aiida.cmdline.verdilib.exec_from_cmdline(sys.argv)
  File "/home/max/.virtualenvs/aiida/local/lib/python2.7/site-packages/aiida/cmdline/verdilib.py", line 1033, in exec_from_cmdline
    CommandClass = list_commands[command]()
  File "/home/max/.virtualenvs/aiida/local/lib/python2.7/site-packages/aiida/cmdline/commands/data.py", line 52, in __init__
    self.routed_subcommands[cmd.name] = verdi
AttributeError: type object '_Psf' has no attribute 'name'

I suggest, there needs to be a test for this. And perhaps the upf-related stuff should move into aiida-quantumespresso, now that the one can also plug into verdi data...

sphuber commented 6 years ago

This is not necessarily a problem with the upf command but rather with the new verdi data command hook that was introduced in PR #993 The problem is with the following snippet

class Data(VerdiCommandRouter):

    def __init__(self):
        ....
        for ep in plugin_list('cmdline.data'):
            cmd = get_plugin('cmdline.data', ep)
            self.routed_subcommands[cmd.name] = verdi

The routed_subcommands is to be dynamically updated with commands registered under the aiida.cmdline.data entry point, however it tries to get the name through the name attribute which is not implemented. I am not sure what should be used here. I thought you tested this (manually) before the release and it worked., but now I can't see how it could ever have, unless you manually implemented a name attribute for your data command class. I am assigning @DropD for this issue

ltalirz commented 6 years ago

Thanks @sphuber for checking this out. I should probably become a bit more rigorous in my pull request reviews... ;) @DropD Could you please look into this? This is currently blocking the next release of Quantum Mobile...

DropD commented 6 years ago

At first glance I think this might have to do click.groupand click.command, one of them having a .name attribute, the other not.

DropD commented 6 years ago

The Data command expects plugin commands to be a click.group

DropD commented 6 years ago

Hang on, what exactly is _Psf? It seems to be advertised as a plugin command by some plugin without actually being one. I don't see this error message and it has not previously appeared when you tested it because the faulty plugin was not there.

I think one of the plugins has not caught up with recent versions and is trying to expose an old-style command instead of a click one.

DropD commented 6 years ago

The easyest solution is to fix the plugin to simply not include old commands in the entry points until they are updated

ltalirz commented 6 years ago

Just tried to reproduce it on a fresh aiida install and indeed I didn't manage to. Very sorry about this! I'll look into what caused it on Quantum Mobile.

ltalirz commented 6 years ago

Ok, the culprit was aiida-siesta. I'll move the issue there.