Skip to content

Commit 323391b

Browse files
committed
create log method
1 parent ef60387 commit 323391b

File tree

3 files changed

+56
-36
lines changed

3 files changed

+56
-36
lines changed

src/sagemaker/config/config.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@
2828
from botocore.utils import merge_dicts
2929
from six.moves.urllib.parse import urlparse
3030
from sagemaker.config.config_schema import SAGEMAKER_PYTHON_SDK_CONFIG_SCHEMA
31-
from sagemaker.config.config_utils import get_sagemaker_config_logger
31+
from sagemaker.config.config_utils import non_repeating_log, get_sagemaker_config_logger
3232

3333
logger = get_sagemaker_config_logger()
34+
log_info_function = non_repeating_log(logger, "info")
3435

3536
_APP_NAME = "sagemaker"
3637
# The default name of the config file.
@@ -52,7 +53,9 @@
5253
S3_PREFIX = "s3://"
5354

5455

55-
def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource=None) -> dict:
56+
def load_sagemaker_config(
57+
additional_config_paths: List[str] = None, s3_resource=None, repeat_log=False
58+
) -> dict:
5659
"""Loads config files and merges them.
5760
5861
By default, this method first searches for config files in the default locations
@@ -99,6 +102,8 @@ def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource
99102
<https://boto3.amazonaws.com/v1/documentation/api\
100103
/latest/reference/core/session.html#boto3.session.Session.resource>`__.
101104
This argument is not needed if the config files are present in the local file system.
105+
repeat_log (bool): Whether the log with the same contents should be emitted.
106+
Default to ``False``
102107
"""
103108
default_config_path = os.getenv(
104109
ENV_VARIABLE_ADMIN_CONFIG_OVERRIDE, _DEFAULT_ADMIN_CONFIG_FILE_PATH
@@ -109,6 +114,11 @@ def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource
109114
config_paths += additional_config_paths
110115
config_paths = list(filter(lambda item: item is not None, config_paths))
111116
merged_config = {}
117+
118+
log_info = log_info_function
119+
if repeat_log:
120+
log_info = logger.info
121+
112122
for file_path in config_paths:
113123
config_from_file = {}
114124
if file_path.startswith(S3_PREFIX):
@@ -130,9 +140,9 @@ def load_sagemaker_config(additional_config_paths: List[str] = None, s3_resource
130140
if config_from_file:
131141
validate_sagemaker_config(config_from_file)
132142
merge_dicts(merged_config, config_from_file)
133-
logger.info("Fetched defaults config from location: %s", file_path)
143+
log_info("Fetched defaults config from location: %s", file_path)
134144
else:
135-
logger.info("Not applying SDK defaults from location: %s", file_path)
145+
log_info("Not applying SDK defaults from location: %s", file_path)
136146

137147
return merged_config
138148

src/sagemaker/config/config_utils.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import logging
2020
import sys
21-
from types import MethodType
21+
from typing import Callable
2222

2323

2424
def get_sagemaker_config_logger():
@@ -200,23 +200,26 @@ def _log_sagemaker_config_merge(
200200
logger.debug("Skipped value because no value defined\n config key = %s", config_key_path)
201201

202202

203-
def non_repeating_logger(logger: logging.Logger) -> logging.Logger:
204-
"""Patch the info method of input logger to remove repeating message.
203+
def non_repeating_log(logger: logging.Logger, method: str) -> Callable:
204+
"""Create log function that filters the repeated messages.
205205
206206
Args:
207-
logger (logging.Logger): the logger to be patched
207+
logger (logging.Logger): the logger to be used to dispatch the message.
208+
method (str): the log method, can be info, warning or debug.
208209
209210
Returns:
210-
(logging.Logger): the patched logger
211+
(Callable): the new log method
211212
"""
213+
if method not in ["info", "warning", "debug"]:
214+
raise ValueError("Not supported logging method.")
215+
212216
_caches = set()
213-
_old_impl = logger.info
217+
log_method = getattr(logger, method)
214218

215-
def _new_impl(_, msg, *args, **kwargs):
219+
def new_log_method(msg, *args, **kwargs):
216220
key = f"{msg}:{args}"
217221
if key not in _caches:
218-
_old_impl(msg, *args, **kwargs)
222+
log_method(msg, *args, **kwargs)
219223
_caches.add(key)
220224

221-
logger.info = MethodType(_new_impl, logger)
222-
return logger
225+
return new_log_method

tests/unit/sagemaker/config/test_config.py

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ def expected_merged_config(get_data_dir):
4545

4646

4747
def test_config_when_default_config_file_and_user_config_file_is_not_found():
48-
assert load_sagemaker_config() == {}
48+
assert load_sagemaker_config(repeat_log=True) == {}
4949

5050

5151
def test_config_when_overriden_default_config_file_is_not_found(get_data_dir):
5252
fake_config_file_path = os.path.join(get_data_dir, "config-not-found.yaml")
5353
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = fake_config_file_path
5454
with pytest.raises(ValueError):
55-
load_sagemaker_config()
55+
load_sagemaker_config(repeat_log=True)
5656
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
5757

5858

@@ -63,14 +63,14 @@ def test_invalid_config_file_which_has_python_code(get_data_dir):
6363
# PyYAML will throw exceptions for yaml.safe_load. SageMaker Config is using
6464
# yaml.safe_load internally
6565
with pytest.raises(ConstructorError) as exception_info:
66-
load_sagemaker_config(additional_config_paths=[invalid_config_file_path])
66+
load_sagemaker_config(additional_config_paths=[invalid_config_file_path], repeat_log=True)
6767
assert "python/object/apply:eval" in str(exception_info.value)
6868

6969

7070
def test_config_when_additional_config_file_path_is_not_found(get_data_dir):
7171
fake_config_file_path = os.path.join(get_data_dir, "config-not-found.yaml")
7272
with pytest.raises(ValueError):
73-
load_sagemaker_config(additional_config_paths=[fake_config_file_path])
73+
load_sagemaker_config(additional_config_paths=[fake_config_file_path], repeat_log=True)
7474

7575

7676
def test_config_factory_when_override_user_config_file_is_not_found(get_data_dir):
@@ -79,15 +79,15 @@ def test_config_factory_when_override_user_config_file_is_not_found(get_data_dir
7979
)
8080
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = fake_additional_override_config_file_path
8181
with pytest.raises(ValueError):
82-
load_sagemaker_config()
82+
load_sagemaker_config(repeat_log=True)
8383
del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
8484

8585

8686
def test_default_config_file_with_invalid_schema(get_data_dir):
8787
config_file_path = os.path.join(get_data_dir, "invalid_config_file.yaml")
8888
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = config_file_path
8989
with pytest.raises(exceptions.ValidationError):
90-
load_sagemaker_config()
90+
load_sagemaker_config(repeat_log=True)
9191
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
9292

9393

@@ -98,7 +98,7 @@ def test_default_config_file_when_directory_is_provided_as_the_path(
9898
expected_config = base_config_with_schema
9999
expected_config["SageMaker"] = valid_config_with_all_the_scopes
100100
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = get_data_dir
101-
assert expected_config == load_sagemaker_config()
101+
assert expected_config == load_sagemaker_config(repeat_log=True)
102102
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
103103

104104

@@ -108,7 +108,9 @@ def test_additional_config_paths_when_directory_is_provided(
108108
# This will try to load config.yaml file from that directory if present.
109109
expected_config = base_config_with_schema
110110
expected_config["SageMaker"] = valid_config_with_all_the_scopes
111-
assert expected_config == load_sagemaker_config(additional_config_paths=[get_data_dir])
111+
assert expected_config == load_sagemaker_config(
112+
additional_config_paths=[get_data_dir], repeat_log=True
113+
)
112114

113115

114116
def test_default_config_file_when_path_is_provided_as_environment_variable(
@@ -118,7 +120,7 @@ def test_default_config_file_when_path_is_provided_as_environment_variable(
118120
# This will try to load config.yaml file from that directory if present.
119121
expected_config = base_config_with_schema
120122
expected_config["SageMaker"] = valid_config_with_all_the_scopes
121-
assert expected_config == load_sagemaker_config()
123+
assert expected_config == load_sagemaker_config(repeat_log=True)
122124
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
123125

124126

@@ -131,7 +133,9 @@ def test_merge_behavior_when_additional_config_file_path_is_not_found(
131133
)
132134
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = valid_config_file_path
133135
with pytest.raises(ValueError):
134-
load_sagemaker_config(additional_config_paths=[fake_additional_override_config_file_path])
136+
load_sagemaker_config(
137+
additional_config_paths=[fake_additional_override_config_file_path], repeat_log=True
138+
)
135139
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
136140

137141

@@ -142,10 +146,10 @@ def test_merge_behavior(get_data_dir, expected_merged_config):
142146
)
143147
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = valid_config_file_path
144148
assert expected_merged_config == load_sagemaker_config(
145-
additional_config_paths=[additional_override_config_file_path]
149+
additional_config_paths=[additional_override_config_file_path], repeat_log=True
146150
)
147151
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = additional_override_config_file_path
148-
assert expected_merged_config == load_sagemaker_config()
152+
assert expected_merged_config == load_sagemaker_config(repeat_log=True)
149153
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
150154
del os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"]
151155

@@ -169,7 +173,7 @@ def test_s3_config_file(
169173
expected_config = base_config_with_schema
170174
expected_config["SageMaker"] = valid_config_with_all_the_scopes
171175
assert expected_config == load_sagemaker_config(
172-
additional_config_paths=[config_file_s3_uri], s3_resource=s3_resource_mock
176+
additional_config_paths=[config_file_s3_uri], s3_resource=s3_resource_mock, repeat_log=True
173177
)
174178

175179

@@ -183,7 +187,9 @@ def test_config_factory_when_default_s3_config_file_is_not_found(s3_resource_moc
183187
config_file_s3_uri = "s3://{}/{}".format(config_file_bucket, config_file_s3_prefix)
184188
with pytest.raises(ValueError):
185189
load_sagemaker_config(
186-
additional_config_paths=[config_file_s3_uri], s3_resource=s3_resource_mock
190+
additional_config_paths=[config_file_s3_uri],
191+
s3_resource=s3_resource_mock,
192+
repeat_log=True,
187193
)
188194

189195

@@ -213,7 +219,7 @@ def test_s3_config_file_when_uri_provided_corresponds_to_a_path(
213219
expected_config = base_config_with_schema
214220
expected_config["SageMaker"] = valid_config_with_all_the_scopes
215221
assert expected_config == load_sagemaker_config(
216-
additional_config_paths=[config_file_s3_uri], s3_resource=s3_resource_mock
222+
additional_config_paths=[config_file_s3_uri], s3_resource=s3_resource_mock, repeat_log=True
217223
)
218224

219225

@@ -242,6 +248,7 @@ def test_merge_of_s3_default_config_file_and_regular_config_file(
242248
assert expected_merged_config == load_sagemaker_config(
243249
additional_config_paths=[additional_override_config_file_path],
244250
s3_resource=s3_resource_mock,
251+
repeat_log=True,
245252
)
246253
del os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"]
247254

@@ -254,7 +261,7 @@ def test_logging_when_overridden_admin_is_found_and_overridden_user_config_is_fo
254261

255262
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = get_data_dir
256263
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = get_data_dir
257-
load_sagemaker_config()
264+
load_sagemaker_config(repeat_log=True)
258265
assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
259266
assert (
260267
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
@@ -275,7 +282,7 @@ def test_logging_when_overridden_admin_is_found_and_default_user_config_not_foun
275282
logger.propagate = True
276283
caplog.set_level(logging.DEBUG, logger=logger.name)
277284
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = get_data_dir
278-
load_sagemaker_config()
285+
load_sagemaker_config(repeat_log=True)
279286
assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
280287
assert (
281288
"Not applying SDK defaults from location: {}".format(_DEFAULT_USER_CONFIG_FILE_PATH)
@@ -297,7 +304,7 @@ def test_logging_when_default_admin_not_found_and_overriden_user_config_is_found
297304
logger.propagate = True
298305
caplog.set_level(logging.DEBUG, logger=logger.name)
299306
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = get_data_dir
300-
load_sagemaker_config()
307+
load_sagemaker_config(repeat_log=True)
301308
assert "Fetched defaults config from location: {}".format(get_data_dir) in caplog.text
302309
assert (
303310
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
@@ -318,7 +325,7 @@ def test_logging_when_default_admin_not_found_and_default_user_config_not_found(
318325
# for admin and user config since both are missing from default location
319326
logger.propagate = True
320327
caplog.set_level(logging.DEBUG, logger=logger.name)
321-
load_sagemaker_config()
328+
load_sagemaker_config(repeat_log=True)
322329
assert (
323330
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
324331
in caplog.text
@@ -351,7 +358,7 @@ def test_logging_when_default_admin_not_found_and_overriden_user_config_not_foun
351358
fake_config_file_path = os.path.join(get_data_dir, "config-not-found.yaml")
352359
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = fake_config_file_path
353360
with pytest.raises(ValueError):
354-
load_sagemaker_config()
361+
load_sagemaker_config(repeat_log=True)
355362
assert (
356363
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
357364
in caplog.text
@@ -374,7 +381,7 @@ def test_logging_when_overriden_admin_not_found_and_overridden_user_config_not_f
374381
os.environ["SAGEMAKER_USER_CONFIG_OVERRIDE"] = fake_config_file_path
375382
os.environ["SAGEMAKER_ADMIN_CONFIG_OVERRIDE"] = fake_config_file_path
376383
with pytest.raises(ValueError):
377-
load_sagemaker_config()
384+
load_sagemaker_config(repeat_log=True)
378385
assert (
379386
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
380387
not in caplog.text
@@ -394,7 +401,7 @@ def test_logging_with_additional_configs_and_none_are_found(caplog):
394401
# Should throw exception when config in additional_config_path is missing
395402
logger.propagate = True
396403
with pytest.raises(ValueError):
397-
load_sagemaker_config(additional_config_paths=["fake-path"])
404+
load_sagemaker_config(additional_config_paths=["fake-path"], repeat_log=True)
398405
assert (
399406
"Not applying SDK defaults from location: {}".format(_DEFAULT_ADMIN_CONFIG_FILE_PATH)
400407
in caplog.text

0 commit comments

Comments
 (0)