From 2eb5e68044f90c62267a4e8c53704d9ce7e6d59a Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 21:44:31 -0400 Subject: [PATCH 01/22] 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, bypasses the GIL (global interpreter lock). --- retry/api.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/retry/api.py b/retry/api.py index 4a404b9..6335bab 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,7 @@ 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: used for sleeping to avoid the GIL. :returns: the result of the f function. """ _tries, _delay = tries, delay @@ -38,8 +39,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. + condition.acquire() + condition.wait(_delay) + condition.release() _delay *= backoff if isinstance(jitter, tuple): @@ -51,7 +55,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 +68,7 @@ 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. + :paran condition: condition variable used to sleep to avoid the GIL. :returns: a retry decorator. """ From 227e22461514d352a52fde5246bbc66674685e2e Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 21:56:27 -0400 Subject: [PATCH 02/22] fixed syntax, propagated condition, added comments. --- retry/api.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/retry/api.py b/retry/api.py index 6335bab..acdf062 100644 --- a/retry/api.py +++ b/retry/api.py @@ -25,7 +25,8 @@ 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: used for sleeping to avoid the GIL. + :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 @@ -55,7 +56,7 @@ 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. @@ -68,7 +69,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. - :paran condition: condition variable used to sleep to avoid the GIL. + :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. """ @@ -77,14 +79,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. @@ -100,8 +102,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) From d75715d0586bf92f9c9dd7b3e33b236fd7724c1f Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 21:44:31 -0400 Subject: [PATCH 03/22] 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, bypasses the GIL (global interpreter lock). --- retry/api.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/retry/api.py b/retry/api.py index 4a404b9..acdf062 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,8 @@ 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 +40,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. + condition.acquire() + condition.wait(_delay) + condition.release() _delay *= backoff if isinstance(jitter, tuple): @@ -51,7 +56,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 +69,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 +79,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 +102,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) From c234dd19f3f30d395a9f7040400645391dcd353d Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:14:26 -0400 Subject: [PATCH 04/22] New Workflow --- .github/workflows/pythonapp.yml | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) 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..900e442 --- /dev/null +++ b/.github/workflows/pythonapp.yml @@ -0,0 +1,36 @@ +# 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: | + pytest From 62932952c7eea73964238a3c8640092fc4c20741 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:23:10 -0400 Subject: [PATCH 05/22] Changed pip install directions in document. This is to point the installer to this repo. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index e1d0e9c..295a3f5 100644 --- a/README.rst +++ b/README.rst @@ -27,7 +27,7 @@ Installation .. code-block:: bash - $ pip install retry + $ pip install git+https://github.com/brookman1/retry.git API From 3be1b7f21851c5f46221543c115b447c823ba3a3 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:26:10 -0400 Subject: [PATCH 06/22] Fixing tests. --- tests/test_retry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index 64f45cd..d5bcbe2 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -12,8 +12,8 @@ import pytest -from retry.api import retry_call -from retry.api import retry +from ..retry.api import retry_call +from ..retry.api import retry def test_retry(monkeypatch): From d8b98d5f09b129e686d1e861f1bec7eec29c05af Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:33:08 -0400 Subject: [PATCH 07/22] Update pythonapp.yml --- .github/workflows/pythonapp.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pythonapp.yml b/.github/workflows/pythonapp.yml index 900e442..238bab6 100644 --- a/.github/workflows/pythonapp.yml +++ b/.github/workflows/pythonapp.yml @@ -33,4 +33,5 @@ jobs: 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 From 37b5bbfa449eec597bbd09b8db98420d376528d4 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:35:31 -0400 Subject: [PATCH 08/22] revert the fix. --- tests/test_retry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index d5bcbe2..64f45cd 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -12,8 +12,8 @@ import pytest -from ..retry.api import retry_call -from ..retry.api import retry +from retry.api import retry_call +from retry.api import retry def test_retry(monkeypatch): From 928c86a71e1068cc2e9d5cd61d9ae76bb775c150 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:43:36 -0400 Subject: [PATCH 09/22] fixed syntax --- retry/api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/retry/api.py b/retry/api.py index acdf062..da2f8e3 100644 --- a/retry/api.py +++ b/retry/api.py @@ -41,7 +41,7 @@ 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) - # the three lines below, sleep. + # the three lines below, sleep _delay seconds. condition.acquire() condition.wait(_delay) condition.release() @@ -57,7 +57,7 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, def retry(exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, logger=logging_logger, - condition=threading.Condition): + condition=threading.Condition()): """Returns a retry decorator. :param exceptions: an exception or a tuple of exceptions to catch. default: Exception. From 1df148e66c2421b93426b58e9c92d51d36cf1e77 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:58:45 -0400 Subject: [PATCH 10/22] testing threading.Condition instead of time.sleep --- tests/test_retry.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index 64f45cd..1b524d7 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -8,21 +8,28 @@ except ImportError: from mock import MagicMock -import time +import threading import pytest from retry.api import retry_call from retry.api import retry +def mock_condition(): + class mockCondition(): + def acquire(self): + pass + def release(self): + pass + def wait(self, seconds): + mock_sleep_time[0] += seconds + return mockCondition() + def test_retry(monkeypatch): mock_sleep_time = [0] - def mock_sleep(seconds): - mock_sleep_time[0] += seconds - - monkeypatch.setattr(time, 'sleep', mock_sleep) + monkeypatch.setattr(threading, 'Condition', mock_condition) hit = [0] @@ -73,17 +80,14 @@ def f(): def test_max_delay(monkeypatch): mock_sleep_time = [0] - def mock_sleep(seconds): - mock_sleep_time[0] += seconds - - monkeypatch.setattr(time, 'sleep', mock_sleep) - hit = [0] tries = 5 delay = 1 backoff = 2 max_delay = delay # Never increase delay + + monkeypatch.setattr(threading, 'Condition', mock_condition) @retry(tries=tries, delay=delay, max_delay=max_delay, backoff=backoff) def f(): From 8aa356616bf96aec0c6e08496baf6aa979b44aef Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 23:09:13 -0400 Subject: [PATCH 11/22] updatin how mock_sleep_time scope works. --- tests/test_retry.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_retry.py b/tests/test_retry.py index 1b524d7..03e32d3 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -15,6 +15,7 @@ from retry.api import retry_call from retry.api import retry +mock_sleep_time = [] def mock_condition(): class mockCondition(): def acquire(self): @@ -27,6 +28,7 @@ def wait(self, seconds): def test_retry(monkeypatch): + global mock_sleep_time mock_sleep_time = [0] monkeypatch.setattr(threading, 'Condition', mock_condition) @@ -78,6 +80,7 @@ def f(): def test_max_delay(monkeypatch): + global mock_sleep_time mock_sleep_time = [0] hit = [0] @@ -101,6 +104,7 @@ def f(): def test_fixed_jitter(monkeypatch): + global mock_sleep_time mock_sleep_time = [0] def mock_sleep(seconds): From ece2b692e1f332a259df8851c07917dbc1aca198 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Sat, 2 May 2020 11:33:06 -0400 Subject: [PATCH 12/22] update to run test. --- tests/test_retry.py | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index 03e32d3..f8511b4 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -1,12 +1,15 @@ 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 mock import threading @@ -14,25 +17,32 @@ from retry.api import retry_call from retry.api import retry +import retry.api -mock_sleep_time = [] +mock_sleep_time = [0] def mock_condition(): + logging_logger.warning('in mock condition') class mockCondition(): def acquire(self): - pass + logging_logger.warning('in acquire') + return True + def release(self): - pass + logging_logger.warning('in release') + def wait(self, seconds): + logging_logger.warning('in wait') mock_sleep_time[0] += seconds + + logging_logger.warning('in mock condition, returning') return mockCondition() -def test_retry(monkeypatch): +def test_retry(): global mock_sleep_time mock_sleep_time = [0] - monkeypatch.setattr(threading, 'Condition', mock_condition) - + monkeypatch.setattr(retry.api.threading, 'Condition', mock_condition) hit = [0] tries = 5 @@ -83,14 +93,14 @@ def test_max_delay(monkeypatch): global mock_sleep_time mock_sleep_time = [0] + monkeypatch.setattr(retry.api.threading, 'Condition', mock_condition) + hit = [0] tries = 5 delay = 1 backoff = 2 max_delay = delay # Never increase delay - - monkeypatch.setattr(threading, 'Condition', mock_condition) @retry(tries=tries, delay=delay, max_delay=max_delay, backoff=backoff) def f(): @@ -107,10 +117,7 @@ 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', mock_condition) hit = [0] From 28c1b4256775dc4040e21dd08727abb61d44017f Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Sat, 2 May 2020 11:50:07 -0400 Subject: [PATCH 13/22] Update test_retry.py --- tests/test_retry.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index f8511b4..b58b2fb 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -7,10 +7,6 @@ from mock import MagicMock import mock - - -import mock - import threading import pytest From d71066bbbf8ae0f4e31e6ba211e5733436bd5109 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Sat, 2 May 2020 11:52:39 -0400 Subject: [PATCH 14/22] Update test_retry.py --- tests/test_retry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index b58b2fb..0d9feef 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -34,7 +34,7 @@ def wait(self, seconds): return mockCondition() -def test_retry(): +def test_retry(monkeypatch): global mock_sleep_time mock_sleep_time = [0] From 3b040d881f05d76ceb1752107bf9f19d97f637e0 Mon Sep 17 00:00:00 2001 From: ys Date: Sat, 2 May 2020 13:30:15 -0400 Subject: [PATCH 15/22] 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, bypasses the GIL (global interpreter lock). --- retry/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/retry/api.py b/retry/api.py index acdf062..32a2056 100644 --- a/retry/api.py +++ b/retry/api.py @@ -28,6 +28,7 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, :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. + :returns: the result of the f function. """ _tries, _delay = tries, delay while _tries: From 13b9e3f84c733a98eb38f5da6383ccf3cae6c550 Mon Sep 17 00:00:00 2001 From: ys Date: Sat, 2 May 2020 13:31:30 -0400 Subject: [PATCH 16/22] 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, bypasses the GIL (global interpreter lock). --- retry/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/retry/api.py b/retry/api.py index 32a2056..3529720 100644 --- a/retry/api.py +++ b/retry/api.py @@ -27,7 +27,7 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, 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. + :returns: the result of the f function. """ _tries, _delay = tries, delay From 595228b07ff933c3aceed2b850860a48f551a482 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:14:26 -0400 Subject: [PATCH 17/22] New Workflow --- .github/workflows/pythonapp.yml | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) 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..900e442 --- /dev/null +++ b/.github/workflows/pythonapp.yml @@ -0,0 +1,36 @@ +# 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: | + pytest From 921adaf941dda0a7a9505c6074484ad5dab7e5a1 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:23:10 -0400 Subject: [PATCH 18/22] Changed pip install directions in document. This is to point the installer to this repo. --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index e1d0e9c..295a3f5 100644 --- a/README.rst +++ b/README.rst @@ -27,7 +27,7 @@ Installation .. code-block:: bash - $ pip install retry + $ pip install git+https://github.com/brookman1/retry.git API From 1cc30ea06c9731676fdb436d8f699cabb51a0806 Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:26:10 -0400 Subject: [PATCH 19/22] Fixing tests. --- tests/test_retry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index 64f45cd..d5bcbe2 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -12,8 +12,8 @@ import pytest -from retry.api import retry_call -from retry.api import retry +from ..retry.api import retry_call +from ..retry.api import retry def test_retry(monkeypatch): From 3bf98e17eb6682ca06d9d89297816011ee2d269f Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 22:33:08 -0400 Subject: [PATCH 20/22] # This is a combination of 6 commits. # This is the 1st commit message: Update pythonapp.yml # This is the commit message #2: revert the fix. # This is the commit message #3: fixed syntax # This is the commit message #4: testing threading.Condition instead of time.sleep # This is the commit message #5: updatin how mock_sleep_time scope works. # This is the commit message #6: Fixed unittest. --- .github/workflows/pythonapp.yml | 1 + retry/api.py | 4 +- tests/test_retry.py | 69 +++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/.github/workflows/pythonapp.yml b/.github/workflows/pythonapp.yml index 900e442..238bab6 100644 --- a/.github/workflows/pythonapp.yml +++ b/.github/workflows/pythonapp.yml @@ -33,4 +33,5 @@ jobs: 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 3529720..b892e75 100644 --- a/retry/api.py +++ b/retry/api.py @@ -42,7 +42,7 @@ 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) - # the three lines below, sleep. + # the three lines below, sleep _delay seconds. condition.acquire() condition.wait(_delay) condition.release() @@ -58,7 +58,7 @@ def __retry_internal(f, exceptions=Exception, tries=-1, delay=0, max_delay=None, def retry(exceptions=Exception, tries=-1, delay=0, max_delay=None, backoff=1, jitter=0, logger=logging_logger, - condition=threading.Condition): + condition=threading.Condition()): """Returns a retry decorator. :param exceptions: an exception or a tuple of exceptions to catch. default: Exception. diff --git a/tests/test_retry.py b/tests/test_retry.py index d5bcbe2..1f7ad8f 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -1,40 +1,53 @@ 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 +59,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 +73,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 +84,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 +96,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 +108,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 +133,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 +146,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 +166,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 +186,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 From 69bc5930b10f85c7ca0ead8118dbd4fa40bce20c Mon Sep 17 00:00:00 2001 From: ys Date: Sat, 2 May 2020 13:33:02 -0400 Subject: [PATCH 21/22] Update pythonapp.yml Update test_retry.py --- tests/test_retry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_retry.py b/tests/test_retry.py index 1f7ad8f..60e73f6 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -36,7 +36,6 @@ def test_retry(monkeypatch): global mock_sleep_time mock_sleep_time = [0] - monkeypatch.setattr(retry.api.threading, 'Condition', mockCondition) hit = [0] From 7ffaaeecbbfc18ab9baa3746e5aa7ffb0240a3fa Mon Sep 17 00:00:00 2001 From: brookman1 <48098113+brookman1@users.noreply.github.com> Date: Wed, 29 Apr 2020 21:44:31 -0400 Subject: [PATCH 22/22] 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, bypasses the GIL (global interpreter lock). Added workflow. --- .github/workflows/pythonapp.yml | 37 ++++++++++++++++++ README.rst | 2 +- retry/api.py | 28 ++++++++++---- tests/test_retry.py | 67 ++++++++++++++++++--------------- 4 files changed, 95 insertions(+), 39 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/README.rst b/README.rst index e1d0e9c..295a3f5 100644 --- a/README.rst +++ b/README.rst @@ -27,7 +27,7 @@ Installation .. code-block:: bash - $ pip install retry + $ pip install git+https://github.com/brookman1/retry.git API 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..b68ac53 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -1,40 +1,51 @@ 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 +57,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 +71,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 +82,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 +94,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 +106,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 +131,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 +144,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 +164,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 +184,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