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

CLI: update to be compatible with aiida-core==2.1 #136

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Nov 8, 2022

Fixes #130

Currently the CLI is broken with aiida-core==2.1. The reason is that we are using the PROFILE option, which relies on the command that it is attached to configure certain things, such as the context. In v2.1, the behavior was changed causing the commands to except because the PROFILE option cannot find certain attributes it expects in the context.

For the PROFILE option to work, the root command should use the class aiida.cmdline.groups.VerdiCommandGroup as its cls. This class defines a custom context class that is necessary. It also automatically provides the verbosity option for all subcommands, so the explicit declaration can now be removed.

The problem was not detected in the unit tests because they only show up when the root command is invoked. The unit tests call the subcommands directly though, and this circumvents the root command. This is documented behavior of click and there is no way around it. The only solution is to call the commands through the command line interface exactly as they would be called normally.

This is now done in the test_commands.py file. The test parametrizes over all existing (sub)commands and directly calls it as a subprocess. This guarantees that the root command is called including its specific context that needs to be setup.

@sphuber sphuber force-pushed the fix/130/cli-verdi-context branch 2 times, most recently from 6d10f63 to 9e4eb40 Compare November 8, 2022 09:15
Currently the CLI is broken with `aiida-core==2.1`. The reason is that
we are using the `PROFILE` option, which relies on the command that it
is attached to configure certain things, such as the context. In v2.1,
the behavior was changed causing the commands to except because the
`PROFILE` option cannot find certain attributes it expects in the
context.

For the `PROFILE` option to work, the root command should use the class
`aiida.cmdline.groups.VerdiCommandGroup` as its `cls`. This class defines
a custom context class that is necessary. It also automatically provides
the verbosity option for all subcommands, so the explicit declaration
can now be removed.

The problem was not detected in the unit tests because they only show up
when the root command is invoked. The unit tests call the subcommands
directly though, and this circumvents the root command. This is
documented behavior of `click` and there is no way around it. The only
solution is to call the commands through the command  line interface
exactly as they would be called normally.

This is now done in the `test_commands.py` file. The test parametrizes
over all existing (sub)commands and directly calls it as a subprocess.
This guarantees that the root command is called including its specific
context that needs to be setup.

The `VerdiCommandGroup` also automatically adds the `-v/--verbosity`
option to all commands. This is useful as it automatically controls the
logging configuration setup by `aiida-core`. However, the `-v` short
flag overlaps with our `-v/--version` option that is used to designate
pseudo potential family versions. To solve this the `VerdiCommandGroup`
clas is subclassed to override the `add_verbosity_option` method so that
we can change the `VERBOSITY` option for a custom one, that only defines
the `--verbosity` long option.
@sphuber sphuber force-pushed the fix/130/cli-verdi-context branch from 9e4eb40 to 14baf9b Compare November 8, 2022 09:23
@sphuber sphuber merged commit a83fd76 into master Nov 8, 2022
@sphuber sphuber deleted the fix/130/cli-verdi-context branch November 8, 2022 09:32
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.

CLI broken when installed with aiida-core==2.1
1 participant