aiidateam / aiida-core

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

computer configure --non-interactive asks for safe interval despite default value #3273

Open ltalirz opened 5 years ago

ltalirz commented 5 years ago

When running verdi computer configure local localhost interactively, I get a default value for the connection cooldown time.

However, this throws an exception:

$ verdi computer configure local localhost -n
Usage: verdi computer configure local [OPTIONS] COMPUTER

Error: Missing option "--safe-interval".

This is unexpected, since it should simply take the default value in non-interactive mode.

I thought that was an easy fix and started looking into the code, only to encounter the fact that the cli setup for transport plugins is really convoluted.

In particular, the options are translated from some custom internal key/value representation https://github.com/aiidateam/aiida-core/blob/a825865f22b4ad4a530a4fd2ad77ee1a876b8040/aiida/transports/plugins/ssh.py#L63-L77 https://github.com/aiidateam/aiida-core/blob/a825865f22b4ad4a530a4fd2ad77ee1a876b8040/aiida/transports/transport.py#L57-L64 to click using a lot of "machinery". Is this really necessary?

This isn't high-priority but couldn't we just use the simpler approach from data plugins, where a plugin simply adds a new command to an existing group?

Mentioning @giovannipizzi @sphuber for comment.

sphuber commented 5 years ago

This was implemented by @DropD to automatically generate the click sub commands based on the _valid_connect_options and _common_auth_options attributes that can be defined on the Transport sub class. I am not 100% certain on the motivation, but I imagine that it was a way to deduplicate code as well as making the generation of new Transport plugins easier with respect to adding also the configure end point. Maybe the latter is now obsolete since we could maybe provide the aiida.cmdline.computer.configure entry point category that people could use.