From 0c4901e45513dfe78061ee3b79e8ccd36886bbc3 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 20 Jun 2024 15:16:04 +0100 Subject: [PATCH 01/10] :bug: Fix for multiline errors when writing to log files --- data_safe_haven/exceptions/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 4921dc4932..a907b551c1 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -7,7 +7,9 @@ def __init__(self, message: str | bytes): # Log exception message as an error logger = get_logger() - logger.error(message) + message_str = message if isinstance(message, str) else message.decode("utf-8") + for line in message_str.split("\n"): + logger.error(line) class DataSafeHavenCloudError(DataSafeHavenError): From ff5028b4bed1f5681e05e9cd0ecaacc636d68e10 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 20 Jun 2024 15:49:18 +0100 Subject: [PATCH 02/10] :sparkles: Use padding for additional lines instead --- data_safe_haven/exceptions/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index a907b551c1..9387fe150b 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -7,9 +7,10 @@ def __init__(self, message: str | bytes): # Log exception message as an error logger = get_logger() + # Pad additional lines with spaces to ensure they line-up + padding = " " * 34 # date (10) + 1 + time (12) + 3 + log_level (5) + 3 message_str = message if isinstance(message, str) else message.decode("utf-8") - for line in message_str.split("\n"): - logger.error(line) + logger.error(message_str.replace("\n", f"\n{padding}")) class DataSafeHavenCloudError(DataSafeHavenError): From 1340ec85242c7d757bd6d8a50f320fe59f2ed01c Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Fri, 21 Jun 2024 15:01:01 +0100 Subject: [PATCH 03/10] Fix linting --- data_safe_haven/exceptions/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 9387fe150b..e7c5aceeac 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -8,7 +8,7 @@ def __init__(self, message: str | bytes): # Log exception message as an error logger = get_logger() # Pad additional lines with spaces to ensure they line-up - padding = " " * 34 # date (10) + 1 + time (12) + 3 + log_level (5) + 3 + padding = " " * 34 # date (10) + 1 + time (12) + 3 + log_level (5) + 3 message_str = message if isinstance(message, str) else message.decode("utf-8") logger.error(message_str.replace("\n", f"\n{padding}")) From 08b56e515da3364d816a70943d417e6963d30230 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Mon, 24 Jun 2024 14:20:41 +0100 Subject: [PATCH 04/10] Join multiline error messages --- data_safe_haven/exceptions/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index e7c5aceeac..90598d8630 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -7,10 +7,9 @@ def __init__(self, message: str | bytes): # Log exception message as an error logger = get_logger() - # Pad additional lines with spaces to ensure they line-up - padding = " " * 34 # date (10) + 1 + time (12) + 3 + log_level (5) + 3 message_str = message if isinstance(message, str) else message.decode("utf-8") - logger.error(message_str.replace("\n", f"\n{padding}")) + # Replace line breaks with escape code + logger.error(message_str.replace("\n", r"\n")) class DataSafeHavenCloudError(DataSafeHavenError): From 883f8af2c8e00817a96c766c76669b2250e2d791 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Mon, 24 Jun 2024 14:31:34 +0100 Subject: [PATCH 05/10] Remove previous exception from exception messages --- .../administration/users/entra_users.py | 12 ++-- .../administration/users/user_handler.py | 12 ++-- data_safe_haven/commands/shm.py | 6 +- data_safe_haven/commands/sre.py | 6 +- data_safe_haven/commands/users.py | 10 ++-- .../context_infrastructure/infrastructure.py | 4 +- data_safe_haven/external/api/azure_api.py | 48 ++++++++------- data_safe_haven/external/api/graph_api.py | 58 +++++++++---------- .../external/interface/azure_authenticator.py | 2 +- .../interface/azure_container_instance.py | 4 +- .../interface/azure_postgresql_database.py | 4 +- data_safe_haven/functions/strings.py | 4 +- .../components/dynamic/blob_container_acl.py | 4 +- .../components/dynamic/entra_application.py | 8 +-- .../components/dynamic/file_share_file.py | 4 +- .../components/dynamic/ssl_certificate.py | 6 +- .../infrastructure/project_manager.py | 28 ++++----- .../serialisers/yaml_serialisable_model.py | 6 +- 18 files changed, 111 insertions(+), 115 deletions(-) diff --git a/data_safe_haven/administration/users/entra_users.py b/data_safe_haven/administration/users/entra_users.py index 6ef39a5744..d3d5dbd0ba 100644 --- a/data_safe_haven/administration/users/entra_users.py +++ b/data_safe_haven/administration/users/entra_users.py @@ -66,7 +66,7 @@ def add(self, new_users: Sequence[ResearchUser]) -> None: f"Ensured user '[green]{user.preferred_username}[/]' exists in Entra ID" ) except DataSafeHavenError as exc: - msg = f"Unable to add users to Entra ID.\n{exc}" + msg = "Unable to add users to Entra ID." raise DataSafeHavenEntraIDError(msg) from exc def list(self) -> Sequence[ResearchUser]: @@ -99,7 +99,7 @@ def list(self) -> Sequence[ResearchUser]: for user_details in user_list ] except DataSafeHavenError as exc: - msg = f"Unable list Entra ID users.\n{exc}" + msg = "Unable list Entra ID users." raise DataSafeHavenEntraIDError(msg) from exc def register(self, sre_name: str, usernames: Sequence[str]) -> None: @@ -114,7 +114,7 @@ def register(self, sre_name: str, usernames: Sequence[str]) -> None: for username in usernames: self.graph_api.add_user_to_group(username, group_name) except DataSafeHavenError as exc: - msg = f"Unable add users to group '{group_name}'.\n{exc}" + msg = f"Unable add users to group '{group_name}'." raise DataSafeHavenEntraIDError(msg) from exc def remove(self, users: Sequence[ResearchUser]) -> None: @@ -132,7 +132,7 @@ def remove(self, users: Sequence[ResearchUser]) -> None: self.graph_api.remove_user(user.username) self.logger.info(f"Removed '{user.preferred_username}'.") except DataSafeHavenError as exc: - msg = f"Unable to remove users from Entra ID.\n{exc}" + msg = "Unable to remove users from Entra ID." raise DataSafeHavenEntraIDError(msg) from exc def set(self, users: Sequence[ResearchUser]) -> None: @@ -148,7 +148,7 @@ def set(self, users: Sequence[ResearchUser]) -> None: users_to_add = [user for user in users if user not in self.list()] self.add(users_to_add) except DataSafeHavenError as exc: - msg = f"Unable to set desired user list in Entra ID.\n{exc}" + msg = "Unable to set desired user list in Entra ID." raise DataSafeHavenEntraIDError(msg) from exc def unregister(self, sre_name: str, usernames: Sequence[str]) -> None: @@ -163,5 +163,5 @@ def unregister(self, sre_name: str, usernames: Sequence[str]) -> None: for username in usernames: self.graph_api.remove_user_from_group(username, group_name) except DataSafeHavenError as exc: - msg = f"Unable to remove users from group {group_name}.\n{exc}" + msg = f"Unable to remove users from group {group_name}." raise DataSafeHavenEntraIDError(msg) from exc diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index f72817cf85..5ac393dc7f 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -68,7 +68,7 @@ def add(self, users_csv_path: pathlib.Path) -> None: # Add users to Entra ID self.entra_users.add(users) except Exception as exc: - msg = f"Could not add users from '{users_csv_path}'.\n{exc}" + msg = f"Could not add users from '{users_csv_path}'." raise DataSafeHavenUserHandlingError(msg) from exc def get_usernames(self, sre_name: str) -> dict[str, list[str]]: @@ -126,7 +126,7 @@ def list(self, sre_name: str) -> None: console.tabulate(user_headers, user_data) except Exception as exc: - msg = f"Could not list users.\n{exc}" + msg = "Could not list users." raise DataSafeHavenUserHandlingError(msg) from exc def register(self, sre_name: str, user_names: Sequence[str]) -> None: @@ -139,7 +139,7 @@ def register(self, sre_name: str, user_names: Sequence[str]) -> None: # Add users to the SRE security group self.entra_users.register(sre_name, user_names) except Exception as exc: - msg = f"Could not register {len(user_names)} users with SRE '{sre_name}'.\n{exc}" + msg = f"Could not register {len(user_names)} users with SRE '{sre_name}'." raise DataSafeHavenUserHandlingError(msg) from exc def remove(self, user_names: Sequence[str]) -> None: @@ -161,7 +161,7 @@ def remove(self, user_names: Sequence[str]) -> None: ) self.entra_users.remove(entra_users_to_remove) except Exception as exc: - msg = f"Could not remove users: {user_names}.\n{exc}" + msg = f"Could not remove users: {user_names}." raise DataSafeHavenUserHandlingError(msg) from exc def set(self, users_csv_path: str) -> None: @@ -208,7 +208,7 @@ def set(self, users_csv_path: str) -> None: # Commit changes self.entra_users.set(entra_desired_users) except Exception as exc: - msg = f"Could not set users from '{users_csv_path}'.\n{exc}" + msg = f"Could not set users from '{users_csv_path}'." raise DataSafeHavenUserHandlingError(msg) from exc def unregister(self, sre_name: str, user_names: Sequence[str]) -> None: @@ -221,5 +221,5 @@ def unregister(self, sre_name: str, user_names: Sequence[str]) -> None: # Remove users from the SRE security group self.entra_users.unregister(sre_name, user_names) except Exception as exc: - msg = f"Could not unregister {len(user_names)} users with SRE '{sre_name}'.\n{exc}" + msg = f"Could not unregister {len(user_names)} users with SRE '{sre_name}'." raise DataSafeHavenUserHandlingError(msg) from exc diff --git a/data_safe_haven/commands/shm.py b/data_safe_haven/commands/shm.py index 4d7c6bd93c..cfcee9beda 100644 --- a/data_safe_haven/commands/shm.py +++ b/data_safe_haven/commands/shm.py @@ -80,7 +80,7 @@ def deploy( # for figuring out what went wrong. # print("Could not deploy Data Safe Haven Management environment.") # raise typer.Exit(code=1) from exc - msg = f"Could not deploy Data Safe Haven Management environment.\n{exc}" + msg = "Could not deploy Data Safe Haven Management environment." raise DataSafeHavenError(msg) from exc finally: # Upload Pulumi config to blob storage @@ -104,7 +104,7 @@ def teardown() -> None: ) stack.teardown() except Exception as exc: - msg = f"Unable to teardown Pulumi infrastructure.\n{exc}" + msg = "Unable to teardown Pulumi infrastructure." raise DataSafeHavenInputError(msg) from exc # Remove information from config file @@ -113,5 +113,5 @@ def teardown() -> None: # Upload Pulumi config to blob storage pulumi_config.upload(context) except DataSafeHavenError as exc: - msg = f"Could not teardown Safe Haven Management environment.\n{exc}" + msg = "Could not teardown Safe Haven Management environment." raise DataSafeHavenError(msg) from exc diff --git a/data_safe_haven/commands/sre.py b/data_safe_haven/commands/sre.py index 9d6d3c7898..d80ddf8edc 100644 --- a/data_safe_haven/commands/sre.py +++ b/data_safe_haven/commands/sre.py @@ -103,7 +103,7 @@ def deploy( ) manager.run() except DataSafeHavenError as exc: - msg = f"Could not deploy Secure Research Environment {sre_config.safe_name}.\n{exc}" + msg = f"Could not deploy Secure Research Environment {sre_config.safe_name}." raise DataSafeHavenError(msg) from exc finally: # Upload Pulumi config to blob storage @@ -139,7 +139,7 @@ def teardown( ) stack.teardown() except Exception as exc: - msg = f"Unable to teardown Pulumi infrastructure.\n{exc}" + msg = "Unable to teardown Pulumi infrastructure." raise DataSafeHavenInputError(msg) from exc # Remove Pulumi project from Pulumi config file @@ -148,5 +148,5 @@ def teardown( # Upload Pulumi config to blob storage pulumi_config.upload(context) except DataSafeHavenError as exc: - msg = f"Could not teardown Secure Research Environment '{sre_config.safe_name}'.\n{exc}" + msg = f"Could not teardown Secure Research Environment '{sre_config.safe_name}'." raise DataSafeHavenError(msg) from exc diff --git a/data_safe_haven/commands/users.py b/data_safe_haven/commands/users.py index 656f6078a6..cfafa29e7b 100644 --- a/data_safe_haven/commands/users.py +++ b/data_safe_haven/commands/users.py @@ -51,7 +51,7 @@ 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}'.\n{exc}" + msg = f"Could not add users to Data Safe Haven '{shm_name}'." raise DataSafeHavenError(msg) from exc @@ -87,7 +87,7 @@ 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}'.\n{exc}" + msg = f"Could not list users for Data Safe Haven '{shm_name}'." raise DataSafeHavenError(msg) from exc @@ -155,7 +155,7 @@ 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}'.\n{exc}" + msg = f"Could not register users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'." raise DataSafeHavenError(msg) from exc @@ -194,7 +194,7 @@ 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}'.\n{exc}" + msg = f"Could not remove users from Data Safe Haven '{shm_name}'." raise DataSafeHavenError(msg) from exc @@ -263,5 +263,5 @@ 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}'.\n{exc}" + msg = f"Could not unregister users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'." raise DataSafeHavenError(msg) from exc diff --git a/data_safe_haven/context_infrastructure/infrastructure.py b/data_safe_haven/context_infrastructure/infrastructure.py index 327a48f759..f0a43cd0bb 100644 --- a/data_safe_haven/context_infrastructure/infrastructure.py +++ b/data_safe_haven/context_infrastructure/infrastructure.py @@ -80,7 +80,7 @@ def create(self) -> None: key_vault_name=keyvault.name, ) except DataSafeHavenAzureError as exc: - msg = f"Failed to create context resources.\n{exc}" + msg = "Failed to create context resources." raise DataSafeHavenAzureError(msg) from exc def teardown(self) -> None: @@ -96,5 +96,5 @@ def teardown(self) -> None: ) self.azure_api.remove_resource_group(self.context.resource_group_name) except DataSafeHavenAzureError as exc: - msg = f"Failed to destroy context resources.\n{exc}" + msg = "Failed to destroy context resources." raise DataSafeHavenAzureError(msg) from exc diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 35ec000e37..bf41a5267a 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -143,7 +143,7 @@ def download_blob( ) return str(blob_content) except AzureError as exc: - msg = f"Blob file '{blob_name}' could not be downloaded from '{storage_account_name}'\n{exc}." + msg = f"Blob file '{blob_name}' could not be downloaded from '{storage_account_name}'." raise DataSafeHavenAzureError(msg) from exc def ensure_dns_txt_record( @@ -183,9 +183,7 @@ def ensure_dns_txt_record( ) return record_set except AzureError as exc: - msg = ( - f"Failed to create DNS record {record_name} in zone {zone_name}.\n{exc}" - ) + msg = f"Failed to create DNS record {record_name} in zone {zone_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_keyvault( @@ -262,7 +260,7 @@ def ensure_keyvault( ) return key_vaults[0] except AzureError as exc: - msg = f"Failed to create key vault {key_vault_name}.\n{exc}" + msg = f"Failed to create key vault {key_vault_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_keyvault_key( @@ -299,7 +297,7 @@ def ensure_keyvault_key( ) return key except AzureError as exc: - msg = f"Failed to create key {key_name}.\n{exc}" + msg = f"Failed to create key {key_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_managed_identity( @@ -333,7 +331,7 @@ def ensure_managed_identity( ) return managed_identity except AzureError as exc: - msg = f"Failed to create managed identity {identity_name}.\n{exc}" + msg = f"Failed to create managed identity {identity_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_resource_group( @@ -375,7 +373,7 @@ def ensure_resource_group( ) return resource_groups[0] except AzureError as exc: - msg = f"Failed to create resource group {resource_group_name}.\n{exc}" + msg = f"Failed to create resource group {resource_group_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_storage_account( @@ -417,7 +415,7 @@ def ensure_storage_account( ) return storage_account except AzureError as exc: - msg = f"Failed to create storage account {storage_account_name}.\n{exc}" + msg = f"Failed to create storage account {storage_account_name}." raise DataSafeHavenAzureError(msg) from exc def ensure_storage_blob_container( @@ -452,7 +450,7 @@ def ensure_storage_blob_container( ) return container except HttpResponseError as exc: - msg = f"Failed to create storage container [green]{container_name}.\n{exc}" + msg = f"Failed to create storage container [green]{container_name}." raise DataSafeHavenAzureError(msg) from exc def get_keyvault_certificate( @@ -475,7 +473,7 @@ def get_keyvault_certificate( try: return certificate_client.get_certificate(certificate_name) except AzureError as exc: - msg = f"Failed to retrieve certificate {certificate_name}.\n{exc}" + msg = f"Failed to retrieve certificate {certificate_name}." raise DataSafeHavenAzureError(msg) from exc def get_keyvault_key(self, key_name: str, key_vault_name: str) -> KeyVaultKey: @@ -496,7 +494,7 @@ def get_keyvault_key(self, key_name: str, key_vault_name: str) -> KeyVaultKey: try: return key_client.get_key(key_name) except (ResourceNotFoundError, HttpResponseError) as exc: - msg = f"Failed to retrieve key {key_name}.\n{exc}" + msg = f"Failed to retrieve key {key_name}." raise DataSafeHavenAzureError(msg) from exc def get_keyvault_secret(self, key_vault_name: str, secret_name: str) -> str: @@ -520,7 +518,7 @@ def get_keyvault_secret(self, key_vault_name: str, secret_name: str) -> str: msg = f"Secret {secret_name} has no value." raise DataSafeHavenAzureError(msg) except AzureError as exc: - msg = f"Failed to retrieve secret {secret_name}.\n{exc}" + msg = f"Failed to retrieve secret {secret_name}." raise DataSafeHavenAzureError(msg) from exc def get_locations(self) -> list[str]: @@ -541,7 +539,7 @@ def get_locations(self) -> list[str]: ) ] except AzureError as exc: - msg = f"Azure locations could not be loaded.\n{exc}" + msg = "Azure locations could not be loaded." raise DataSafeHavenAzureError(msg) from exc def get_storage_account_keys( @@ -581,7 +579,7 @@ def get_storage_account_keys( raise DataSafeHavenAzureError(msg) return keys except AzureError as exc: - msg = f"Keys could not be loaded for {msg_sa} in {msg_rg}.\n{exc}" + msg = f"Keys could not be loaded for {msg_sa} in {msg_rg}." raise DataSafeHavenAzureError(msg) from exc def import_keyvault_certificate( @@ -625,7 +623,7 @@ def import_keyvault_certificate( ) return certificate except AzureError as exc: - msg = f"Failed to import certificate '{certificate_name}'.\n{exc}" + msg = f"Failed to import certificate '{certificate_name}'." raise DataSafeHavenAzureError(msg) from exc def list_available_vm_skus(self, location: str) -> dict[str, dict[str, Any]]: @@ -653,7 +651,7 @@ def list_available_vm_skus(self, location: str) -> dict[str, dict[str, Any]]: skus[resource_sku.name][capability.name] = capability.value return skus except AzureError as exc: - msg = f"Failed to load available VM sizes for Azure location {location}.\n{exc}" + msg = f"Failed to load available VM sizes for Azure location {location}." raise DataSafeHavenAzureError(msg) from exc def purge_keyvault_certificate( @@ -690,7 +688,7 @@ def purge_keyvault_certificate( f"Purged certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) except AzureError as exc: - msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" + msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'." raise DataSafeHavenAzureError(msg) from exc def remove_blob( @@ -728,7 +726,7 @@ def remove_blob( f"Removed file [green]{blob_name}[/] from blob storage.", ) except AzureError as exc: - msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/].\n{exc}" + msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/]." raise DataSafeHavenAzureError(msg) from exc def remove_dns_txt_record( @@ -772,7 +770,7 @@ def remove_dns_txt_record( f"Ensured that DNS record [green]{record_name}[/] is removed from zone [green]{zone_name}[/].", ) except AzureError as exc: - msg = f"Failed to remove DNS record [green]{record_name}[/] from zone [green]{zone_name}[/].\n{exc}" + msg = f"Failed to remove DNS record [green]{record_name}[/] from zone [green]{zone_name}[/]." raise DataSafeHavenAzureError(msg) from exc def remove_keyvault_certificate( @@ -829,7 +827,7 @@ def remove_keyvault_certificate( f"Removed certificate [green]{certificate_name}[/] from Key Vault [green]{key_vault_name}[/].", ) except AzureError as exc: - msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'.\n{exc}" + msg = f"Failed to remove certificate '{certificate_name}' from Key Vault '{key_vault_name}'." raise DataSafeHavenAzureError(msg) from exc def remove_resource_group(self, resource_group_name: str) -> None: @@ -873,7 +871,7 @@ def remove_resource_group(self, resource_group_name: str) -> None: f"Ensured that resource group [green]{resource_group_name}[/] does not exist.", ) except AzureError as exc: - msg = f"Failed to remove resource group {resource_group_name}.\n{exc}" + msg = f"Failed to remove resource group {resource_group_name}." raise DataSafeHavenAzureError(msg) from exc def run_remote_script( @@ -925,7 +923,7 @@ def run_remote_script( # Return any stdout/stderr from the command return str(result.value[0].message) if result.value else "" except AzureError as exc: - msg = f"Failed to run command on '{vm_name}'.\n{exc}" + msg = f"Failed to run command on '{vm_name}'." raise DataSafeHavenAzureError(msg) from exc def run_remote_script_waiting( @@ -1006,7 +1004,7 @@ def set_blob_container_acl( # Set the desired ACL directory_client.set_access_control_recursive(acl=desired_acl) except AzureError as exc: - msg = f"Failed to set ACL '{desired_acl}' on container '{container_name}'.\n{exc}" + msg = f"Failed to set ACL '{desired_acl}' on container '{container_name}'." raise DataSafeHavenAzureError(msg) from exc def upload_blob( @@ -1038,5 +1036,5 @@ def upload_blob( f"Uploaded file [green]{blob_name}[/] to blob storage.", ) except AzureError as exc: - msg = f"Blob file '{blob_name}' could not be uploaded to '{storage_account_name}'\n{exc}." + msg = f"Blob file '{blob_name}' could not be uploaded to '{storage_account_name}'." raise DataSafeHavenAzureError(msg) from exc diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 03a4264d87..555b70fb68 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -127,7 +127,7 @@ def add_custom_domain(self, domain_name: str) -> str: raise DataSafeHavenMicrosoftGraphError(msg) return txt_records[0] except Exception as exc: - msg = f"Could not register domain '{domain_name}'.\n{exc}" + msg = f"Could not register domain '{domain_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def add_user_to_group( @@ -164,7 +164,7 @@ def add_user_to_group( f"Added user [green]'{username}'[/] to group [green]'{group_name}'[/]." ) except DataSafeHavenMicrosoftGraphError as exc: - msg = f"Could not add user '{username}' to group '{group_name}'.\n{exc}" + msg = f"Could not add user '{username}' to group '{group_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_application( @@ -261,7 +261,7 @@ def create_application( # Return JSON representation of the Entra application return json_response except Exception as exc: - msg = f"Could not create application '{application_name}'.\n{exc}" + msg = f"Could not create application '{application_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_application_secret( @@ -309,7 +309,7 @@ def create_application_secret( ) return str(json_response["secretText"]) except Exception as exc: - msg = f"Could not create application secret '{application_secret_name}'.\n{exc}" + msg = f"Could not create application secret '{application_secret_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_group(self, group_name: str) -> None: @@ -343,7 +343,7 @@ def create_group(self, group_name: str) -> None: f"Created Entra group '[green]{group_name}[/]'.", ) except Exception as exc: - msg = f"Could not create Entra group '{group_name}'.\n{exc}" + msg = f"Could not create Entra group '{group_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def ensure_application_service_principal( @@ -379,7 +379,7 @@ def ensure_application_service_principal( raise DataSafeHavenMicrosoftGraphError(msg) return application_sp except Exception as exc: - msg = f"Could not create service principal for application '[green]{application_name}[/]'.\n{exc}" + msg = f"Could not create service principal for application '[green]{application_name}[/]'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_token_administrator(self) -> str: @@ -430,7 +430,7 @@ def create_token_administrator(self) -> str: error_description = "Could not create Microsoft Graph access token." if isinstance(result, dict) and "error_description" in result: error_description += f"\n{result['error_description']}." - msg = f"{error_description}\n{exc}" + msg = f"{error_description}" raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_token_application( @@ -462,7 +462,7 @@ def create_token_application( error_description = "Could not create access token" if result and "error_description" in result: error_description += f": {result['error_description']}" - msg = f"{error_description}.\n{exc}" + msg = f"{error_description}." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_user( @@ -506,7 +506,9 @@ def create_user( ) except DataSafeHavenMicrosoftGraphError as exc: if "already registered" not in str(exc): - msg = f"Failed to add authentication email address '{email_address}'.\n{exc}" + msg = ( + f"Failed to add authentication email address '{email_address}'." + ) raise DataSafeHavenMicrosoftGraphError(msg) from exc # Set the authentication phone number try: @@ -516,7 +518,7 @@ def create_user( ) except DataSafeHavenMicrosoftGraphError as exc: if "already registered" not in str(exc): - msg = f"Failed to add authentication phone number '{phone_number}'.\n{exc}" + msg = f"Failed to add authentication phone number '{phone_number}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc # Ensure user is enabled self.http_patch( @@ -527,7 +529,7 @@ def create_user( f"{final_verb}d Entra user '[green]{username}[/]'.", ) except DataSafeHavenMicrosoftGraphError as exc: - msg = f"Could not {final_verb.lower()} user {username}.\n{exc}" + msg = f"Could not {final_verb.lower()} user {username}." raise DataSafeHavenMicrosoftGraphError(msg) from exc def delete_application( @@ -552,7 +554,7 @@ def delete_application( f"Deleted application '[green]{application_name}[/]'.", ) except Exception as exc: - msg = f"Could not delete application '{application_name}'.\n{exc}" + msg = f"Could not delete application '{application_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def get_application_by_name(self, application_name: str) -> dict[str, Any] | None: @@ -685,7 +687,7 @@ def grant_application_role_permissions( f"Assigned application role '[green]{application_role_name}[/]' to '{application_name}'.", ) except Exception as exc: - msg = f"Could not assign application role '{application_role_name}' to application '{application_name}'.\n{exc}" + msg = f"Could not assign application role '{application_role_name}' to application '{application_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def grant_delegated_role_permissions( @@ -749,7 +751,7 @@ def grant_delegated_role_permissions( f"Assigned delegated role '[green]{application_role_name}[/]' to '{application_name}'.", ) except Exception as exc: - msg = f"Could not assign delegated role '{application_role_name}' to application '{application_name}'.\n{exc}" + msg = f"Could not assign delegated role '{application_role_name}' to application '{application_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def http_delete(self, url: str, **kwargs: Any) -> requests.Response: @@ -777,7 +779,7 @@ def http_delete(self, url: str, **kwargs: Any) -> requests.Response: return response raise DataSafeHavenInternalError(response.content) except Exception as exc: - msg = f"Could not execute DELETE request to '{url}'.\n{exc}" + msg = f"Could not execute DELETE request to '{url}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def http_get(self, url: str, **kwargs: Any) -> requests.Response: @@ -805,7 +807,7 @@ def http_get(self, url: str, **kwargs: Any) -> requests.Response: return response raise DataSafeHavenInternalError(response.content) except Exception as exc: - msg = f"Could not execute GET request from '{url}'.\n{exc}" + msg = f"Could not execute GET request from '{url}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def http_patch(self, url: str, **kwargs: Any) -> requests.Response: @@ -833,7 +835,7 @@ def http_patch(self, url: str, **kwargs: Any) -> requests.Response: return response raise DataSafeHavenInternalError(response.content) except Exception as exc: - msg = f"Could not execute PATCH request to '{url}'.\n{exc}" + msg = f"Could not execute PATCH request to '{url}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def http_post(self, url: str, **kwargs: Any) -> requests.Response: @@ -862,7 +864,7 @@ def http_post(self, url: str, **kwargs: Any) -> requests.Response: return response raise DataSafeHavenInternalError(response.content) except Exception as exc: - msg = f"Could not execute POST request to '{url}'.\n{exc}" + msg = f"Could not execute POST request to '{url}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_applications(self) -> Sequence[dict[str, Any]]: @@ -882,7 +884,7 @@ def read_applications(self) -> Sequence[dict[str, Any]]: ] ] except Exception as exc: - msg = f"Could not load list of applications.\n{exc}" + msg = "Could not load list of applications." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_application_permissions( @@ -905,7 +907,7 @@ def read_application_permissions( ).json()["value"] return [dict(obj) for obj in (delegated + application)] except Exception as exc: - msg = f"Could not load list of application permissions.\n{exc}" + msg = "Could not load list of application permissions." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_domains(self) -> Sequence[dict[str, Any]]: @@ -921,7 +923,7 @@ def read_domains(self) -> Sequence[dict[str, Any]]: json_response = self.http_get(f"{self.base_endpoint}/domains").json() return [dict(obj) for obj in json_response["value"]] except Exception as exc: - msg = f"Could not load list of domains.\n{exc}" + msg = "Could not load list of domains." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_groups( @@ -942,7 +944,7 @@ def read_groups( endpoint += f"?$select={','.join(attributes)}" return [dict(obj) for obj in self.http_get(endpoint).json()["value"]] except Exception as exc: - msg = f"Could not load list of groups.\n{exc}" + msg = "Could not load list of groups." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_service_principals(self) -> Sequence[dict[str, Any]]: @@ -955,7 +957,7 @@ def read_service_principals(self) -> Sequence[dict[str, Any]]: ).json()["value"] ] except Exception as exc: - msg = f"Could not load list of service principals.\n{exc}" + msg = "Could not load list of service principals." raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_users( @@ -1005,7 +1007,7 @@ def read_users( ) return users except Exception as exc: - msg = f"Could not load list of users.\n{exc}" + msg = "Could not load list of users." raise DataSafeHavenMicrosoftGraphError(msg) from exc def remove_user( @@ -1025,7 +1027,7 @@ def remove_user( ) return except Exception as exc: - msg = f"Could not remove user '{username}'.\n{exc}" + msg = f"Could not remove user '{username}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def remove_user_from_group( @@ -1060,9 +1062,7 @@ def remove_user_from_group( f"User [green]'{username}'[/] does not belong to group [green]'{group_name}'[/]." ) except Exception as exc: - msg = ( - f"Could not remove user '{username}' from group '{group_name}'.\n{exc}" - ) + msg = f"Could not remove user '{username}' from group '{group_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def verify_custom_domain( @@ -1125,5 +1125,5 @@ def verify_custom_domain( if not response.json()["isVerified"]: raise DataSafeHavenMicrosoftGraphError(response.content) except Exception as exc: - msg = f"Could not verify domain '{domain_name}'.\n{exc}" + msg = f"Could not verify domain '{domain_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc diff --git a/data_safe_haven/external/interface/azure_authenticator.py b/data_safe_haven/external/interface/azure_authenticator.py index 4d7a4641b0..b5e765e02c 100644 --- a/data_safe_haven/external/interface/azure_authenticator.py +++ b/data_safe_haven/external/interface/azure_authenticator.py @@ -64,7 +64,7 @@ def login(self) -> None: self.tenant_id_ = subscription.tenant_id break except ClientAuthenticationError as exc: - msg = f"Failed to authenticate with Azure API.\n{exc}" + msg = "Failed to authenticate with Azure API." raise DataSafeHavenAzureAPIAuthenticationError(msg) from exc if not (self.subscription_id and self.tenant_id): msg = f"Could not find subscription '{self.subscription_name}'" diff --git a/data_safe_haven/external/interface/azure_container_instance.py b/data_safe_haven/external/interface/azure_container_instance.py index 012d29f429..b2e5cbdafa 100644 --- a/data_safe_haven/external/interface/azure_container_instance.py +++ b/data_safe_haven/external/interface/azure_container_instance.py @@ -86,9 +86,7 @@ def restart(self, target_ip_address: str | None = None) -> None: f" with IP address [green]{self.current_ip_address}[/].", ) except Exception as exc: - msg = ( - f"Could not restart container group {self.container_group_name}.\n{exc}" - ) + msg = f"Could not restart container group {self.container_group_name}." raise DataSafeHavenAzureError(msg) from exc def run_executable(self, container_name: str, executable_path: str) -> list[str]: diff --git a/data_safe_haven/external/interface/azure_postgresql_database.py b/data_safe_haven/external/interface/azure_postgresql_database.py index 4c089c363b..e40d7beee1 100644 --- a/data_safe_haven/external/interface/azure_postgresql_database.py +++ b/data_safe_haven/external/interface/azure_postgresql_database.py @@ -110,7 +110,7 @@ def db_connection(self, n_retries: int = 0) -> psycopg.Connection: n_retries -= 1 time.sleep(10) except Exception as exc: - msg = f"Could not connect to database.\n{exc}" + msg = "Could not connect to database." raise DataSafeHavenAzureError(msg) from exc return connection @@ -158,7 +158,7 @@ def execute_scripts( connection.commit() self.logger.debug(f"Finished running {len(filepaths)} SQL scripts.") except (Exception, psycopg.Error) as exc: - msg = f"Error while connecting to PostgreSQL.\n{exc}" + msg = "Error while connecting to PostgreSQL." raise DataSafeHavenAzureError(msg) from exc finally: # Close the connection if it is open diff --git a/data_safe_haven/functions/strings.py b/data_safe_haven/functions/strings.py index 0c3695da08..5ec11f664d 100644 --- a/data_safe_haven/functions/strings.py +++ b/data_safe_haven/functions/strings.py @@ -63,10 +63,10 @@ def next_occurrence( msg = f"Time format '{time_format}' was not recognised." raise DataSafeHavenInputError(msg) except pytz.exceptions.UnknownTimeZoneError as exc: - msg = f"Timezone '{timezone}' was not recognised.\n{exc}" + msg = f"Timezone '{timezone}' was not recognised." raise DataSafeHavenInputError(msg) from exc except ValueError as exc: - msg = f"Time '{hour}:{minute}' was not recognised.\n{exc}" + msg = f"Time '{hour}:{minute}' was not recognised." raise DataSafeHavenInputError(msg) from exc diff --git a/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py b/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py index 8c2291c501..509683291d 100644 --- a/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py +++ b/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py @@ -63,7 +63,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: storage_account_name=props["storage_account_name"], ) except Exception as exc: - msg = f"Failed to set ACLs on storage account [green]{props['storage_account_name']}[/].\n{exc}" + msg = f"Failed to set ACLs on storage account [green]{props['storage_account_name']}[/]." raise DataSafeHavenPulumiError(msg) from exc return CreateResult( f"BlobContainerAcl-{props['container_name']}", @@ -83,7 +83,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: storage_account_name=props["storage_account_name"], ) except Exception as exc: - msg = f"Failed to delete custom ACLs on storage account [green]{props['storage_account_name']}[/].\n{exc}" + msg = f"Failed to delete custom ACLs on storage account [green]{props['storage_account_name']}[/]." raise DataSafeHavenPulumiError(msg) from exc def diff( diff --git a/data_safe_haven/infrastructure/components/dynamic/entra_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_application.py index 0fc4d11eb6..5f8464068c 100644 --- a/data_safe_haven/infrastructure/components/dynamic/entra_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_application.py @@ -59,7 +59,7 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]: ) return outs except Exception as exc: - msg = f"Failed to refresh application [green]{props['application_name']}[/] in Entra ID.\n{exc}" + msg = f"Failed to refresh application [green]{props['application_name']}[/] in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create(self, props: dict[str, Any]) -> CreateResult: @@ -110,7 +110,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: else "" ) except Exception as exc: - msg = f"Failed to create application [green]{props['application_name']}[/] in Entra ID.\n{exc}" + msg = f"Failed to create application [green]{props['application_name']}[/] in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc return CreateResult( f"EntraApplication-{props['application_name']}", @@ -125,7 +125,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: graph_api = GraphApi(auth_token=props["auth_token"]) graph_api.delete_application(props["application_name"]) except Exception as exc: - msg = f"Failed to delete application [green]{props['application_name']}[/] from Entra ID.\n{exc}" + msg = f"Failed to delete application [green]{props['application_name']}[/] from Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc def diff( @@ -156,7 +156,7 @@ def update( updated = self.create(new_props) return UpdateResult(outs=updated.outs) except Exception as exc: - msg = f"Failed to update application [green]{new_props['application_name']}[/] in Entra ID.\n{exc}" + msg = f"Failed to update application [green]{new_props['application_name']}[/] in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc diff --git a/data_safe_haven/infrastructure/components/dynamic/file_share_file.py b/data_safe_haven/infrastructure/components/dynamic/file_share_file.py index 3420e7a34a..0ce98a1d66 100644 --- a/data_safe_haven/infrastructure/components/dynamic/file_share_file.py +++ b/data_safe_haven/infrastructure/components/dynamic/file_share_file.py @@ -94,7 +94,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: outs["file_name"] = file_client.file_name except Exception as exc: file_name = file_client.file_name if file_client else "" - msg = f"Failed to upload data to [green]{file_name}[/] in [green]{props['share_name']}[/].\n{exc}" + msg = f"Failed to upload data to [green]{file_name}[/] in [green]{props['share_name']}[/]." raise DataSafeHavenAzureError(msg) from exc return CreateResult( f"filesharefile-{props['destination_path'].replace('/', '-')}", @@ -118,7 +118,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: file_client.delete_file() except Exception as exc: file_name = file_client.file_name if file_client else "" - msg = f"Failed to delete file [green]{file_name}[/] in [green]{props['share_name']}[/].\n{exc}" + msg = f"Failed to delete file [green]{file_name}[/] in [green]{props['share_name']}[/]." raise DataSafeHavenAzureError(msg) from exc def diff( diff --git a/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py b/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py index 7cb967af95..ad07ea5f14 100644 --- a/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py +++ b/data_safe_haven/infrastructure/components/dynamic/ssl_certificate.py @@ -58,7 +58,7 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]: except Exception as exc: cert_name = f"[green]{props['certificate_secret_name']}[/]" domain_name = f"[green]{props['domain_name']}[/]" - msg = f"Failed to refresh SSL certificate {cert_name} for {domain_name}.\n{exc}" + msg = f"Failed to refresh SSL certificate {cert_name} for {domain_name}." raise DataSafeHavenSSLError(msg) from exc def create(self, props: dict[str, Any]) -> CreateResult: @@ -140,7 +140,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: except Exception as exc: cert_name = f"[green]{props['certificate_secret_name']}[/]" domain_name = f"[green]{props['domain_name']}[/]" - msg = f"Failed to create SSL certificate {cert_name} for {domain_name}.\n{exc}" + msg = f"Failed to create SSL certificate {cert_name} for {domain_name}." raise DataSafeHavenSSLError(msg) from exc return CreateResult( f"SSLCertificate-{props['certificate_secret_name']}", @@ -167,7 +167,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: except Exception as exc: cert_name = f"[green]{props['certificate_secret_name']}[/]" domain_name = f"[green]{props['domain_name']}[/]" - msg = f"Failed to delete SSL certificate {cert_name} for {domain_name}.\n{exc}" + msg = f"Failed to delete SSL certificate {cert_name} for {domain_name}." raise DataSafeHavenSSLError(msg) from exc def diff( diff --git a/data_safe_haven/infrastructure/project_manager.py b/data_safe_haven/infrastructure/project_manager.py index d14d71e671..1b982232a1 100644 --- a/data_safe_haven/infrastructure/project_manager.py +++ b/data_safe_haven/infrastructure/project_manager.py @@ -115,7 +115,7 @@ def pulumi_project(self) -> DSHPulumiProject: try: self._pulumi_project = self.pulumi_config[self.pulumi_project_name] except (KeyError, TypeError) as exc: - msg = f"No SHM/SRE named {self.pulumi_project_name} is defined.\n{exc}" + msg = f"No SHM/SRE named {self.pulumi_project_name} is defined." raise DataSafeHavenConfigError(msg) from exc return self._pulumi_project @@ -140,7 +140,7 @@ def stack(self) -> automation.Stack: # Ensure encrypted key is stored in the Pulumi configuration self.update_dsh_pulumi_config() except automation.CommandError as exc: - msg = f"Could not load Pulumi stack {self.stack_name}.\n{exc}" + msg = f"Could not load Pulumi stack {self.stack_name}." raise DataSafeHavenPulumiError(msg) from exc return self._stack @@ -163,7 +163,7 @@ def apply_config_options(self) -> None: self.ensure_config(name, value, secret=is_secret) self.options = {} except Exception as exc: - msg = f"Applying Pulumi configuration options failed.\n{exc}." + msg = "Applying Pulumi configuration options failed.." raise DataSafeHavenPulumiError(msg) from exc def cancel(self) -> None: @@ -196,7 +196,7 @@ def deploy(self, *, force: bool = False) -> None: self.preview() self.update() except Exception as exc: - msg = f"Pulumi deployment failed.\n{exc}" + msg = "Pulumi deployment failed." raise DataSafeHavenPulumiError(msg) from exc def destroy(self) -> None: @@ -222,7 +222,7 @@ def destroy(self) -> None: ): time.sleep(10) else: - msg = f"Pulumi resource destruction failed.\n{exc}" + msg = "Pulumi resource destruction failed." raise DataSafeHavenPulumiError(msg) from exc # Remove stack JSON try: @@ -232,7 +232,7 @@ def destroy(self) -> None: self.logger.info(f"Removed Pulumi stack [green]{self.stack_name}[/].") except automation.CommandError as exc: if "no stack named" not in str(exc): - msg = f"Pulumi stack could not be removed.\n{exc}" + msg = "Pulumi stack could not be removed." raise DataSafeHavenPulumiError(msg) from exc # Remove stack JSON backup stack_backup_name = f"{self.stack_name}.json.bak" @@ -256,10 +256,10 @@ def destroy(self) -> None: f"Pulumi stack backup [green]{stack_backup_name}[/] could not be found." ) else: - msg = f"Pulumi stack backup could not be removed.\n{exc}" + msg = "Pulumi stack backup could not be removed." raise DataSafeHavenPulumiError(msg) from exc except DataSafeHavenPulumiError as exc: - msg = f"Pulumi destroy failed.\n{exc}" + msg = "Pulumi destroy failed." raise DataSafeHavenPulumiError(msg) from exc def ensure_config(self, name: str, value: str, *, secret: bool) -> None: @@ -289,7 +289,7 @@ def install_plugins(self) -> None: "random", metadata.version("pulumi-random") ) except Exception as exc: - msg = f"Installing Pulumi plugins failed.\n{exc}." + msg = "Installing Pulumi plugins failed.." raise DataSafeHavenPulumiError(msg) from exc def log_message(self, message: str) -> None: @@ -315,7 +315,7 @@ def preview(self) -> None: **self.pulumi_extra_args, ) except Exception as exc: - msg = f"Pulumi preview failed.\n{exc}." + msg = "Pulumi preview failed.." raise DataSafeHavenPulumiError(msg) from exc def refresh(self) -> None: @@ -325,7 +325,7 @@ def refresh(self) -> None: # Note that we disable parallelisation which can cause deadlock self.stack.refresh(parallel=1, **self.pulumi_extra_args) except automation.CommandError as exc: - msg = f"Pulumi refresh failed.\n{exc}" + msg = "Pulumi refresh failed." raise DataSafeHavenPulumiError(msg) from exc def run_pulumi_command(self, command: str) -> str: @@ -334,7 +334,7 @@ def run_pulumi_command(self, command: str) -> str: result = self.stack._run_pulumi_cmd_sync(command.split()) return str(result.stdout) except automation.CommandError as exc: - msg = f"Failed to run command.\n{exc}" + msg = "Failed to run command." raise DataSafeHavenPulumiError(msg) from exc def secret(self, name: str) -> str: @@ -357,7 +357,7 @@ def teardown(self) -> None: self.destroy() self.update_dsh_pulumi_project() except Exception as exc: - msg = f"Tearing down Pulumi infrastructure failed.\n{exc}." + msg = "Tearing down Pulumi infrastructure failed.." raise DataSafeHavenPulumiError(msg) from exc def update(self) -> None: @@ -370,7 +370,7 @@ def update(self) -> None: self.evaluate(result.summary.result) self.update_dsh_pulumi_project() except automation.CommandError as exc: - msg = f"Pulumi update failed.\n{exc}" + msg = "Pulumi update failed." raise DataSafeHavenPulumiError(msg) from exc @property diff --git a/data_safe_haven/serialisers/yaml_serialisable_model.py b/data_safe_haven/serialisers/yaml_serialisable_model.py index e2fd0ca23f..78ecf51b9f 100644 --- a/data_safe_haven/serialisers/yaml_serialisable_model.py +++ b/data_safe_haven/serialisers/yaml_serialisable_model.py @@ -31,7 +31,7 @@ def from_filepath(cls: type[T], config_file_path: PathType) -> T: settings_yaml = f_yaml.read() return cls.from_yaml(settings_yaml) except FileNotFoundError as exc: - msg = f"Could not find file {config_file_path}.\n{exc}" + msg = f"Could not find file {config_file_path}." raise DataSafeHavenConfigError(msg) from exc @classmethod @@ -40,7 +40,7 @@ def from_yaml(cls: type[T], settings_yaml: str) -> T: try: settings_dict = yaml.safe_load(settings_yaml) except yaml.YAMLError as exc: - msg = f"Could not parse {cls.config_type} configuration as YAML.\n{exc}" + msg = f"Could not parse {cls.config_type} configuration as YAML." raise DataSafeHavenConfigError(msg) from exc if not isinstance(settings_dict, dict): @@ -50,7 +50,7 @@ def from_yaml(cls: type[T], settings_yaml: str) -> T: try: return cls.model_validate(settings_dict) except ValidationError as exc: - msg = f"Could not load {cls.config_type} configuration.\n{exc}" + msg = f"Could not load {cls.config_type} configuration." raise DataSafeHavenParameterError(msg) from exc def to_filepath(self, config_file_path: PathType) -> None: From a1fe5a16d5db36a57e777963455dae77451c0b74 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 24 Jun 2024 23:15:45 +0100 Subject: [PATCH 06/10] :memo: Fix grammar --- data_safe_haven/administration/users/entra_users.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/administration/users/entra_users.py b/data_safe_haven/administration/users/entra_users.py index d3d5dbd0ba..248a11376a 100644 --- a/data_safe_haven/administration/users/entra_users.py +++ b/data_safe_haven/administration/users/entra_users.py @@ -99,7 +99,7 @@ def list(self) -> Sequence[ResearchUser]: for user_details in user_list ] except DataSafeHavenError as exc: - msg = "Unable list Entra ID users." + msg = "Unable to list Entra ID users." raise DataSafeHavenEntraIDError(msg) from exc def register(self, sre_name: str, usernames: Sequence[str]) -> None: @@ -114,7 +114,7 @@ def register(self, sre_name: str, usernames: Sequence[str]) -> None: for username in usernames: self.graph_api.add_user_to_group(username, group_name) except DataSafeHavenError as exc: - msg = f"Unable add users to group '{group_name}'." + msg = f"Unable to add users to group '{group_name}'." raise DataSafeHavenEntraIDError(msg) from exc def remove(self, users: Sequence[ResearchUser]) -> None: From f4284c6294c469861de2c117692df1d613584304 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Tue, 25 Jun 2024 10:58:46 +0100 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: James Robinson --- data_safe_haven/external/api/azure_api.py | 4 ++-- .../components/dynamic/blob_container_acl.py | 4 ++-- .../components/dynamic/entra_application.py | 8 ++++---- .../infrastructure/components/dynamic/file_share_file.py | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index bf41a5267a..09ec85a5bf 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -726,7 +726,7 @@ def remove_blob( f"Removed file [green]{blob_name}[/] from blob storage.", ) except AzureError as exc: - msg = f"Blob file [green]'{blob_name}'[/] could not be removed from [green]'{storage_account_name}'[/]." + msg = f"Blob file '{blob_name}' could not be removed from '{storage_account_name}'." raise DataSafeHavenAzureError(msg) from exc def remove_dns_txt_record( @@ -770,7 +770,7 @@ def remove_dns_txt_record( f"Ensured that DNS record [green]{record_name}[/] is removed from zone [green]{zone_name}[/].", ) except AzureError as exc: - msg = f"Failed to remove DNS record [green]{record_name}[/] from zone [green]{zone_name}[/]." + msg = f"Failed to remove DNS record {record_name} from zone {zone_name}." raise DataSafeHavenAzureError(msg) from exc def remove_keyvault_certificate( diff --git a/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py b/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py index 509683291d..fe433d1ca7 100644 --- a/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py +++ b/data_safe_haven/infrastructure/components/dynamic/blob_container_acl.py @@ -63,7 +63,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: storage_account_name=props["storage_account_name"], ) except Exception as exc: - msg = f"Failed to set ACLs on storage account [green]{props['storage_account_name']}[/]." + msg = f"Failed to set ACLs on storage account '{props['storage_account_name']}'." raise DataSafeHavenPulumiError(msg) from exc return CreateResult( f"BlobContainerAcl-{props['container_name']}", @@ -83,7 +83,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: storage_account_name=props["storage_account_name"], ) except Exception as exc: - msg = f"Failed to delete custom ACLs on storage account [green]{props['storage_account_name']}[/]." + msg = f"Failed to delete custom ACLs on storage account '{props['storage_account_name']}'." raise DataSafeHavenPulumiError(msg) from exc def diff( diff --git a/data_safe_haven/infrastructure/components/dynamic/entra_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_application.py index 5f8464068c..0e13dfc7e8 100644 --- a/data_safe_haven/infrastructure/components/dynamic/entra_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_application.py @@ -59,7 +59,7 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]: ) return outs except Exception as exc: - msg = f"Failed to refresh application [green]{props['application_name']}[/] in Entra ID." + msg = f"Failed to refresh application '{props['application_name']}' in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create(self, props: dict[str, Any]) -> CreateResult: @@ -110,7 +110,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: else "" ) except Exception as exc: - msg = f"Failed to create application [green]{props['application_name']}[/] in Entra ID." + msg = f"Failed to create application '{props['application_name']}' in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc return CreateResult( f"EntraApplication-{props['application_name']}", @@ -125,7 +125,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: graph_api = GraphApi(auth_token=props["auth_token"]) graph_api.delete_application(props["application_name"]) except Exception as exc: - msg = f"Failed to delete application [green]{props['application_name']}[/] from Entra ID." + msg = f"Failed to delete application '{props['application_name']}' from Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc def diff( @@ -156,7 +156,7 @@ def update( updated = self.create(new_props) return UpdateResult(outs=updated.outs) except Exception as exc: - msg = f"Failed to update application [green]{new_props['application_name']}[/] in Entra ID." + msg = f"Failed to update application '{new_props['application_name']}' in Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) from exc diff --git a/data_safe_haven/infrastructure/components/dynamic/file_share_file.py b/data_safe_haven/infrastructure/components/dynamic/file_share_file.py index 0ce98a1d66..6e7f146efd 100644 --- a/data_safe_haven/infrastructure/components/dynamic/file_share_file.py +++ b/data_safe_haven/infrastructure/components/dynamic/file_share_file.py @@ -94,7 +94,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: outs["file_name"] = file_client.file_name except Exception as exc: file_name = file_client.file_name if file_client else "" - msg = f"Failed to upload data to [green]{file_name}[/] in [green]{props['share_name']}[/]." + msg = f"Failed to upload data to '{file_name}' in [green]{props['share_name']}[/]." raise DataSafeHavenAzureError(msg) from exc return CreateResult( f"filesharefile-{props['destination_path'].replace('/', '-')}", @@ -118,7 +118,7 @@ def delete(self, id_: str, props: dict[str, Any]) -> None: file_client.delete_file() except Exception as exc: file_name = file_client.file_name if file_client else "" - msg = f"Failed to delete file [green]{file_name}[/] in [green]{props['share_name']}[/]." + msg = f"Failed to delete file '{file_name}' in [green]{props['share_name']}[/]." raise DataSafeHavenAzureError(msg) from exc def diff( From 12b56c6226a2e36fbe8ef7255acdb0da7e35ac07 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Tue, 25 Jun 2024 10:59:39 +0100 Subject: [PATCH 08/10] Fix linting --- data_safe_haven/commands/sre.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/commands/sre.py b/data_safe_haven/commands/sre.py index d80ddf8edc..8607b8f8fa 100644 --- a/data_safe_haven/commands/sre.py +++ b/data_safe_haven/commands/sre.py @@ -148,5 +148,7 @@ def teardown( # Upload Pulumi config to blob storage pulumi_config.upload(context) except DataSafeHavenError as exc: - msg = f"Could not teardown Secure Research Environment '{sre_config.safe_name}'." + msg = ( + f"Could not teardown Secure Research Environment '{sre_config.safe_name}'." + ) raise DataSafeHavenError(msg) from exc From 49fe33c0b390da680bbb0317bfb55085207e81aa Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 25 Jun 2024 11:17:23 +0100 Subject: [PATCH 09/10] :art: Remove some additional coloured exception messages --- data_safe_haven/external/api/azure_api.py | 2 +- data_safe_haven/external/api/graph_api.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/external/api/azure_api.py b/data_safe_haven/external/api/azure_api.py index 09ec85a5bf..fe926d8ee8 100644 --- a/data_safe_haven/external/api/azure_api.py +++ b/data_safe_haven/external/api/azure_api.py @@ -450,7 +450,7 @@ def ensure_storage_blob_container( ) return container except HttpResponseError as exc: - msg = f"Failed to create storage container [green]{container_name}." + msg = f"Failed to create storage container '{container_name}'." raise DataSafeHavenAzureError(msg) from exc def get_keyvault_certificate( diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 555b70fb68..0e643f2a64 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -379,7 +379,7 @@ def ensure_application_service_principal( raise DataSafeHavenMicrosoftGraphError(msg) return application_sp except Exception as exc: - msg = f"Could not create service principal for application '[green]{application_name}[/]'." + msg = f"Could not create service principal for application '{application_name}'." raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_token_administrator(self) -> str: From 7e0be6b12b91530d1c97b983d6d6dd0f02d4128a Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Tue, 25 Jun 2024 11:32:40 +0100 Subject: [PATCH 10/10] Correct test exceptions --- tests/context/test_context_settings.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/context/test_context_settings.py b/tests/context/test_context_settings.py index 3b14731aa3..eba8df4cc6 100644 --- a/tests/context/test_context_settings.py +++ b/tests/context/test_context_settings.py @@ -150,14 +150,7 @@ def test_null_selected(self, context_yaml): def test_missing_selected(self, context_yaml): context_yaml = "\n".join(context_yaml.splitlines()[1:]) - msg = "\n".join( - [ - "Could not load ContextSettings configuration.", - "1 validation error for ContextSettings", - "selected", - " Field required", - ] - ) + msg = "Could not load ContextSettings configuration." with pytest.raises(DataSafeHavenParameterError, match=msg): ContextSettings.from_yaml(context_yaml) @@ -168,7 +161,7 @@ def test_invalid_selected_input(self, context_yaml): with pytest.raises( DataSafeHavenParameterError, - match="Selected context 'invalid' is not defined.", + match="Could not load ContextSettings configuration.", ): ContextSettings.from_yaml(context_yaml)