feat: don't prompt for env deletion on tutor config save -i

The introduction of the `-c/--clean` option caused the deletion prompt
to be displayed for every call to `tutor config save --interactive`.
This is not the desired behaviour, as decided here:
overhangio#1086 (comment)

With this change, the prompt is only displayed when running: `tutor
config save --interactive --clean`. The environment is still deleted on
`tutor config save --clean`, but without prompt.

We refactored the code with hooks, which simplifies the signature of the
interactive prompt function.

changelog.d/20241031_144431_regis_no_delete_env.md

@@ -0,0 +1 @@
+- [Improvement] Do not prompt for environment deletion by default on `tutor config save --interactive`. (by @regisb)

tests/test_config.py

@@ -48,7 +48,7 @@ def mock_prompt(*_args: None, **kwargs: str) -> str:
             with patch.object(click, "prompt", new=mock_prompt):
                 with patch.object(click, "confirm", new=mock_prompt):
                     config = tutor_config.load_minimal(rootdir)
-                    interactive.ask_questions(config, rootdir)
+                    interactive.ask_questions(config)
 
         self.assertIn("MYSQL_ROOT_PASSWORD", config)
         self.assertEqual(8, len(get_typed(config, "MYSQL_ROOT_PASSWORD", str)))

tutor/commands/compose.py

@@ -197,7 +197,6 @@ def interactive_configuration(
         click.echo(fmt.title("Interactive platform configuration"))
         interactive_config.ask_questions(
             config,
-            context.obj.root,
             run_for_prod=run_for_prod,
         )
     tutor_config.save_config_file(context.obj.root, config)

tutor/commands/config.py

@@ -7,12 +7,12 @@
 import click.shell_completion
 
 from tutor import config as tutor_config
-from tutor import env, exceptions, fmt
+from tutor import env, exceptions, fmt, hooks
 from tutor import interactive as interactive_config
 from tutor import serialize
 from tutor.commands.context import Context
 from tutor.commands.params import ConfigLoaderParam
-from tutor.types import ConfigValue
+from tutor.types import Config, ConfigValue
 
 
 @click.group(
@@ -155,9 +155,23 @@ def save(
     clean_env: bool,
 ) -> None:
     config = tutor_config.load_minimal(context.root)
+
+    # Add question to interactive prompt, such that the environment is automatically
+    # deleted if necessary in interactive mode.
+    @hooks.Actions.CONFIG_INTERACTIVE.add()
+    def _prompt_for_env_deletion(_config: Config) -> None:
+        if clean_env:
+            run_clean = click.confirm(
+                fmt.question("Remove existing Tutor environment directory?"),
+                prompt_suffix=" ",
+                default=True,
+            )
+            if run_clean:
+                env.delete_env_dir(context.root)
+
     if interactive:
-        interactive_config.ask_questions(config, context.root, clean_env_prompt=True)
-    if clean_env:
+        interactive_config.ask_questions(config)
+    elif clean_env:
         env.delete_env_dir(context.root)
     if set_vars:
         for key, value in set_vars:

tutor/commands/k8s.py

@@ -228,7 +228,7 @@ def launch(context: click.Context, non_interactive: bool) -> None:
     config = tutor_config.load_minimal(context.obj.root)
     if not non_interactive:
         click.echo(fmt.title("Interactive platform configuration"))
-        interactive_config.ask_questions(config, context.obj.root, run_for_prod=True)
+        interactive_config.ask_questions(config, run_for_prod=True)
     tutor_config.save_config_file(context.obj.root, config)
     config = tutor_config.load_full(context.obj.root)
     tutor_env.save(context.obj.root, config)

tutor/env.py

@@ -539,10 +539,10 @@ def delete_env_dir(root: str) -> None:
     try:
         shutil.rmtree(env_path)
         fmt.echo_alert(f"Removed existing Tutor environment at: {env_path}")
-    except PermissionError:
+    except PermissionError as e:
         raise exceptions.TutorError(
             f"Permission Denied while trying to remove existing Tutor environment at: {env_path}"
-        )
+        ) from e
     except FileNotFoundError:
         fmt.echo_info(f"No existing Tutor environment to remove at: {env_path}")
 

tutor/interactive.py

@@ -7,21 +7,14 @@
 from .types import Config, get_typed
 
 
-def ask_questions(
-    config: Config,
-    root: str,
-    run_for_prod: Optional[bool] = None,
-    clean_env_prompt: bool = False,
-) -> None:
+def ask_questions(config: Config, run_for_prod: Optional[bool] = None) -> None:
     """
     Interactively ask questions to collect configuration values from the user.
 
     Arguments:
         config: Existing (or minimal) configuration. Modified in-place.
         run_for_prod: Whether platform should be configured for production.
             If None, then ask the user.
-        clean_env_prompt: Whether to show the clean environment prompt before running.
-            defaults to False.
     Returns:
         None
     """
@@ -157,14 +150,6 @@ def ask_questions(
             config,
             defaults,
         )
-    if clean_env_prompt:
-        run_clean = click.confirm(
-            fmt.question("Remove existing Tutor environment directory?"),
-            prompt_suffix=" ",
-            default=True,
-        )
-        if run_clean:
-            env.delete_env_dir(root)
 
     hooks.Actions.CONFIG_INTERACTIVE.do(config)