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

Add more informative error messages to context commands #1916

Merged
merged 35 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d2e243b
Add more informative error messages to context commands
craddm May 23, 2024
6abd1f6
Catch non-existent context file for teardown
craddm May 23, 2024
f08ea72
Add error when not logged in to az cli
craddm May 23, 2024
32abe77
Fixlinting errors
craddm May 23, 2024
e242821
fix typo
craddm May 28, 2024
533512f
Remove unnecessary whitespace
craddm May 28, 2024
b9da2b2
Clearer explanation of missing context for teardown
craddm May 28, 2024
06ac61b
Fuller explanation of error when az login has expired
craddm May 28, 2024
0e78a96
Catch attempted teardown when no context infrastructure deployed
craddm May 28, 2024
873d6f1
Handle errors when no context selected
craddm May 29, 2024
0d34102
Improve resource group removal log messages
JimMadge May 31, 2024
1ebe3dc
Remove from exc from typer.Exit
JimMadge May 31, 2024
3e076e7
Make messages more consistent
JimMadge May 31, 2024
d9b91fe
Catch exception if Azure API auth fails
JimMadge May 31, 2024
f65f45b
Fix linting
JimMadge May 31, 2024
0938502
Correct logger construction
JimMadge May 31, 2024
e1e41ff
Add tests for updated context commands
JimMadge May 31, 2024
d228e5f
Run lint:fmt
JimMadge May 31, 2024
528aab9
Restructure exceptions so auth failure can be caught
JimMadge May 31, 2024
82c9b42
Fix KeyClient exceptions
JimMadge May 31, 2024
14eb66c
Use Azure SDK base exception where appropriate
JimMadge May 31, 2024
2ece789
Merge branch 'alan-turing-institute:develop' into context-errors
craddm Jun 3, 2024
ae12e11
Merge branch 'alan-turing-institute:develop' into context-errors
craddm Jun 4, 2024
3d98eb7
ignore python virtual environments
craddm Jun 4, 2024
16cd09d
Change from run to use
craddm Jun 4, 2024
fc1161b
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
23c9e71
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
fb3050a
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
fc1e649
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
82cd6f8
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
86beb56
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
1be93bb
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
74f6f60
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
9da2cff
Update data_safe_haven/commands/context.py
craddm Jun 4, 2024
89ab2c4
:loud_sound: Use LoggingSingleton for error message before exit
jemrobinson Jun 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ environment_configs/package_lists/dependency-cache.json

# Python build caches
__pycache__/
.venv/

# Development tools
.vscode
Expand Down
100 changes: 83 additions & 17 deletions data_safe_haven/commands/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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()

Expand Down Expand Up @@ -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,
Expand All @@ -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
9 changes: 7 additions & 2 deletions data_safe_haven/context_infrastructure/infrastructure.py
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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

Expand All @@ -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
8 changes: 8 additions & 0 deletions data_safe_haven/exceptions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ class DataSafeHavenAzureError(DataSafeHavenCloudError):
pass


class DataSafeHavenAzureAPIError(DataSafeHavenError):
JimMadge marked this conversation as resolved.
Show resolved Hide resolved
pass


class DataSafeHavenAzureAPIAuthenticationError(DataSafeHavenAzureAPIError):
pass


class DataSafeHavenUserHandlingError(DataSafeHavenInternalError):
pass

Expand Down
Loading
Loading