Skip to content

Commit a7f0ec8

Browse files
committed
fix: update logging configs to ignore debug flag when given given a user provided logger
1 parent 2cca70f commit a7f0ec8

File tree

4 files changed

+67
-91
lines changed

4 files changed

+67
-91
lines changed

src/amplitude_experiment/local/config.py

Lines changed: 24 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,37 +2,30 @@
22
import sys
33

44
from ..assignment import AssignmentConfig
5-
from ..cohort.cohort_sync_config import (
6-
CohortSyncConfig,
7-
DEFAULT_COHORT_SYNC_URL,
8-
EU_COHORT_SYNC_URL,
9-
)
5+
from ..cohort.cohort_sync_config import CohortSyncConfig, DEFAULT_COHORT_SYNC_URL, EU_COHORT_SYNC_URL
106
from ..server_zone import ServerZone
117

12-
DEFAULT_SERVER_URL = "https://api.lab.amplitude.com"
13-
EU_SERVER_URL = "https://flag.lab.eu.amplitude.com"
8+
DEFAULT_SERVER_URL = 'https://api.lab.amplitude.com'
9+
EU_SERVER_URL = 'https://flag.lab.eu.amplitude.com'
1410

15-
DEFAULT_STREAM_URL = "https://stream.lab.amplitude.com"
16-
EU_STREAM_SERVER_URL = "https://stream.lab.eu.amplitude.com"
11+
DEFAULT_STREAM_URL = 'https://stream.lab.amplitude.com'
12+
EU_STREAM_SERVER_URL = 'https://stream.lab.eu.amplitude.com'
1713

1814

1915
class LocalEvaluationConfig:
2016
"""Experiment Local Client Configuration"""
2117

