From 7e958f1eaef0e06c682c5d318dee166a11abce1f Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Tue, 24 Apr 2018 15:59:22 -0700 Subject: [PATCH 1/2] Avoid mutating the root logger Using `logging.basicConfig(level=logging.INFO)` mutates the root logger for the current Python interpreter. This means that any library which imports the pydocumentdb library will have its log level set to INFO which leads to unexpected behavior. Some libraries even have tests to defend against this behavior, e.g. Celery: https://github.com/celery/celery/blob/120770929f/t/unit/conftest.py#L231-L244 To fix this unexpected behavior, this commit introduces a new logger that's specific to the endpoint discovery retry policy and only mutates the log level of that specific logger. --- pydocumentdb/endpoint_discovery_retry_policy.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pydocumentdb/endpoint_discovery_retry_policy.py b/pydocumentdb/endpoint_discovery_retry_policy.py index ab1028f..afde9a5 100644 --- a/pydocumentdb/endpoint_discovery_retry_policy.py +++ b/pydocumentdb/endpoint_discovery_retry_policy.py @@ -24,6 +24,9 @@ import logging +logger = logging.getLogger(__name__) + + class _EndpointDiscoveryRetryPolicy(object): """The endpoint discovery retry policy class used for geo-replicated database accounts to handle the write forbidden exceptions due to writable/readable location changes @@ -40,7 +43,7 @@ def __init__(self, global_endpoint_manager): self._max_retry_attempt_count = _EndpointDiscoveryRetryPolicy.Max_retry_attempt_count self.current_retry_attempt_count = 0 self.retry_after_in_milliseconds = _EndpointDiscoveryRetryPolicy.Retry_after_in_milliseconds - logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.INFO) + logger.setLevel(logging.INFO) def ShouldRetry(self, exception): """Returns true if should retry based on the passed-in exception. @@ -53,7 +56,7 @@ def ShouldRetry(self, exception): """ if self.current_retry_attempt_count < self._max_retry_attempt_count and self.global_endpoint_manager.EnableEndpointDiscovery: self.current_retry_attempt_count += 1 - logging.info('Write location was changed, refreshing the locations list from database account and will retry the request.') + logger.info('Write location was changed, refreshing the locations list from database account and will retry the request.') # Refresh the endpoint list to refresh the new writable and readable locations self.global_endpoint_manager.RefreshEndpointList() From bb74225a8983061627889b352f8b789462898e10 Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Thu, 26 Apr 2018 08:58:44 -0700 Subject: [PATCH 2/2] Emulate full basicConfig logging setup --- pydocumentdb/endpoint_discovery_retry_policy.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pydocumentdb/endpoint_discovery_retry_policy.py b/pydocumentdb/endpoint_discovery_retry_policy.py index afde9a5..d825015 100644 --- a/pydocumentdb/endpoint_discovery_retry_policy.py +++ b/pydocumentdb/endpoint_discovery_retry_policy.py @@ -25,6 +25,11 @@ import logging logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) +log_formatter = logging.Formatter('%(levelname)s:%(message)s') +log_handler = logging.StreamHandler() +log_handler.setFormatter(log_formatter) +logger.addHandler(log_handler) class _EndpointDiscoveryRetryPolicy(object): @@ -43,7 +48,6 @@ def __init__(self, global_endpoint_manager): self._max_retry_attempt_count = _EndpointDiscoveryRetryPolicy.Max_retry_attempt_count self.current_retry_attempt_count = 0 self.retry_after_in_milliseconds = _EndpointDiscoveryRetryPolicy.Retry_after_in_milliseconds - logger.setLevel(logging.INFO) def ShouldRetry(self, exception): """Returns true if should retry based on the passed-in exception.