fix: log message when sdk defaults not applied (aws#4104)

* fix: log message when sdk defaults not applied

* change: add debug level message and additional unit test for sdk default

* chore: black-format

* chore: pylint

* fix: typos

Author: benieric

src/sagemaker/config/config.py

@@ -110,7 +110,7 @@ def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource
         else:
             try:
                 config_from_file = _load_config_from_file(file_path)
-            except ValueError:
+            except ValueError as error:
                 if file_path not in (
                     _DEFAULT_ADMIN_CONFIG_FILE_PATH,
                     _DEFAULT_USER_CONFIG_FILE_PATH,
@@ -119,12 +119,15 @@ def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource
                     # If there are no files in the Default config file locations, don't throw
                     # Exceptions.
                     raise
+
+                logger.debug(error)
         if config_from_file:
             validate_sagemaker_config(config_from_file)
             merge_dicts(merged_config, config_from_file)
             logger.info("Fetched defaults config from location: %s", file_path)
         else:
-            logger.debug("Fetched defaults config from location: %s, but it was empty", file_path)
+            logger.info("Not applying SDK defaults from location: %s", file_path)
+
     return merged_config
 
 

tests/unit/sagemaker/config/test_config.py

@@ -15,9 +15,15 @@
 import os
 import pytest
 import yaml
+import logging
 from mock import Mock, MagicMock
 
-from sagemaker.config.config import load_sagemaker_config
+from sagemaker.config.config import (
+    load_sagemaker_config,
+    logger,
+    _DEFAULT_ADMIN_CONFIG_FILE_PATH,
+    _DEFAULT_USER_CONFIG_FILE_PATH,
+)
 from jsonschema import exceptions
 from yaml.constructor import ConstructorError
 
@@ -236,3 +242,163 @@ def test_merge_of_s3_default_config_file_and_regular_config_file(
         s3_resource=s3_resource_mock,
     )
     del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
+
+
+def test_logging_when_overridden_admin_is_found_and_overridden_user_config_is_found(
+    get_data_dir, caplog
+):
+    # Should log info message stating defaults were fetched since both exist
+    logger.propagate = True
+
+    os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = get_data_dir
+    os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = get_data_dir
+    load_sagemaker_config()
+    assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        not in caplog.text
+    )
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        not in caplog.text
+    )
+    del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
+    del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
+    logger.propagate = False
+
+
+def test_logging_when_overridden_admin_is_found_and_default_user_config_not_found(
+    get_data_dir, caplog
+):
+    logger.propagate = True
+    caplog.set_level(logging.DEBUG, logger=logger.name)
+    os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = get_data_dir
+    load_sagemaker_config()
+    assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Unable to load the config file from the location: {}".format(
+            _DEFAULT_USER_CONFIG_FILE_PATH
+        )
+        in caplog.text
+    )
+    del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
+    logger.propagate = False
+
+
+def test_logging_when_default_admin_not_found_and_overriden_user_config_is_found(
+    get_data_dir, caplog
+):
+    logger.propagate = True
+    caplog.set_level(logging.DEBUG, logger=logger.name)
+    os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = get_data_dir
+    load_sagemaker_config()
+    assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Unable to load the config file from the location: {}".format(
+            _DEFAULT_ADMIN_CONFIG_FILE_PATH
+        )
+        in caplog.text
+    )
+    del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
+    logger.propagate = False
+
+
+def test_logging_when_default_admin_not_found_and_default_user_config_not_found(caplog):
+    # Should log info message stating sdk defaults were not applied
+    # for admin and user config since both are missing from default location
+    logger.propagate = True
+    caplog.set_level(logging.DEBUG, logger=logger.name)
+    load_sagemaker_config()
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Unable to load the config file from the location: {}".format(
+            _DEFAULT_ADMIN_CONFIG_FILE_PATH
+        )
+        in caplog.text
+    )
+    assert (
+        "Unable to load the config file from the location: {}".format(
+            _DEFAULT_USER_CONFIG_FILE_PATH
+        )
+        in caplog.text
+    )
+    logger.propagate = False
+
+
+def test_logging_when_default_admin_not_found_and_overriden_user_config_not_found(
+    get_data_dir, caplog
+):
+    # Should only log info message stating sdk defaults were not applied from default admin config.
+    # Failing to load overriden user config should throw exception.
+    logger.propagate = True
+    fake_config_file_path = os.path.join(get_data_dir, "config-not-found.yaml")
+    os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = fake_config_file_path
+    with pytest.raises(ValueError):
+        load_sagemaker_config()
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        not in caplog.text
+    )
+    del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
+    logger.propagate = False
+
+
+def test_logging_when_overriden_admin_not_found_and_overridden_user_config_not_found(
+    get_data_dir, caplog
+):
+    # Should not log any info messages since both config paths are overridden.
+    # Should throw an exception on failure since both will fail to load.
+    logger.propagate = True
+    fake_config_file_path = os.path.join(get_data_dir, "config-not-found.yaml")
+    os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = fake_config_file_path
+    os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = fake_config_file_path
+    with pytest.raises(ValueError):
+        load_sagemaker_config()
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        not in caplog.text
+    )
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        not in caplog.text
+    )
+    del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
+    del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
+    logger.propagate = False
+
+
+def test_logging_with_additional_configs_and_none_are_found(caplog):
+    # Should log info message stating sdk defaults were not applied
+    # for admin and user config since both are missing from default location.
+    # Should throw exception when config in additional_config_path is missing
+    logger.propagate = True
+    with pytest.raises(ValueError):
+        load_sagemaker_config(additional_config_paths=["fake-path"])
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    assert (
+        "Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
+        in caplog.text
+    )
+    logger.propagate = False