From 148587aa9be55248d5d56e55e4eb44ada912cbc8 Mon Sep 17 00:00:00 2001
From: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
Date: Thu, 7 Mar 2024 15:04:59 -0600
Subject: [PATCH] fix(job_attachments): improvements to nonvalid error messages

Signed-off-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
---
 .../job_attachments/_aws/aws_clients.py       |  2 +-
 src/deadline/job_attachments/upload.py        |  6 ++--
 .../deadline_job_attachments/test_upload.py   | 35 +++++++++++++------
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/deadline/job_attachments/_aws/aws_clients.py b/src/deadline/job_attachments/_aws/aws_clients.py
index 7c230a54..fd3f6629 100644
--- a/src/deadline/job_attachments/_aws/aws_clients.py
+++ b/src/deadline/job_attachments/_aws/aws_clients.py
@@ -94,7 +94,7 @@ def get_s3_max_pool_connections() -> int:
         ) from ve
     if s3_max_pool_connections <= 0:
         raise AssetSyncError(
-            f"s3_max_pool_connections ({s3_max_pool_connections}) must be positive integer."
+            f"Nonvalid value for configuration setting: 's3_max_pool_connections' ({s3_max_pool_connections}) must be positive integer."
         )
     return s3_max_pool_connections
 
diff --git a/src/deadline/job_attachments/upload.py b/src/deadline/job_attachments/upload.py
index eae9429a..d07d57ee 100644
--- a/src/deadline/job_attachments/upload.py
+++ b/src/deadline/job_attachments/upload.py
@@ -123,13 +123,13 @@ def __init__(
         # Confirm that the settings values are all positive.
         error_msg = ""
         if small_file_threshold_multiplier <= 0:
-            error_msg = f"small_file_threshold_multiplier ({small_file_threshold_multiplier}) must be positive integer."
+            error_msg = f"'small_file_threshold_multiplier' ({small_file_threshold_multiplier}) must be positive integer."
         elif s3_max_pool_connections <= 0:
             error_msg = (
-                f"s3_max_pool_connections ({s3_max_pool_connections}) must be positive integer."
+                f"'s3_max_pool_connections' ({s3_max_pool_connections}) must be positive integer."
             )
         if error_msg:
-            raise AssetSyncError("Invalid value for configuration setting: " + error_msg)
+            raise AssetSyncError("Nonvalid value for configuration setting: " + error_msg)
 
     def upload_assets(
         self,
diff --git a/test/unit/deadline_job_attachments/test_upload.py b/test/unit/deadline_job_attachments/test_upload.py
index c300cc76..7c62d89b 100644
--- a/test/unit/deadline_job_attachments/test_upload.py
+++ b/test/unit/deadline_job_attachments/test_upload.py
@@ -1278,37 +1278,50 @@ def test_asset_uploader_constructor_with_non_integer_config_settings(
         assert "Failed to parse configuration settings." in str(err.value)
 
     @pytest.mark.parametrize(
-        "setting_name, invalid_value",
+        "setting_name, nonvalid_value, expected_error_msg",
         [
             pytest.param(
                 "s3_max_pool_connections",
                 "-100",
-                id="Invalid s3_max_pool_connections value: negative",
+                "'s3_max_pool_connections' (-100) must be positive integer.",
+                id="s3_max_pool_connections value is negative.",
             ),
             pytest.param(
                 "s3_max_pool_connections",
                 "0",
-                id="Invalid s3_max_pool_connections value: 0",
+                "'s3_max_pool_connections' (0) must be positive integer.",
+                id="s3_max_pool_connections value is 0.",
+            ),
+            pytest.param(
+                "s3_max_pool_connections",
+                "some string",
+                "Failed to parse configuration settings. Please ensure that the following settings in the config file are integers",
+                id="s3_max_pool_connections value is not a number.",
             ),
             pytest.param(
                 "small_file_threshold_multiplier",
                 "-12",
-                id="Invalid small_file_threshold_multiplier value: negative",
+                "'small_file_threshold_multiplier' (-12) must be positive integer.",
+                id="small_file_threshold_multiplier value is negative.",
+            ),
+            pytest.param(
+                "small_file_threshold_multiplier",
+                "some string",
+                "Failed to parse configuration settings. Please ensure that the following settings in the config file are integers",
+                id="small_file_threshold_multiplier value is not a number.",
             ),
         ],
     )
-    def test_asset_uploader_constructor_with_invalid_config_settings(
-        self, setting_name, invalid_value, fresh_deadline_config
+    def test_asset_uploader_constructor_with_nonvalid_config_settings(
+        self, setting_name, nonvalid_value, expected_error_msg, fresh_deadline_config
     ):
         """
-        Tests that when the asset uploader is created with invalid config settings, an AssetSyncError is raised.
+        Tests that when the asset uploader is created with nonvalid config settings, an AssetSyncError is raised.
         """
-        config.set_setting(f"settings.{setting_name}", invalid_value)
+        config.set_setting(f"settings.{setting_name}", nonvalid_value)
         with pytest.raises(AssetSyncError) as err:
             _ = S3AssetUploader()
-        assert (
-            f"Invalid value for configuration setting: {setting_name} ({invalid_value}) must be positive integer."
-        ) in str(err.value)
+        assert expected_error_msg in str(err.value)
 
     @mock_sts
     def test_file_already_uploaded_bucket_in_different_account(self):