Remove retry circular dependency

common/logs.py

@@ -13,9 +13,11 @@
 # limitations under the License.
 """Set up for logging."""
 from enum import Enum
+
 import logging
 import os
 import sys
+import time
 import traceback
 
 import google.cloud.logging
@@ -25,8 +27,8 @@
 # Disable this check since we have a bunch of non-constant globals in this file.
 # pylint: disable=invalid-name
 
-from common import retry
 from common import utils
+from common import retry
 
 _default_logger = None
 _log_client = None
@@ -37,6 +39,7 @@
 
 NUM_RETRIES = 5
 RETRY_DELAY = 1
+BACKOFF = 2
 
 
 def _initialize_cloud_clients():
@@ -153,47 +156,61 @@ class LogSeverity(Enum):
     DEBUG = logging.DEBUG
 
 
-@retry.wrap(NUM_RETRIES, RETRY_DELAY, 'common.logs.log', log_retries=False)
 def log(logger, severity, message, *args, extras=None):
     """Log a message with severity |severity|. If using stack driver logging
     then |extras| is also logged (in addition to default extras)."""
-    message = str(message)
-    if args:
-        message = message % args
-
-    if utils.is_local():
-        if extras:
-            message += ' Extras: ' + str(extras)
-        logging.log(severity, message)
-        return
-
-    if logger is None:
-        logger = _default_logger
-    assert logger
-
-    struct_message = {
-        'message': message,
-    }
-    all_extras = _default_extras.copy()
-    extras = extras or {}
-    all_extras.update(extras)
-    struct_message.update(all_extras)
-    severity = LogSeverity(severity).name
-    logger.log_struct(struct_message, severity=severity)
+    # Custom retry logic to avoid circular dependency
+    # as retry from retry.py uses log
+    for num_try in range(1, NUM_RETRIES + 1):
+        try:
+            message = str(message)
+            if args:
+                message = message % args
+
+            if utils.is_local():
+                if extras:
+                    message += ' Extras: ' + str(extras)
+                logging.log(severity, message)
+                return
+
+            if logger is None:
+                logger = _default_logger
+            assert logger
+
+            struct_message = {
+                'message': message,
+            }
+            all_extras = _default_extras.copy()
+            extras = extras or {}
+            all_extras.update(extras)
+            struct_message.update(all_extras)
+            severity = LogSeverity(severity).name
+            logger.log_struct(struct_message, severity=severity)
+            break
+        except Exception:  # pylint: disable=broad-except
+            # We really dont want do to do anything here except sleep here,
+            # since we cant log it out as log itself is already failing
+            time.sleep(retry.get_delay(num_try, RETRY_DELAY, BACKOFF))
 
 
 def error(message, *args, extras=None, logger=None):
     """Logs |message| to stackdriver logging and error reporting (including
     exception if there was one."""
 
-    @retry.wrap(NUM_RETRIES,
-                RETRY_DELAY,
-                'common.logs.error._report_error_with_retries',
-                log_retries=False)
     def _report_error_with_retries(message):
         if utils.is_local():
             return
-        _error_reporting_client.report(message)
+
+        # Custom retry logic to avoid circular dependency
+        # as retry from retry.py uses log
+        for num_try in range(1, NUM_RETRIES + 1):
+            try:
+                _error_reporting_client.report(message)
+                break
+            except Exception:  # pylint: disable=broad-except
+                # We really dont want do to do anything here except sleep here,
+                # since we cant log it out as log itself is already failing
+                time.sleep(retry.get_delay(num_try, RETRY_DELAY, BACKOFF))
 
     if not any(sys.exc_info()):
         _report_error_with_retries(message % args)

common/retry.py

@@ -20,6 +20,8 @@
 import sys
 import time
 
+from common import logs
+
 
 def sleep(seconds):
     """Invoke time.sleep."""
@@ -37,11 +39,8 @@ def wrap(  # pylint: disable=too-many-arguments
         function,
         backoff=2,
         exception_type=Exception,
-        log_retries=True,
         retry_on_false=False):
     """Retry decorator for a function."""
-    # To avoid circular dependency.
-    from common import logs  # pylint:disable=import-outside-toplevel
 
     assert delay > 0
     assert backoff >= 1
@@ -60,10 +59,9 @@ def handle_retry(num_try, exception=None):
 
             if (exception is None or
                     isinstance(exception, exception_type)) and num_try < tries:
-                if log_retries:
-                    logs.info('Retrying on %s failed with %s. Retrying again.',
-                              function_with_type,
-                              sys.exc_info()[1])
+                logs.info('Retrying on %s failed with %s. Retrying again.',
+                          function_with_type,
+                          sys.exc_info()[1])
                 sleep(get_delay(num_try, delay, backoff))
                 return True