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 9 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
44 changes: 37 additions & 7 deletions data_safe_haven/commands/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def show() -> None:
try:
settings = ContextSettings.from_file()
except DataSafeHavenConfigError as exc:
print("No context configuration file.")
print("No context configuration file. Run `dsh context add` to create one.")
raise typer.Exit(code=1) from exc

current_context_key = settings.selected
Expand All @@ -39,7 +39,11 @@ def show() -> None:
@context_command_group.command()
def available() -> None:
"""Show the available contexts."""
settings = ContextSettings.from_file()
try:
settings = ContextSettings.from_file()
except DataSafeHavenConfigError as exc:
print("No context configuration file. Run `dsh context add` to create one.")
jemrobinson marked this conversation as resolved.
Show resolved Hide resolved
raise typer.Exit(code=1) from exc

current_context_key = settings.selected
available = settings.available
Expand All @@ -56,7 +60,11 @@ 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:
print("No context configuration file. Run `dsh context add` to create one.")
raise typer.Exit(code=1) from exc
settings.selected = key
settings.write()

Expand Down Expand Up @@ -145,7 +153,12 @@ def update(
] = None,
) -> None:
"""Update the selected context settings."""
settings = ContextSettings.from_file()
try:
settings = ContextSettings.from_file()
except DataSafeHavenConfigError as exc:
print("No context configuration file. Run `dsh context add` to create one.")
raise typer.Exit(code=1) from exc

settings.update(
admin_group_id=admin_group,
location=location,
Expand All @@ -160,22 +173,39 @@ 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:
print("No context configuration file, so no contexts to remove.")
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:
print(
"No context configuration file. Run `dsh context add` before creating infrastructure."
)
raise typer.Exit(code=1) from exc

JimMadge marked this conversation as resolved.
Show resolved Hide resolved
context_infra = ContextInfrastructure(context)
context_infra.create()


@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:
print(
"No context configuration file. A context configuration must exist before the context can be torn down."
)
raise typer.Exit(code=1) from exc
JimMadge marked this conversation as resolved.
Show resolved Hide resolved
context_infra = ContextInfrastructure(context)
context_infra.teardown()
6 changes: 6 additions & 0 deletions data_safe_haven/context_infrastructure/infrastructure.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ def __init__(self, context: Context) -> None:
self.azure_api_: AzureApi | None = None
self.context = context
self.tags = {"component": "context"} | context.tags
try:
if self.azure_api.subscription_id:
pass
except Exception as exc:
msg = "Not logged in or login has expired. \nAuthenticate using az login before creating or tearing down context infrastructure."
raise DataSafeHavenAzureError(msg) from exc
JimMadge marked this conversation as resolved.
Show resolved Hide resolved

@property
def azure_api(self) -> AzureApi:
Expand Down
5 changes: 5 additions & 0 deletions data_safe_haven/external/api/azure_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,11 @@ def remove_resource_group(self, resource_group_name: str) -> None:
self.logger.debug(
f"Removing resource group [green]{resource_group_name}[/] if it exists...",
)
if not resource_client.resource_groups.check_existence(resource_group_name):
self.logger.error(
f"Resource group [green]{resource_group_name}[/] does not exist. There is no context infrastructure to teardown.",
)
return
JimMadge marked this conversation as resolved.
Show resolved Hide resolved
poller = resource_client.resource_groups.begin_delete(
resource_group_name,
)
Expand Down
Loading