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 comments/docstrings to exceptions #1873

Merged
merged 68 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
95dbc72
Add comment for EntraID error
craddm May 9, 2024
d82fd48
Move comment to docstring
craddm May 10, 2024
fef1660
Add docstring for configerror
craddm May 10, 2024
16ab4af
Add docstring for SSLError
craddm May 10, 2024
6b9e4c7
shorten descriptive text in docstrings
craddm May 10, 2024
20b0693
Add docstring for CloudError
craddm May 10, 2024
0c97db8
Add docstring for CloudError
craddm May 10, 2024
a3249c9
Add docstring for IPRangeError
craddm May 10, 2024
bbcf278
Finish AzureError docstring, add for DataSafeHavenError
craddm May 10, 2024
09736be
Add docstring for DataSafeHavenError
craddm May 10, 2024
0ed15f5
SSLError docstring
craddm May 10, 2024
81f8914
Merge branch 'alan-turing-institute:develop' into add-exception-comments
craddm May 13, 2024
bd33362
add .venv/ to gitignore
craddm May 14, 2024
6de1f1d
add several further expection docstrings
craddm May 15, 2024
30d104a
Add additional exception docstrings
craddm May 15, 2024
ccddc09
document use of DSHInputError
craddm May 15, 2024
ffa682e
Use Pulumi error for consistency
craddm May 15, 2024
599ed03
Merge branch 'develop' into add-exception-comments
jemrobinson Jun 24, 2024
f95d611
Merge branch 'alan-turing-institute:develop' into add-exception-comments
craddm Jun 25, 2024
d4c4dff
Add more detailed comments and notes for error classes
craddm Jun 25, 2024
f420e41
Remove DataSafeHavenNotImplementedError
craddm Jun 25, 2024
b3a932c
Merge branch 'develop' into add-exception-comments
craddm Jun 25, 2024
71e4e1e
Remove unnecessary spaces
craddm Jun 25, 2024
0cc6331
Reorder error classes
craddm Jun 25, 2024
8dbb4ab
Grammar fixes
craddm Jun 25, 2024
8582ca6
More concise docstring for IPRangeError
craddm Jun 25, 2024
ca347e2
Make userhandlingerror no longer an internalerror
craddm Jun 25, 2024
ee71d15
remove internalerror from azure_api and replace with AzureError
craddm Jun 25, 2024
09a9a12
Add DSHType andDSHValue Error classes
craddm Jun 25, 2024
6ef4800
change parameter errors to valueerrors
craddm Jun 25, 2024
681285e
Change DSHInputErrors to DSHTypeErrors for Entra users
craddm Jun 25, 2024
0c049df
Change from InputError to ValueError
craddm Jun 25, 2024
0adda50
Change from InputError to ValueError
craddm Jun 25, 2024
edbfcd6
Change from InputError to ValueError
craddm Jun 25, 2024
c3fcdfe
Remove DSHInputError class
craddm Jun 25, 2024
951205a
Replace InputError with ValueError
craddm Jun 25, 2024
5e84f06
Replace DSHInputError with DSHPulumiError
craddm Jun 25, 2024
d4abfc2
Fix linting errors
craddm Jun 25, 2024
88fe681
Replace DSHInternal with DSHMicrosoftGraph errors
craddm Jun 25, 2024
354ccf9
Replace ParameterError with TypeError
craddm Jun 25, 2024
a7b110a
fix linting error
craddm Jun 25, 2024
fb29ffb
Update strings tests to check for Value rather than Input errors
craddm Jun 25, 2024
277663b
Remove DSHInternalError class
craddm Jun 25, 2024
afa9272
Update tests from ParameterError to TypeError
craddm Jun 25, 2024
2a7e42b
Update tests to reflect changes in error types
craddm Jun 25, 2024
6a31329
Change ParameterError to TypeError
craddm Jun 25, 2024
ece0a8c
Change ParameterError to Type and ValueErrors as applicable
craddm Jun 25, 2024
3fd1a1f
Change to DSHTypeError
craddm Jun 25, 2024
9c24d7c
Remove DSHParameterError class
craddm Jun 25, 2024
d6d1dff
Change tests to check for TypeError
craddm Jun 25, 2024
5598759
Decode bytes for printing
craddm Jun 25, 2024
2ed00ca
Merge branch 'alan-turing-institute:develop' into add-exception-comments
craddm Jun 26, 2024
5b8afff
Fix expected exception message in tests
JimMadge Jun 26, 2024
65b8888
Change AzureAPIAuthenticationError parent
JimMadge Jun 26, 2024
7d48502
Fix linting
JimMadge Jun 26, 2024
6f8b99e
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
f7bd0cf
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
9a80026
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
9e38d1e
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
a4bc4ad
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
319fbc7
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
f1709bb
Update data_safe_haven/exceptions/__init__.py
craddm Jun 26, 2024
1da2f5a
Use typer.Exit() pattern
craddm Jun 26, 2024
ab0ca99
Change to raise typer.Exit(1) pattern
craddm Jun 26, 2024
b30af88
place message directly in logger call
craddm Jun 26, 2024
43a9698
fix linting
craddm Jun 26, 2024
8caebec
Replace remaining non-typer, terminating raises
JimMadge Jun 27, 2024
c45f705
Fix linting
JimMadge Jun 27, 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
6 changes: 3 additions & 3 deletions data_safe_haven/administration/users/entra_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from data_safe_haven.exceptions import (
DataSafeHavenEntraIDError,
DataSafeHavenError,
DataSafeHavenInputError,
DataSafeHavenTypeError,
)
from data_safe_haven.external import GraphApi
from data_safe_haven.functions import password
Expand Down Expand Up @@ -55,10 +55,10 @@ def add(self, new_users: Sequence[ResearchUser]) -> None:
msg = (
f"User '[green]{user.username}[/]' is missing an email address."
)
raise DataSafeHavenInputError(msg)
raise DataSafeHavenTypeError(msg)
if not user.phone_number:
msg = f"User '[green]{user.username}[/]' is missing a phone number."
raise DataSafeHavenInputError(msg)
raise DataSafeHavenTypeError(msg)
self.graph_api.create_user(
request_json, user.email_address, user.phone_number
)
Expand Down
20 changes: 9 additions & 11 deletions data_safe_haven/commands/shm.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@

