diff --git a/src/sagemaker/config/config.py b/src/sagemaker/config/config.py index ce994f2520..3beb9dfe21 100644 --- a/src/sagemaker/config/config.py +++ b/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 diff --git a/tests/unit/sagemaker/config/test_config.py b/tests/unit/sagemaker/config/test_config.py index e07606000c..54d236eaab 100644 --- a/tests/unit/sagemaker/config/test_config.py +++ b/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