Merge pull request #1916 from craddm/context-errors

Add more informative error messages to context commands

.gitignore

@@ -14,6 +14,7 @@ environment_configs/package_lists/dependency-cache.json
 
 # Python build caches
 __pycache__/
+.venv/
 
 # Development tools
 .vscode

data_safe_haven/commands/context.py

@@ -3,17 +3,21 @@
 from typing import Annotated, Optional
 
 import typer
-from rich import print
 
 from data_safe_haven import validators
 from data_safe_haven.context import (
     Context,
     ContextSettings,
 )
 from data_safe_haven.context_infrastructure import ContextInfrastructure
-from data_safe_haven.exceptions import DataSafeHavenConfigError
+from data_safe_haven.exceptions import (
+    DataSafeHavenAzureAPIAuthenticationError,
+    DataSafeHavenConfigError,
+)
+from data_safe_haven.utility import LoggingSingleton
 
 context_command_group = typer.Typer()
+logger = LoggingSingleton()
 
 
 @context_command_group.command()
@@ -22,24 +26,32 @@ def show() -> None:
     try:
         settings = ContextSettings.from_file()
     except DataSafeHavenConfigError as exc:
-        print("No context configuration file.")
+        logger.critical(
+            "No context configuration file. Use `dsh context add` to create one."
+        )
         raise typer.Exit(code=1) from exc
 
     current_context_key = settings.selected
     current_context = settings.context
 
-    print(f"Current context: [green]{current_context_key}")
+    logger.info(f"Current context: [green]{current_context_key}")
     if current_context is not None:
-        print(f"\tName: {current_context.name}")
-        print(f"\tAdmin Group ID: {current_context.admin_group_id}")
-        print(f"\tSubscription name: {current_context.subscription_name}")
-        print(f"\tLocation: {current_context.location}")
+        logger.info(f"\tName: {current_context.name}")
+        logger.info(f"\tAdmin Group ID: {current_context.admin_group_id}")
+        logger.info(f"\tSubscription name: {current_context.subscription_name}")
+        logger.info(f"\tLocation: {current_context.location}")
 
 
 @context_command_group.command()
 def available() -> None:
     """Show the available contexts."""
-    settings = ContextSettings.from_file()
+    try:
+        settings = ContextSettings.from_file()
+    except DataSafeHavenConfigError as exc:
+        logger.critical(
+            "No context configuration file. Use `dsh context add` to create one."
+        )
+        raise typer.Exit(code=1) from exc
 
     current_context_key = settings.selected
     available = settings.available
@@ -48,15 +60,21 @@ def available() -> None:
         available.remove(current_context_key)
         available = [f"[green]{current_context_key}*[/]", *available]
 
-    print("\n".join(available))
+    logger.info("\n".join(available))
 
 
 @context_command_group.command()
 def switch(
     key: Annotated[str, typer.Argument(help="Key of the context to switch to.")]
 ) -> None:
     """Switch the selected context."""
-    settings = ContextSettings.from_file()
+    try:
+        settings = ContextSettings.from_file()
+    except DataSafeHavenConfigError as exc:
+        logger.critical(
+            "No context configuration file. Use `dsh context add` to create one."
+        )
+        raise typer.Exit(code=1) from exc
     settings.selected = key
     settings.write()
 
@@ -146,7 +164,14 @@ def update(
     ] = None,
 ) -> None:
     """Update the selected context settings."""