22-
def __init__(
23-
self,
24-
debug: bool = False,
25-
server_url: str = DEFAULT_SERVER_URL,
26-
server_zone: ServerZone = ServerZone.US,
27-
flag_config_polling_interval_millis: int = 30000,
28-
flag_config_poller_request_timeout_millis: int = 10000,
29-
stream_updates: bool = False,
30-
stream_server_url: str = DEFAULT_STREAM_URL,
31-
stream_flag_conn_timeout: int = 1500,
32-
assignment_config: AssignmentConfig = None,
33-
cohort_sync_config: CohortSyncConfig = None,
34-
logger: logging.Logger = None,
35-
):
18+
def __init__(self, debug: bool = False,
19+
server_url: str = DEFAULT_SERVER_URL,
20+
server_zone: ServerZone = ServerZone.US,
21+
flag_config_polling_interval_millis: int = 30000,
22+
flag_config_poller_request_timeout_millis: int = 10000,
23+
stream_updates: bool = False,
24+
stream_server_url: str = DEFAULT_STREAM_URL,
25+
stream_flag_conn_timeout: int = 1500,
26+
assignment_config: AssignmentConfig = None,
27+
cohort_sync_config: CohortSyncConfig = None,
28+
logger: logging.Logger = None):
3629
"""
3730
Initialize a config
3831
Parameters:
@@ -46,7 +39,7 @@ def __init__(
4639
assignment_config (AssignmentConfig): The assignment configuration.
4740
cohort_sync_config (CohortSyncConfig): The cohort sync configuration.
4841
logger (logging.Logger): Optional logger instance. If provided, this logger will be used instead of
49-
creating a new one. The debug flag will still be applied to set the log level.
42+
creating a new one. The debug flag only applies when no logger is provided.
5043
5144
Returns:
5245
The config object
@@ -57,20 +50,16 @@ def __init__(
5750
self.cohort_sync_config = cohort_sync_config
5851
if server_url == DEFAULT_SERVER_URL and server_zone == ServerZone.EU:
5952
self.server_url = EU_SERVER_URL
60-
if (
61-
cohort_sync_config is not None
62-
and cohort_sync_config.cohort_server_url == DEFAULT_COHORT_SYNC_URL
63-
):
53+
if (cohort_sync_config is not None and
54+
cohort_sync_config.cohort_server_url == DEFAULT_COHORT_SYNC_URL):
6455
self.cohort_sync_config.cohort_server_url = EU_COHORT_SYNC_URL
6556

6657
self.stream_server_url = stream_server_url
6758
if stream_server_url == DEFAULT_SERVER_URL and server_zone == ServerZone.EU:
6859
self.stream_server_url = EU_STREAM_SERVER_URL
6960

7061
self.flag_config_polling_interval_millis = flag_config_polling_interval_millis
71-
self.flag_config_poller_request_timeout_millis = (
72-
flag_config_poller_request_timeout_millis
73-
)
62+
self.flag_config_poller_request_timeout_millis = flag_config_poller_request_timeout_millis
7463
self.stream_updates = stream_updates
7564
self.stream_flag_conn_timeout = stream_flag_conn_timeout
7665
self.assignment_config = assignment_config
@@ -82,10 +71,9 @@ def __init__(
8271
if not self.logger.handlers:
8372
handler = logging.StreamHandler(sys.stderr)
8473
self.logger.addHandler(handler)
74+
# Set log level: DEBUG if debug=True, otherwise WARNING
75+
# Only apply debug flag to default logger, not user-provided loggers
76+
log_level = logging.DEBUG if self.debug else logging.WARNING
77+
self.logger.setLevel(log_level)
8578
else:
8679
self.logger = logger
87-
88-
# Set log level: DEBUG if debug=True, otherwise WARNING
89-
# This applies to both provided loggers and the default logger
90-
log_level = logging.DEBUG if self.debug else logging.WARNING
91-
self.logger.setLevel(log_level)

src/amplitude_experiment/remote/config.py

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,24 @@
11
from ..server_zone import ServerZone
22

3-
DEFAULT_SERVER_URL = "https://api.lab.amplitude.com"
4-
EU_SERVER_URL = "https://api.lab.eu.amplitude.com"
3+
DEFAULT_SERVER_URL = 'https://api.lab.amplitude.com'
4+
EU_SERVER_URL = 'https://api.lab.eu.amplitude.com'
55
import logging
66
import sys
77

88

99
class RemoteEvaluationConfig:
1010
"""Experiment Remote Client Configuration"""
1111

12-
def __init__(
13-
self,
14-
debug=False,
15-
server_url=DEFAULT_SERVER_URL,
16-
fetch_timeout_millis=10000,
17-
fetch_retries=0,
18-
fetch_retry_backoff_min_millis=500,
19-
fetch_retry_backoff_max_millis=10000,
20-
fetch_retry_backoff_scalar=1.5,
21-
fetch_retry_timeout_millis=10000,
22-
server_zone: ServerZone = ServerZone.US,
23-
logger=None,
24-
):
12+
def __init__(self, debug=False,
13+
server_url=DEFAULT_SERVER_URL,
14+
fetch_timeout_millis=10000,
15+
fetch_retries=0,
16+
fetch_retry_backoff_min_millis=500,
17+
fetch_retry_backoff_max_millis=10000,
18+
fetch_retry_backoff_scalar=1.5,
19+
fetch_retry_timeout_millis=10000,
20+
server_zone: ServerZone = ServerZone.US,
21+
logger=None):
2522
"""
2623
Initialize a config
2724
Parameters:
@@ -38,7 +35,7 @@ def __init__(
3835
fetch_retry_timeout_millis (int): The request timeout for retrying fetch requests.
3936
server_zone (str): Select the Amplitude data center to get flags and variants from, US or EU.
4037
logger (logging.Logger): Optional logger instance. If provided, this logger will be used instead of
41-
creating a new one. The debug flag will still be applied to set the log level.
38+
creating a new one. The debug flag only applies when no logger is provided.
4239
4340
Returns:
4441
The config object
@@ -62,10 +59,9 @@ def __init__(
6259
if not self.logger.handlers:
6360
handler = logging.StreamHandler(sys.stderr)
6461
self.logger.addHandler(handler)
62+
# Set log level: DEBUG if debug=True, otherwise WARNING
63+
# Only apply debug flag to default logger, not user-provided loggers
64+
log_level = logging.DEBUG if self.debug else logging.WARNING
65+
self.logger.setLevel(log_level)
6566
else:
6667
self.logger = logger
67-
68-
# Set log level: DEBUG if debug=True, otherwise WARNING
69-
# This applies to both provided loggers and the default logger
70-
log_level = logging.DEBUG if self.debug else logging.WARNING
71-
self.logger.setLevel(log_level)

tests/local/config_test.py

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,21 @@ def test_custom_logger_is_used(self):
4141
self.assertEqual(config.logger, custom_logger)
4242
self.assertEqual(config.logger.name, "CustomLogger")
4343

44-
def test_custom_logger_has_debug_level_when_debug_true(self):
45-
"""Test that custom logger has DEBUG level set when debug=True"""
46-
custom_logger = logging.getLogger("CustomLogger")
47-
config = LocalEvaluationConfig(debug=True, logger=custom_logger)
48-
self.assertEqual(config.logger.level, logging.DEBUG)
49-
50-
def test_custom_logger_has_warning_level_when_debug_false(self):
51-
"""Test that custom logger has WARNING level set when debug=False"""
52-
custom_logger = logging.getLogger("CustomLogger")
53-
config = LocalEvaluationConfig(debug=False, logger=custom_logger)
54-
self.assertEqual(config.logger.level, logging.WARNING)
55-
56-
def test_custom_logger_debug_flag_takes_precedence(self):
57-
"""Test that debug flag takes precedence over logger's existing level"""
58-
custom_logger = logging.getLogger("CustomLogger")
44+
def test_custom_logger_level_not_modified_by_debug_flag(self):
45+
"""Test that custom logger level is not modified by debug flag"""
46+
# Test with debug=True
47+
custom_logger = logging.getLogger("CustomLoggerDebug")
5948
custom_logger.setLevel(logging.ERROR)
6049
config = LocalEvaluationConfig(debug=True, logger=custom_logger)
61-
# Debug flag should override to DEBUG
62-
self.assertEqual(config.logger.level, logging.DEBUG)
50+
# Logger level should remain unchanged (ERROR), not modified to DEBUG
51+
self.assertEqual(config.logger.level, logging.ERROR)
52+
53+
# Test with debug=False
54+
custom_logger2 = logging.getLogger("CustomLoggerWarning")
55+
custom_logger2.setLevel(logging.INFO)
56+
config2 = LocalEvaluationConfig(debug=False, logger=custom_logger2)
57+
# Logger level should remain unchanged (INFO), not modified to WARNING
58+
self.assertEqual(config2.logger.level, logging.INFO)
6359

6460
def test_default_logger_only_one_handler_added(self):
6561
"""Test that only one handler is added to default logger"""

tests/remote/config_test.py

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,21 @@ def test_custom_logger_is_used(self):
4141
self.assertEqual(config.logger, custom_logger)
4242
self.assertEqual(config.logger.name, "CustomLogger")
4343

44-
def test_custom_logger_has_debug_level_when_debug_true(self):
45-
"""Test that custom logger has DEBUG level set when debug=True"""
46-
custom_logger = logging.getLogger("CustomLogger")
47-
config = RemoteEvaluationConfig(debug=True, logger=custom_logger)
48-
self.assertEqual(config.logger.level, logging.DEBUG)
49-
50-
def test_custom_logger_has_warning_level_when_debug_false(self):
51-
"""Test that custom logger has WARNING level set when debug=False"""
52-
custom_logger = logging.getLogger("CustomLogger")
53-
config = RemoteEvaluationConfig(debug=False, logger=custom_logger)
54-
self.assertEqual(config.logger.level, logging.WARNING)
55-
56-
def test_custom_logger_debug_flag_takes_precedence(self):
57-
"""Test that debug flag takes precedence over logger's existing level"""
58-
custom_logger = logging.getLogger("CustomLogger")
44+
def test_custom_logger_level_not_modified_by_debug_flag(self):
45+
"""Test that custom logger level is not modified by debug flag"""
46+
# Test with debug=True
47+
custom_logger = logging.getLogger("CustomLoggerDebug")
5948
custom_logger.setLevel(logging.ERROR)
6049
config = RemoteEvaluationConfig(debug=True, logger=custom_logger)
61-
# Debug flag should override to DEBUG
62-
self.assertEqual(config.logger.level, logging.DEBUG)
50+
# Logger level should remain unchanged (ERROR), not modified to DEBUG
51+
self.assertEqual(config.logger.level, logging.ERROR)
52+
53+
# Test with debug=False
54+
custom_logger2 = logging.getLogger("CustomLoggerWarning")
55+
custom_logger2.setLevel(logging.INFO)
56+
config2 = RemoteEvaluationConfig(debug=False, logger=custom_logger2)
57+
# Logger level should remain unchanged (INFO), not modified to WARNING
58+
self.assertEqual(config2.logger.level, logging.INFO)
6359

6460
def test_default_logger_only_one_handler_added(self):
6561
"""Test that only one handler is added to default logger"""

0 commit comments

Comments
 (0)