CLI: Usability improvements for interactive verdi setup

There were a number of ways that a user could break the command by
providing incorrect input that was not caught by validation:

 * The following options are now required and can no longer incorrectly
   be skipped with `!`: `user_email`, `user_first_name`, `user_last_name`
   `user_institution`, `db_engine`, `db_backend`, `db_host`, `db_port`
   and the `repository_path`.
 * For a missing parameter in interactive mode the error now reads:
      Error: Institution has to be specified
   Instead of:
      Error: Missing parameter: institution
   which should be more intuitive
 * The message that a profile has successfully been created is now only
   displayed if the storage backend initialized successfully. Before,
   this was shown before storage initialization, which could then still
   fail, making the success message confusing.

aiida/cmdline/commands/cmd_setup.py

@@ -89,7 +89,6 @@ def setup(
     config.add_profile(profile)
     config.set_default_profile(profile.name)
     load_profile(profile.name)
-    echo.echo_success(f'created new profile `{profile.name}`.')
 
     # Initialise the storage
     echo.echo_report('initialising the profile storage.')
@@ -114,6 +113,7 @@ def setup(
 
     # store the updated configuration
     config.store()
+    echo.echo_success(f'created new profile `{profile.name}`.')
 
 
 @verdi.command('quicksetup')

aiida/cmdline/params/options/interactive.py

@@ -124,7 +124,10 @@ def process_value(self, ctx: click.Context, value: t.Any) -> t.Any:
             return super().process_value(ctx, value)
         except click.BadParameter as exception:
             if source is click.core.ParameterSource.PROMPT and self.is_interactive(ctx):
-                click.echo(f'Error: {exception}')
+                if isinstance(exception, click.MissingParameter):
+                    click.echo(f'Error: {self._prompt} has to be specified')
+                else:
+                    click.echo(f'Error: {exception}')
                 return self.prompt_for_value(ctx)
             raise
 

aiida/cmdline/params/options/main.py

@@ -285,24 +285,31 @@ def set_log_level(_ctx, _param, value):
 
 DB_ENGINE = OverridableOption(
     '--db-engine',
+    required=True,
     help='Engine to use to connect to the database.',
     default='postgresql_psycopg2',
     type=click.Choice(['postgresql_psycopg2'])
 )
 
 DB_BACKEND = OverridableOption(
-    '--db-backend', type=click.Choice(['core.psql_dos']), default='core.psql_dos', help='Database backend to use.'
+    '--db-backend',
+    required=True,
+    type=click.Choice(['core.psql_dos']),
+    default='core.psql_dos',
+    help='Database backend to use.'
 )
 
 DB_HOST = OverridableOption(
     '--db-host',
+    required=True,
     type=types.HostnameType(),
     help='Database server host. Leave empty for "peer" authentication.',
     default='localhost'
 )
 
 DB_PORT = OverridableOption(
     '--db-port',
+    required=True,
     type=click.INT,
     help='Database server port.',
     default=DEFAULT_DBINFO['port'],
@@ -371,7 +378,7 @@ def set_log_level(_ctx, _param, value):
 )
 
 REPOSITORY_PATH = OverridableOption(
-    '--repository', type=click.Path(file_okay=False), help='Absolute path to the file repository.'
+    '--repository', type=click.Path(file_okay=False), required=True, help='Absolute path to the file repository.'
 )
 
 PROFILE_ONLY_CONFIG = OverridableOption(

docs/source/reference/command_line.rst

@@ -381,11 +381,11 @@ Below is a list with all available subcommands.
       --last-name NONEMPTYSTRING      Last name of the user.  [required]
       --institution NONEMPTYSTRING    Institution of the user.  [required]
       --db-engine [postgresql_psycopg2]
-                                      Engine to use to connect to the database.
-      --db-backend [core.psql_dos]    Database backend to use.
+                                      Engine to use to connect to the database.  [required]
+      --db-backend [core.psql_dos]    Database backend to use.  [required]
       --db-host HOSTNAME              Database server host. Leave empty for "peer"
-                                      authentication.
-      --db-port INTEGER               Database server port.
+                                      authentication.  [required]
+      --db-port INTEGER               Database server port.  [required]
       --db-name NONEMPTYSTRING        Name of the database to create.
       --db-username NONEMPTYSTRING    Name of the database user to create.
       --db-password TEXT              Password of the database user.
@@ -404,7 +404,7 @@ Below is a list with all available subcommands.
       --broker-port INTEGER           Port for the message broker.  [default: 5672]
       --broker-virtual-host TEXT      Name of the virtual host for the message broker without
                                       leading forward slash.
-      --repository DIRECTORY          Absolute path to the file repository.
+      --repository DIRECTORY          Absolute path to the file repository.  [required]
       --test-profile                  Designate the profile to be used for running the test
                                       suite only.
       --config FILEORURL              Load option values from configuration file in yaml
@@ -485,11 +485,11 @@ Below is a list with all available subcommands.
       --last-name NONEMPTYSTRING      Last name of the user.  [required]
       --institution NONEMPTYSTRING    Institution of the user.  [required]
       --db-engine [postgresql_psycopg2]
-                                      Engine to use to connect to the database.
-      --db-backend [core.psql_dos]    Database backend to use.
+                                      Engine to use to connect to the database.  [required]
+      --db-backend [core.psql_dos]    Database backend to use.  [required]
       --db-host HOSTNAME              Database server host. Leave empty for "peer"
-                                      authentication.
-      --db-port INTEGER               Database server port.
+                                      authentication.  [required]
+      --db-port INTEGER               Database server port.  [required]
       --db-name NONEMPTYSTRING        Name of the database to create.  [required]
       --db-username NONEMPTYSTRING    Name of the database user to create.  [required]
       --db-password TEXT              Password of the database user.  [required]
@@ -504,7 +504,7 @@ Below is a list with all available subcommands.
       --broker-port INTEGER           Port for the message broker.  [required]
       --broker-virtual-host TEXT      Name of the virtual host for the message broker without
                                       leading forward slash.  [required]
-      --repository DIRECTORY          Absolute path to the file repository.
+      --repository DIRECTORY          Absolute path to the file repository.  [required]
       --test-profile                  Designate the profile to be used for running the test
                                       suite only.
       --config FILEORURL              Load option values from configuration file in yaml

tests/cmdline/commands/test_setup.py

@@ -49,7 +49,7 @@ def test_help(self):
         self.cli_runner(cmd_setup.setup, ['--help'])
         self.cli_runner(cmd_setup.quicksetup, ['--help'])
 
-    def test_quicksetup(self):
+    def test_quicksetup(self, tmp_path):
         """Test `verdi quicksetup`."""
         profile_name = 'testing'
         user_email = 'some@email.com'
@@ -60,7 +60,8 @@ def test_quicksetup(self):
         options = [
             '--non-interactive', '--profile', profile_name, '--email', user_email, '--first-name', user_first_name,
             '--last-name', user_last_name, '--institution', user_institution, '--db-port', self.pg_test.dsn['port'],
-            '--db-backend', self.storage_backend_name
+            '--db-backend', self.storage_backend_name, '--repository',
+            str(tmp_path)
         ]
 
         self.cli_runner(cmd_setup.quicksetup, options, use_subprocess=False)
@@ -83,7 +84,7 @@ def test_quicksetup(self):
         backend = profile.storage_cls(profile)
         assert backend.get_global_variable('repository|uuid') == backend.get_repository().uuid
 
-    def test_quicksetup_from_config_file(self):
+    def test_quicksetup_from_config_file(self, tmp_path):
         """Test `verdi quicksetup` from configuration file."""
         with tempfile.NamedTemporaryFile('w') as handle:
             handle.write(
@@ -94,12 +95,13 @@ def test_quicksetup_from_config_file(self):
 institution: EPFL
 db_backend: {self.storage_backend_name}
 db_port: {self.pg_test.dsn['port']}
-email: 123@234.de"""
+email: 123@234.de
+repository: {tmp_path}"""
             )
             handle.flush()
             self.cli_runner(cmd_setup.quicksetup, ['--config', os.path.realpath(handle.name)], use_subprocess=False)
 
-    def test_quicksetup_autofill_on_early_exit(self):
+    def test_quicksetup_autofill_on_early_exit(self, tmp_path):
         """Test `verdi quicksetup` stores user information even if command fails."""
         config = configuration.get_config()
         assert config.get_option('autofill.user.email', scope=None) is None
@@ -117,7 +119,8 @@ def test_quicksetup_autofill_on_early_exit(self):
         options = [
             '--non-interactive', '--profile', 'testing', '--email', user_email, '--first-name', user_first_name,
             '--last-name', user_last_name, '--institution', user_institution, '--db-port',
-            self.pg_test.dsn['port'] + 100
+            self.pg_test.dsn['port'] + 100, '--repository',
+            str(tmp_path)
         ]
 
         self.cli_runner(cmd_setup.quicksetup, options, raises=True, use_subprocess=False)

tests/cmdline/params/options/test_interactive.py

@@ -497,7 +497,7 @@ def test_interactive_ignore_default_required_option(run_cli_command):
     """Test that if an option is required, ``!`` is translated to ``None`` and so is not accepted."""
     cmd = create_command(required=True)
     result = run_cli_command(cmd, user_input='!\nvalue\n')
-    expected = 'Opt: !\nError: Missing parameter: opt\nOpt: value\nvalue\n'
+    expected = 'Opt: !\nError: Opt has to be specified\nOpt: value\nvalue\n'
     assert expected in result.output