From 450d572ccdbd3b2bb8be8c41ab0954792a825195 Mon Sep 17 00:00:00 2001 From: ys Date: Sun, 3 May 2020 13:32:17 -0400 Subject: [PATCH] Changed time.sleep to condition.wait() time.sleep(n) is a kernel call that pauses the entire Python interpreter, Python threads a virtual. Using condition.wait(n) lets the other threads execute while waiting, this bypasses the GIL (global interpreter lock). Added workflow to enable tests after commit. --- .github/workflows/pythonapp.yml | 37 ++++++++++++++++++ retry/api.py | 28 ++++++++++---- tests/test_retry.py | 68 ++++++++++++++++++--------------- 3 files changed, 95 insertions(+), 38 deletions(-) create mode 100644 .github/workflows/pythonapp.yml diff --git a/.github/workflows/pythonapp.yml b/.github/workflows/pythonapp.yml new file mode 100644 index 0000000..238bab6 --- /dev/null +++ b/.github/workflows/pythonapp.yml @@ -0,0 +1,37 @@ +# This workflow will install Python dependencies, run tests and lint with a single version of Python +# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions + +name: Python application + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Set up Python 3.8 + uses: actions/setup-python@v1 + with: + python-version: 3.8 + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install flake8 pytest + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide + flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Test with pytest + run: | + pip install git+https://github.com/brookman1/retry.git + pytest diff --git a/retry/api.py b/retry/api.py index 4a404b9..b892e75 100644 --- a/retry/api.py +++ b/retry/api.py @@ -1,6 +1,6 @@ import logging import random -import time +import threading from functools import partial @@ -11,7 +11,7 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, - logger=logging_logger): + logger=logging_logger, condition=threading.Condition()): """ Executes a function and retries it if it failed. @@ -25,6 +25,9 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param condition: a threading.Condition that has aquire / release and wait(n) methods. + Used instead of time.sleep to bypass global interpreter lock (GIL). + :returns: the result of the f function. """ _tries, _delay = tries, delay @@ -38,8 +41,11 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, if logger is not None: logger.warning('%s, retrying in %s seconds...', e, _delay) - - time.sleep(_delay) + + # the three lines below, sleep _delay seconds. + condition.acquire() + condition.wait(_delay) + condition.release() _delay *= backoff if isinstance(jitter, tuple): @@ -51,7 +57,8 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, _delay = min(_delay, max_delay) -def retry(exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, logger=logging_logger): +def retry(exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, logger=logging_logger, + condition=threading.Condition()): """Returns a retry decorator. :param exceptions: an exception or a tuple of exceptions to catch. default: Exception. @@ -63,6 +70,8 @@ def retry(exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, ji fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param condition: a threading.Condition that has aquire / release and wait(n) methods. + Used instead of time.sleep to bypass global interpreter lock (GIL). :returns: a retry decorator. """ @@ -71,14 +80,14 @@ def retry_decorator(f, *fargs, **fkwargs): args = fargs if fargs else list() kwargs = fkwargs if fkwargs else dict() return __retry_internal(partial(f, *args, **kwargs), exceptions, tries, delay, max_delay, backoff, jitter, - logger) + logger, condition=condition) return retry_decorator def retry_call(f, fargs=None, fkwargs=None, exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, - logger=logging_logger): + logger=logging_logger, condition=threading.Condition()): """ Calls a function and re-executes it if it failed. @@ -94,8 +103,11 @@ def retry_call(f, fargs=None, fkwargs=None, exceptions=Exception, tries=-1, dela fixed if a number, random if a range tuple (min, max) :param logger: logger.warning(fmt, error, delay) will be called on failed attempts. default: retry.logging_logger. if None, logging is disabled. + :param condition: a threading.Condition that has aquire / release and wait(n) methods. + Used instead of time.sleep to bypass global interpreter lock (GIL). :returns: the result of the f function. """ args = fargs if fargs else list() kwargs = fkwargs if fkwargs else dict() - return __retry_internal(partial(f, *args, **kwargs), exceptions, tries, delay, max_delay, backoff, jitter, logger) + return __retry_internal(partial(f, *args, **kwargs), exceptions, tries, delay, max_delay, backoff, jitter, logger, + condition=condition) diff --git a/tests/test_retry.py b/tests/test_retry.py index 64f45cd..60e73f6 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -1,40 +1,52 @@ try: from unittest.mock import create_autospec -except ImportError: - from mock import create_autospec - -try: from unittest.mock import MagicMock + from unittest import mock except ImportError: + from mock import create_autospec from mock import MagicMock + import mock -import time +import threading import pytest -from retry.api import retry_call -from retry.api import retry +import retry.api +import logging -def test_retry(monkeypatch): - mock_sleep_time = [0] +logging_logger = logging.Logger(__name__) - def mock_sleep(seconds): - mock_sleep_time[0] += seconds +mock_sleep_time = [0] - monkeypatch.setattr(time, 'sleep', mock_sleep) +class mockCondition(): + def acquire(self): + logging_logger.warning('in acquire') + return True + + def release(self): + logging_logger.warning('in release') + + def wait(self, seconds): + logging_logger.warning('in wait') + mock_sleep_time[0] += seconds + +def test_retry(monkeypatch): + global mock_sleep_time + mock_sleep_time = [0] + + monkeypatch.setattr(retry.api.threading, 'Condition', mockCondition) hit = [0] tries = 5 delay = 1 backoff = 2 - @retry(tries=tries, delay=delay, backoff=backoff) + @retry.api.retry(tries=tries, delay=delay, backoff=backoff, condition=mockCondition()) def f(): hit[0] += 1 1 / 0 - with pytest.raises(ZeroDivisionError): f() assert hit[0] == tries @@ -46,7 +58,7 @@ def test_tries_inf(): hit = [0] target = 10 - @retry(tries=float('inf')) + @retry.api.retry(tries=float('inf'), condition=mockCondition()) def f(): hit[0] += 1 if hit[0] == target: @@ -60,7 +72,7 @@ def test_tries_minus1(): hit = [0] target = 10 - @retry(tries=-1) + @retry.api.retry(tries=-1) def f(): hit[0] += 1 if hit[0] == target: @@ -71,12 +83,10 @@ def f(): def test_max_delay(monkeypatch): + global mock_sleep_time mock_sleep_time = [0] - def mock_sleep(seconds): - mock_sleep_time[0] += seconds - - monkeypatch.setattr(time, 'sleep', mock_sleep) + monkeypatch.setattr(retry.api.threading, 'Condition', mockCondition) hit = [0] @@ -85,7 +95,7 @@ def mock_sleep(seconds): backoff = 2 max_delay = delay # Never increase delay - @retry(tries=tries, delay=delay, max_delay=max_delay, backoff=backoff) + @retry.api.retry(tries=tries, delay=delay, max_delay=max_delay, backoff=backoff, condition=mockCondition()) def f(): hit[0] += 1 1 / 0 @@ -97,19 +107,17 @@ def f(): def test_fixed_jitter(monkeypatch): + global mock_sleep_time mock_sleep_time = [0] - def mock_sleep(seconds): - mock_sleep_time[0] += seconds - - monkeypatch.setattr(time, 'sleep', mock_sleep) + monkeypatch.setattr(retry.api.threading, 'Condition', mockCondition) hit = [0] tries = 10 jitter = 1 - @retry(tries=tries, jitter=jitter) + @retry.api.retry(tries=tries, jitter=jitter, condition=mockCondition()) def f(): hit[0] += 1 1 / 0 @@ -124,7 +132,7 @@ def test_retry_call(): f_mock = MagicMock(side_effect=RuntimeError) tries = 2 try: - retry_call(f_mock, exceptions=RuntimeError, tries=tries) + retry.api.retry_call(f_mock, exceptions=RuntimeError, tries=tries, condition=mockCondition()) except RuntimeError: pass @@ -137,7 +145,7 @@ def test_retry_call_2(): tries = 5 result = None try: - result = retry_call(f_mock, exceptions=RuntimeError, tries=tries) + result = retry.api.retry_call(f_mock, exceptions=RuntimeError, tries=tries, condition=mockCondition()) except RuntimeError: pass @@ -157,7 +165,7 @@ def f(value=0): result = None f_mock = MagicMock(spec=f, return_value=return_value) try: - result = retry_call(f_mock, fargs=[return_value]) + result = retry.api.retry_call(f_mock, fargs=[return_value], condition=mockCondition()) except RuntimeError: pass @@ -177,7 +185,7 @@ def f(value=0): result = None f_mock = MagicMock(spec=f, return_value=kwargs['value']) try: - result = retry_call(f_mock, fkwargs=kwargs) + result = retry.api.retry_call(f_mock, fkwargs=kwargs, condition=mockCondition()) except RuntimeError: pass