from data_safe_haven.config import DSHPulumiConfig, SHMConfig
from data_safe_haven.context import ContextSettings
from data_safe_haven.exceptions import DataSafeHavenError, DataSafeHavenInputError
from data_safe_haven.exceptions import DataSafeHavenError, DataSafeHavenPulumiError
from data_safe_haven.external import GraphApi
from data_safe_haven.infrastructure import SHMProjectManager
from data_safe_haven.logging import get_logger

shm_command_group = typer.Typer()

Expand All @@ -31,6 +32,7 @@ def deploy(
context, encrypted_key=None, projects={}
)

logger = get_logger()
try:
# Connect to GraphAPI interactively
graph_api = GraphApi(
Expand Down Expand Up @@ -75,13 +77,8 @@ def deploy(
stack.output("networking")["fqdn_nameservers"],
)
except DataSafeHavenError as exc:
# Note, would like to exit with a non-zero code here.
# However, typer.Exit does not print the exception tree which is very unhelpful
# for figuring out what went wrong.
# print("Could not deploy Data Safe Haven Management environment.")
# raise typer.Exit(code=1) from exc
msg = "Could not deploy Data Safe Haven Management environment."
raise DataSafeHavenError(msg) from exc
logger.critical("Could not deploy Data Safe Haven Management environment.")
raise typer.Exit(code=1) from exc
finally:
# Upload Pulumi config to blob storage
pulumi_config.upload(context)
Expand All @@ -94,6 +91,7 @@ def teardown() -> None:
config = SHMConfig.from_remote(context)
pulumi_config = DSHPulumiConfig.from_remote(context)

logger = get_logger()
try:
# Remove infrastructure deployed with Pulumi
try:
Expand All @@ -105,13 +103,13 @@ def teardown() -> None:
stack.teardown()
except Exception as exc:
msg = "Unable to teardown Pulumi infrastructure."
raise DataSafeHavenInputError(msg) from exc
raise DataSafeHavenPulumiError(msg) from exc

# Remove information from config file
del pulumi_config[context.shm_name]

# Upload Pulumi config to blob storage
pulumi_config.upload(context)
except DataSafeHavenError as exc:
msg = "Could not teardown Safe Haven Management environment."
raise DataSafeHavenError(msg) from exc
logger.critical("Could not teardown Safe Haven Management environment.")
raise typer.Exit(1) from exc
17 changes: 11 additions & 6 deletions data_safe_haven/commands/sre.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@

from data_safe_haven.config import DSHPulumiConfig, SHMConfig, SREConfig
from data_safe_haven.context import ContextSettings
from data_safe_haven.exceptions import DataSafeHavenError, DataSafeHavenInputError
from data_safe_haven.exceptions import DataSafeHavenError, DataSafeHavenPulumiError
from data_safe_haven.external import GraphApi
from data_safe_haven.infrastructure import SHMProjectManager, SREProjectManager
from data_safe_haven.logging import get_logger
from data_safe_haven.provisioning import SREProvisioningManager

sre_command_group = typer.Typer()
Expand All @@ -32,6 +33,7 @@ def deploy(
shm_config = SHMConfig.from_remote(context)
sre_config = SREConfig.from_remote_by_name(context, name)

logger = get_logger()
try:
# Load GraphAPI as this may require user-interaction that is not possible as
# part of a Pulumi declarative command
Expand Down Expand Up @@ -103,8 +105,10 @@ def deploy(
)
manager.run()
except DataSafeHavenError as exc:
msg = f"Could not deploy Secure Research Environment {sre_config.safe_name}."
raise DataSafeHavenError(msg) from exc
logger.critical(
f"Could not deploy Secure Research Environment {sre_config.safe_name}."
)
raise typer.Exit(code=1) from exc
finally:
# Upload Pulumi config to blob storage
pulumi_config.upload(context)
Expand All @@ -120,6 +124,7 @@ def teardown(
shm_config = SHMConfig.from_remote(context)
sre_config = SREConfig.from_remote_by_name(context, name)

logger = get_logger()
try:
# Load GraphAPI as this may require user-interaction that is not possible as
# part of a Pulumi declarative command
Expand All @@ -140,15 +145,15 @@ def teardown(
stack.teardown()
except Exception as exc:
msg = "Unable to teardown Pulumi infrastructure."
raise DataSafeHavenInputError(msg) from exc
raise DataSafeHavenPulumiError(msg) from exc

# Remove Pulumi project from Pulumi config file
del pulumi_config[name]

# Upload Pulumi config to blob storage
pulumi_config.upload(context)
except DataSafeHavenError as exc:
msg = (
logger.critical(
f"Could not teardown Secure Research Environment '{sre_config.safe_name}'."
)
raise DataSafeHavenError(msg) from exc
raise typer.Exit(1) from exc
24 changes: 14 additions & 10 deletions data_safe_haven/commands/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ def add(
users = UserHandler(context, pulumi_config, graph_api)
users.add(csv)
except DataSafeHavenError as exc:
msg = f"Could not add users to Data Safe Haven '{shm_name}'."
raise DataSafeHavenError(msg) from exc
logger.critical(f"Could not add users to Data Safe Haven '{shm_name}'.")
raise typer.Exit(1) from exc


@users_command_group.command("list")
Expand Down Expand Up @@ -87,8 +87,8 @@ def list_users(
users = UserHandler(context, pulumi_config, graph_api)
users.list(sre)
except DataSafeHavenError as exc:
msg = f"Could not list users for Data Safe Haven '{shm_name}'."
raise DataSafeHavenError(msg) from exc
logger.critical(f"Could not list users for Data Safe Haven '{shm_name}'.")
raise typer.Exit(1) from exc


@users_command_group.command()
Expand Down Expand Up @@ -155,8 +155,10 @@ def register(
)
users.register(sre_name, usernames_to_register)
except DataSafeHavenError as exc:
msg = f"Could not register users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'."
raise DataSafeHavenError(msg) from exc
logger.critical(
f"Could not register users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'."
)
raise typer.Exit(1) from exc


@users_command_group.command()
Expand Down Expand Up @@ -194,8 +196,8 @@ def remove(
users = UserHandler(context, pulumi_config, graph_api)
users.remove(usernames)
except DataSafeHavenError as exc:
msg = f"Could not remove users from Data Safe Haven '{shm_name}'."
raise DataSafeHavenError(msg) from exc
logger.critical(f"Could not remove users from Data Safe Haven '{shm_name}'.")
raise typer.Exit(1) from exc


@users_command_group.command()
Expand Down Expand Up @@ -263,5 +265,7 @@ def unregister(
):
users.unregister(group_name, usernames_to_unregister)
except DataSafeHavenError as exc:
msg = f"Could not unregister users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'."
raise DataSafeHavenError(msg) from exc
logger.critical(
f"Could not unregister users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'."
)
raise typer.Exit(1) from exc
8 changes: 4 additions & 4 deletions data_safe_haven/context/context_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from data_safe_haven.directories import config_dir
from data_safe_haven.exceptions import (
DataSafeHavenConfigError,
DataSafeHavenParameterError,
DataSafeHavenValueError,
)
from data_safe_haven.logging import get_logger
from data_safe_haven.serialisers import YAMLSerialisableModel
Expand Down Expand Up @@ -68,7 +68,7 @@ def selected(self, context_name: str | None) -> None:
self.logger.info(f"Switched context to '{context_name}'.")
else:
msg = f"Context '{context_name}' is not defined."
raise DataSafeHavenParameterError(msg)
raise DataSafeHavenValueError(msg)

@property
def context(self) -> Context | None:
Expand Down Expand Up @@ -127,7 +127,7 @@ def add(
# Ensure context is not already present
if key in self.available:
msg = f"A context with key '{key}' is already defined."
raise DataSafeHavenParameterError(msg)
raise DataSafeHavenValueError(msg)

self.contexts[key] = Context(
name=name,
Expand All @@ -139,7 +139,7 @@ def add(
def remove(self, key: str) -> None:
if key not in self.available:
msg = f"No context with key '{key}'."
raise DataSafeHavenParameterError(msg)
raise DataSafeHavenValueError(msg)

del self.contexts[key]

Expand Down
84 changes: 67 additions & 17 deletions data_safe_haven/exceptions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@


class DataSafeHavenError(Exception):
"""
Parent class for all DataSafeHaven exceptions.

This class is not intended to be instantiated directly. Developers should use one of the subclasses instead.
"""

def __init__(self, message: str | bytes):
super().__init__(message)

Expand All @@ -12,61 +18,105 @@ def __init__(self, message: str | bytes):
logger.error(message_str.replace("\n", r"\n"))


class DataSafeHavenCloudError(DataSafeHavenError):
pass
class DataSafeHavenAzureError(DataSafeHavenError):
"""
Exception class for handling errors when interacting with Azure.

Raise this error when, for example, creating resources in Azure fails.
"""

class DataSafeHavenConfigError(DataSafeHavenError):
pass


class DataSafeHavenEntraIDError(DataSafeHavenCloudError):
class DataSafeHavenAzureAPIAuthenticationError(DataSafeHavenError):
"""
Exception class for handling errors when authenticating against the Azure API.

Used to capture exceptions generated when the user is not authenticated or authentication has expired.
"""

pass


class DataSafeHavenInputError(DataSafeHavenError):
class DataSafeHavenConfigError(DataSafeHavenError):
"""
Exception class for handling errors related to configuration files.

Examples include missing configuration files or invalid configuration values.
"""

pass


class DataSafeHavenInternalError(DataSafeHavenError):
class DataSafeHavenEntraIDError(DataSafeHavenError):
"""
Exception class for handling errors when interacting with Entra ID.

For example, when adding users to an Entra group fails.
"""

pass


class DataSafeHavenIPRangeError(DataSafeHavenError):
"""Exception raised when it is not possible to generate a valid IPv4 range."""

pass


class DataSafeHavenNotImplementedError(DataSafeHavenInternalError):
class DataSafeHavenMicrosoftGraphError(DataSafeHavenAzureError):
"""
Exception class for handling errors when interacting with the Microsoft Graph API.
"""

pass


class DataSafeHavenParameterError(DataSafeHavenError):
class DataSafeHavenPulumiError(DataSafeHavenError):
"""
Exception class for handling errors when interacting with Pulumi.

For example, when a Pulumi operation such as a deployment fails.
"""

pass


class DataSafeHavenSSLError(DataSafeHavenError):
pass
"""
Exception class for handling errors related to administration of SSL certificates.

For example, errors refreshing or creating SSL certificates.
"""

class DataSafeHavenAzureError(DataSafeHavenCloudError):
pass


class DataSafeHavenAzureAPIError(DataSafeHavenError):
pass
class DataSafeHavenTypeError(DataSafeHavenError):
"""
Exception class for handling errors related to type checking.

For example, when a function is called with an argument of the wrong type.
"""

class DataSafeHavenAzureAPIAuthenticationError(DataSafeHavenAzureAPIError):
pass


class DataSafeHavenUserHandlingError(DataSafeHavenInternalError):
pass
class DataSafeHavenUserHandlingError(DataSafeHavenError):
"""
Exception class for handling errors related to user handling.

For example, when listing or registering users fails.
"""

class DataSafeHavenMicrosoftGraphError(DataSafeHavenAzureError):
pass


class DataSafeHavenPulumiError(DataSafeHavenCloudError):
class DataSafeHavenValueError(DataSafeHavenError):
"""
Exception class for handling errors related to value checking.

For example, when a function is called with an argument of the wrong value.
"""

pass
7 changes: 2 additions & 5 deletions data_safe_haven/external/api/azure_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@
from azure.storage.blob import BlobClient, BlobServiceClient
from azure.storage.filedatalake import DataLakeServiceClient

from data_safe_haven.exceptions import (
DataSafeHavenAzureError,
DataSafeHavenInternalError,
)
from data_safe_haven.exceptions import DataSafeHavenAzureError
from data_safe_haven.external.interface.azure_authenticator import AzureAuthenticator
from data_safe_haven.logging import get_logger

Expand Down Expand Up @@ -866,7 +863,7 @@ def remove_resource_group(self, resource_group_name: str) -> None:
]
if resource_groups:
msg = f"There are still {len(resource_groups)} resource group(s) remaining."
raise DataSafeHavenInternalError(msg)
raise DataSafeHavenAzureError(msg)
self.logger.info(
f"Ensured that resource group [green]{resource_group_name}[/] does not exist.",
)
Expand Down
Loading
Loading