-    settings = ContextSettings.from_file()
+    try:
+        settings = ContextSettings.from_file()
+    except DataSafeHavenConfigError as exc:
+        logger.critical(
+            "No context configuration file. Use `dsh context add` to create one."
+        )
+        raise typer.Exit(code=1) from exc
+
     settings.update(
         admin_group_id=admin_group,
         location=location,
@@ -161,22 +186,63 @@ def remove(
     key: Annotated[str, typer.Argument(help="Name of the context to remove.")],
 ) -> None:
     """Removes a context."""
-    settings = ContextSettings.from_file()
+    try:
+        settings = ContextSettings.from_file()
+    except DataSafeHavenConfigError as exc:
+        logger.critical("No context configuration file.")
+        raise typer.Exit(code=1) from exc
     settings.remove(key)
     settings.write()
 
 
 @context_command_group.command()
 def create() -> None:
     """Create Data Safe Haven context infrastructure."""
-    context = ContextSettings.from_file().assert_context()
+    try:
+        context = ContextSettings.from_file().assert_context()
+    except DataSafeHavenConfigError as exc:
+        if exc.args[0] == "No context selected":
+            logger.critical(
+                "No context selected. Use `dsh context switch` to select one."
+            )
+        else:
+            logger.critical(
+                "No context configuration file. Use `dsh context add` before creating infrastructure."
+            )
+        raise typer.Exit(code=1) from exc
+
     context_infra = ContextInfrastructure(context)
-    context_infra.create()
+    try:
+        context_infra.create()
+    except DataSafeHavenAzureAPIAuthenticationError as exc:
+        logger.critical(
+            "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`."
+        )
+        raise typer.Exit(1) from exc
 
 
 @context_command_group.command()
 def teardown() -> None:
     """Tear down Data Safe Haven context infrastructure."""
-    context = ContextSettings.from_file().assert_context()
+    try:
+        context = ContextSettings.from_file().assert_context()
+    except DataSafeHavenConfigError as exc:
+        if exc.args[0] == "No context selected":
+            logger.critical(
+                "No context selected. Use `dsh context switch` to select one."
+            )
+        else:
+            logger.critical(
+                "No context configuration file. Use `dsh context add` before creating infrastructure."
+            )
+        raise typer.Exit(code=1) from exc
+
     context_infra = ContextInfrastructure(context)
-    context_infra.teardown()
+
+    try:
+        context_infra.teardown()
+    except DataSafeHavenAzureAPIAuthenticationError as exc:
+        logger.critical(
+            "Failed to authenticate with the Azure API. You may not be logged into the Azure CLI, or your login may have expired. Try running `az login`."
+        )
+        raise typer.Exit(1) from exc

data_safe_haven/context_infrastructure/infrastructure.py

@@ -1,6 +1,7 @@
 from data_safe_haven.context import Context
 from data_safe_haven.exceptions import DataSafeHavenAzureError
 from data_safe_haven.external import AzureApi
+from data_safe_haven.utility import LoggingSingleton
 
 
 class ContextInfrastructure:
@@ -78,7 +79,7 @@ def create(self) -> None:
                 key_name=self.context.pulumi_encryption_key_name,
                 key_vault_name=keyvault.name,
             )
-        except Exception as exc:
+        except DataSafeHavenAzureError as exc:
             msg = f"Failed to create context resources.\n{exc}"
             raise DataSafeHavenAzureError(msg) from exc
 
@@ -88,8 +89,12 @@ def teardown(self) -> None:
         Raises:
             DataSafeHavenAzureError if any resources cannot be destroyed
         """
+        logger = LoggingSingleton()
         try:
+            logger.info(
+                f"Removing context {self.context.name} resource group {self.context.resource_group_name}"
+            )
             self.azure_api.remove_resource_group(self.context.resource_group_name)
-        except Exception as exc:
+        except DataSafeHavenAzureError as exc:
             msg = f"Failed to destroy context resources.\n{exc}"
             raise DataSafeHavenAzureError(msg) from exc

data_safe_haven/exceptions/__init__.py

@@ -42,6 +42,14 @@ class DataSafeHavenAzureError(DataSafeHavenCloudError):
     pass
 
 
+class DataSafeHavenAzureAPIError(DataSafeHavenError):
+    pass
+
+
+class DataSafeHavenAzureAPIAuthenticationError(DataSafeHavenAzureAPIError):
+    pass
+
+
 class DataSafeHavenUserHandlingError(DataSafeHavenInternalError):
     pass