From efce4b68a652405ffe1b351fd4fe940f62d42960 Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Tue, 21 Dec 2021 16:53:24 -0800 Subject: [PATCH 1/4] Use common logger object across entire library The only exception is the `monkey.py` module, because I don't want to introduce a dependency in our monkey patches to any other module in the library. --- pottery/__init__.py | 20 +++++++++++--------- pottery/base.py | 10 +++++----- pottery/cache.py | 6 ++---- pottery/nextid.py | 14 +++++--------- pottery/redlock.py | 25 +++++++++++-------------- tests/base.py | 2 +- tests/test_redlock.py | 8 ++++---- 7 files changed, 39 insertions(+), 46 deletions(-) diff --git a/pottery/__init__.py b/pottery/__init__.py index b27e438a..e6d7fc89 100644 --- a/pottery/__init__.py +++ b/pottery/__init__.py @@ -26,18 +26,20 @@ from typing import Tuple +# TODO: When we drop support for Python 3.7, change the following import to: +# from typing import Final from typing_extensions import Final -__title__ = 'pottery' -__version__ = '2.2.1' -__description__ = __doc__.split(sep='\n\n', maxsplit=1)[0] -__url__ = 'https://github.com/brainix/pottery' -__author__ = 'Rajiv Bakulesh Shah' -__author_email__ = 'brainix@gmail.com' -__license__ = 'Apache 2.0' -__keywords__ = 'Redis client persistent storage' -__copyright__ = f'Copyright © 2015-2021, {__author__}, original author.' +__title__: Final[str] = 'pottery' +__version__: Final[str] = '2.2.1' +__description__: Final[str] = __doc__.split(sep='\n\n', maxsplit=1)[0] +__url__: Final[str] = 'https://github.com/brainix/pottery' +__author__: Final[str] = 'Rajiv Bakulesh Shah' +__author_email__: Final[str] = 'brainix@gmail.com' +__license__: Final[str] = 'Apache 2.0' +__keywords__: Final[str] = 'Redis client persistent storage' +__copyright__: Final[str] = f'Copyright © 2015-2021, {__author__}, original author.' from .exceptions import PotteryError # isort:skip diff --git a/pottery/base.py b/pottery/base.py index 9b417362..dbc4d48a 100644 --- a/pottery/base.py +++ b/pottery/base.py @@ -48,9 +48,9 @@ from .exceptions import RandomKeyError +logger: Final[logging.Logger] = logging.getLogger('pottery') _default_url: Final[str] = os.environ.get('REDIS_URL', 'redis://localhost:6379/') _default_redis: Final[Redis] = Redis.from_url(_default_url, socket_timeout=1) -_logger: Final[logging.Logger] = logging.getLogger('pottery') def random_key(*, @@ -93,7 +93,7 @@ def __init__(self, def __del__(self) -> None: if self.key.startswith(self._RANDOM_KEY_PREFIX): self.redis.delete(self.key) - _logger.warning( + logger.warning( "Deleted tmp <%s key='%s'> (instance is about to be destroyed)", self.__class__.__name__, self.key, @@ -117,7 +117,7 @@ def key(self, value: str) -> None: def _random_key(self) -> str: key = random_key(redis=self.redis, prefix=self._RANDOM_KEY_PREFIX) - _logger.warning( + logger.warning( "Self-assigning tmp key <%s key='%s'>", self.__class__.__name__, key, @@ -186,14 +186,14 @@ def __watch_keys(self, try: yield pipeline except Exception as error: - _logger.warning( + logger.warning( 'Caught %s; aborting pipeline of %d commands', error.__class__.__name__, len(pipeline), ) raise else: - _logger.info( + logger.info( 'Running EXEC on pipeline of %d commands', len(pipeline), ) diff --git a/pottery/cache.py b/pottery/cache.py index 532573ba..5de38e5e 100644 --- a/pottery/cache.py +++ b/pottery/cache.py @@ -20,7 +20,6 @@ import contextlib import functools import itertools -import logging from typing import Any from typing import Callable from typing import ClassVar @@ -39,6 +38,7 @@ from .base import JSONTypes from .base import _default_redis +from .base import logger from .base import random_key from .dict import InitArg from .dict import InitIter @@ -48,8 +48,6 @@ _DEFAULT_TIMEOUT: Final[int] = 60 # seconds -_logger: Final[logging.Logger] = logging.getLogger('pottery') - class CacheInfo(NamedTuple): '''Caching decorator information. @@ -129,7 +127,7 @@ def decorator(func: F) -> F: nonlocal redis, key if key is None: # pragma: no cover key = random_key(redis=cast(Redis, redis)) - _logger.warning( + logger.warning( "Self-assigning key redis_cache(key='%s') for function %s", key, func.__qualname__, diff --git a/pottery/nextid.py b/pottery/nextid.py index 0981cd0b..27431310 100644 --- a/pottery/nextid.py +++ b/pottery/nextid.py @@ -26,7 +26,6 @@ import concurrent.futures import contextlib -import logging from typing import ClassVar from typing import Iterable from typing import List @@ -38,17 +37,14 @@ from redis import Redis from redis import RedisError from redis.client import Script -from typing_extensions import Final from .base import Primitive +from .base import logger from .exceptions import QuorumIsImpossible from .exceptions import QuorumNotAchieved from .executor import BailOutExecutor -_logger: Final[logging.Logger] = logging.getLogger('pottery') - - class _Scripts: '''Parent class to define/register Lua scripts for Redis. @@ -79,7 +75,7 @@ def __init__(self, def __register_set_id_script(self) -> None: if self._set_id_script is None: class_name = self.__class__.__qualname__ - _logger.info('Registering %s._set_id_script', class_name) + logger.info('Registering %s._set_id_script', class_name) master = next(iter(self.masters)) # type: ignore self.__class__._set_id_script = master.register_script(''' local curr = tonumber(redis.call('get', KEYS[1])) @@ -179,7 +175,7 @@ def __current_id(self) -> int: current_id = int(cast(bytes, future.result() or b'0')) except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.__current_id() getter caught %s', self.__class__.__name__, error.__class__.__name__, @@ -215,7 +211,7 @@ def __current_id(self, value: int) -> None: num_masters_set += future.result() == value except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.__current_id() setter caught %s', self.__class__.__name__, error.__class__.__name__, @@ -245,7 +241,7 @@ def reset(self) -> None: future.result() except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.reset() caught %s', self.__class__.__name__, error.__class__.__name__, diff --git a/pottery/redlock.py b/pottery/redlock.py index 2f2adb20..da7878dd 100644 --- a/pottery/redlock.py +++ b/pottery/redlock.py @@ -35,7 +35,6 @@ import concurrent.futures import contextlib import functools -import logging import math import random import time @@ -59,6 +58,7 @@ from .annotations import F from .base import Primitive +from .base import logger from .exceptions import ExtendUnlockedLock from .exceptions import QuorumNotAchieved from .exceptions import ReleaseUnlockedLock @@ -70,9 +70,6 @@ AUTO_RELEASE_TIME: Final[int] = 10 * 1000 -_logger: Final[logging.Logger] = logging.getLogger('pottery') - - class _Scripts: '''Parent class to define/register Lua scripts for Redis. @@ -107,7 +104,7 @@ def __init__(self, def __register_acquired_script(self) -> None: if self._acquired_script is None: class_name = self.__class__.__qualname__ - _logger.info('Registering %s._acquired_script', class_name) + logger.info('Registering %s._acquired_script', class_name) master = next(iter(self.masters)) # type: ignore self.__class__._acquired_script = master.register_script(''' if redis.call('get', KEYS[1]) == ARGV[1] then @@ -121,7 +118,7 @@ def __register_acquired_script(self) -> None: def __register_extend_script(self) -> None: if self._extend_script is None: class_name = self.__class__.__qualname__ - _logger.info('Registering %s._extend_script', class_name) + logger.info('Registering %s._extend_script', class_name) master = next(iter(self.masters)) # type: ignore self.__class__._extend_script = master.register_script(''' if redis.call('get', KEYS[1]) == ARGV[1] then @@ -134,7 +131,7 @@ def __register_extend_script(self) -> None: def __register_release_script(self) -> None: if self._release_script is None: class_name = self.__class__.__qualname__ - _logger.info('Registering %s._release_script', class_name) + logger.info('Registering %s._release_script', class_name) master = next(iter(self.masters)) # type: ignore self.__class__._release_script = master.register_script(''' if redis.call('get', KEYS[1]) == ARGV[1] then @@ -319,7 +316,7 @@ def __acquire_masters(self, num_masters_acquired += future.result() except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.__acquire_masters() caught %s', self.__class__.__name__, error.__class__.__name__, @@ -396,7 +393,7 @@ def acquire(self, def log_time_enqueued(timer: ContextTimer, acquired: bool) -> None: key_suffix = self.key.split(':', maxsplit=1)[1] time_enqueued = math.ceil(timer.elapsed()) - _logger.info( + logger.info( 'source=pottery sample#redlock.enqueued.%s=%dms sample#redlock.acquired.%s=%d', key_suffix, time_enqueued, @@ -465,7 +462,7 @@ def locked(self, *, raise_on_redis_errors: Optional[bool] = None) -> int: ttl = future.result() except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.locked() caught %s', self.__class__.__name__, error.__class__.__name__, @@ -519,7 +516,7 @@ def extend(self, *, raise_on_redis_errors: Optional[bool] = None) -> None: num_masters_extended += future.result() except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.extend() caught %s', self.__class__.__name__, error.__class__.__name__, @@ -566,7 +563,7 @@ def release(self, *, raise_on_redis_errors: Optional[bool] = None) -> None: num_masters_released += future.result() except RedisError as error: redis_errors.append(error) - _logger.exception( + logger.exception( '%s.release() caught %s', self.__class__.__name__, error.__class__.__name__, @@ -726,7 +723,7 @@ def _log_synchronize(func: F, holding_timer: ContextTimer, ) -> None: try: - _logger.info( + logger.info( '%s() waited for %s for %d ms; held for %d ms', func.__qualname__, redlock.key, @@ -737,7 +734,7 @@ def _log_synchronize(func: F, # holding_timer.elapsed() threw a RuntimeError, which means that # holding_timer never started, which means that we never acquired the # lock / entered the critical section. - _logger.info( + logger.info( '%s() waited for %s for %d ms; never acquired lock', func.__qualname__, redlock.key, diff --git a/tests/base.py b/tests/base.py index fdb2c0f2..3c530907 100644 --- a/tests/base.py +++ b/tests/base.py @@ -27,12 +27,12 @@ from redis import Redis from pottery import PotteryWarning +from pottery.base import logger class TestCase(unittest.TestCase): @classmethod def setUpClass(cls) -> None: - logger = logging.getLogger('pottery') logger.setLevel(logging.CRITICAL) warnings.filterwarnings('ignore', category=PotteryWarning) diff --git a/tests/test_redlock.py b/tests/test_redlock.py index f7aafc82..e6e40535 100644 --- a/tests/test_redlock.py +++ b/tests/test_redlock.py @@ -34,7 +34,7 @@ from pottery import ReleaseUnlockedLock from pottery import TooManyExtensions from pottery import synchronize -from pottery.redlock import _logger +from pottery.base import logger from tests.base import TestCase @@ -59,7 +59,7 @@ def test_acquire_and_time_out(self): def test_acquire_same_lock_twice_blocking_without_timeout(self): assert not self.redis.exists(self.redlock.key) with ContextTimer() as timer, \ - unittest.mock.patch.object(_logger, 'info') as info: + unittest.mock.patch.object(logger, 'info') as info: assert self.redlock.acquire() assert self.redis.exists(self.redlock.key) assert self.redlock.acquire() @@ -69,7 +69,7 @@ def test_acquire_same_lock_twice_blocking_without_timeout(self): @unittest.skipIf('CI' in os.environ, 'this unit test is flaky on CI') def test_acquire_same_lock_twice_blocking_with_timeout(self): - with unittest.mock.patch.object(_logger, 'info') as info: + with unittest.mock.patch.object(logger, 'info') as info: assert not self.redis.exists(self.redlock.key) assert self.redlock.acquire() assert self.redis.exists(self.redlock.key) @@ -300,7 +300,7 @@ def func(): time.sleep(1) return time.time() - with unittest.mock.patch.object(_logger, 'info') as info: + with unittest.mock.patch.object(logger, 'info') as info: with concurrent.futures.ThreadPoolExecutor() as executor: futures = {executor.submit(func) for _ in range(3)} results = sorted(future.result() for future in futures) From 1458bf904e3110b417122c405b2c789d9f000c80 Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Tue, 21 Dec 2021 16:56:17 -0800 Subject: [PATCH 2/4] Write TODOs --- pottery/base.py | 3 +++ pottery/cache.py | 3 +++ pottery/monkey.py | 2 ++ pottery/redlock.py | 4 ++++ 4 files changed, 12 insertions(+) diff --git a/pottery/base.py b/pottery/base.py index dbc4d48a..268990c1 100644 --- a/pottery/base.py +++ b/pottery/base.py @@ -40,6 +40,9 @@ from redis import Redis from redis import RedisError from redis.client import Pipeline + +# TODO: When we drop support for Python 3.7, change the following import to: +# from typing import Final from typing_extensions import Final from . import monkey diff --git a/pottery/cache.py b/pottery/cache.py index 5de38e5e..c868af09 100644 --- a/pottery/cache.py +++ b/pottery/cache.py @@ -34,6 +34,9 @@ from redis import Redis from redis.exceptions import WatchError + +# TODO: When we drop support for Python 3.7, change the following import to: +# from typing import Final from typing_extensions import Final from .base import JSONTypes diff --git a/pottery/monkey.py b/pottery/monkey.py index 5fadd0da..21810626 100644 --- a/pottery/monkey.py +++ b/pottery/monkey.py @@ -23,6 +23,8 @@ from typing import List from typing import Union +# TODO: When we drop support for Python 3.7, change the following import to: +# from typing import Final from typing_extensions import Final diff --git a/pottery/redlock.py b/pottery/redlock.py index da7878dd..7b9632d1 100644 --- a/pottery/redlock.py +++ b/pottery/redlock.py @@ -53,7 +53,11 @@ from redis import Redis from redis import RedisError from redis.client import Script + +# TODO: When we drop support for Python 3.7, change the following import to: +# from typing import Final from typing_extensions import Final + from typing_extensions import Literal from .annotations import F From 89390c6c9ebe7cb4a82b709d54f5d6376a442bc5 Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Tue, 21 Dec 2021 17:10:55 -0800 Subject: [PATCH 3/4] Fix linting --- pottery/base.py | 1 - pottery/cache.py | 1 - pottery/redlock.py | 2 -- 3 files changed, 4 deletions(-) diff --git a/pottery/base.py b/pottery/base.py index 268990c1..9c3996f5 100644 --- a/pottery/base.py +++ b/pottery/base.py @@ -40,7 +40,6 @@ from redis import Redis from redis import RedisError from redis.client import Pipeline - # TODO: When we drop support for Python 3.7, change the following import to: # from typing import Final from typing_extensions import Final diff --git a/pottery/cache.py b/pottery/cache.py index c868af09..349e951d 100644 --- a/pottery/cache.py +++ b/pottery/cache.py @@ -34,7 +34,6 @@ from redis import Redis from redis.exceptions import WatchError - # TODO: When we drop support for Python 3.7, change the following import to: # from typing import Final from typing_extensions import Final diff --git a/pottery/redlock.py b/pottery/redlock.py index 7b9632d1..31cc21b9 100644 --- a/pottery/redlock.py +++ b/pottery/redlock.py @@ -53,11 +53,9 @@ from redis import Redis from redis import RedisError from redis.client import Script - # TODO: When we drop support for Python 3.7, change the following import to: # from typing import Final from typing_extensions import Final - from typing_extensions import Literal from .annotations import F From 33c9478132e5add6f5b6c72fcb819a1693bca765 Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Tue, 21 Dec 2021 17:18:27 -0800 Subject: [PATCH 4/4] Write TODO --- pottery/base.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pottery/base.py b/pottery/base.py index 9c3996f5..7152509d 100644 --- a/pottery/base.py +++ b/pottery/base.py @@ -40,9 +40,11 @@ from redis import Redis from redis import RedisError from redis.client import Pipeline -# TODO: When we drop support for Python 3.7, change the following import to: +# TODO: When we drop support for Python 3.7, change the following imports to: # from typing import Final +# from typing import final from typing_extensions import Final +from typing_extensions import final from . import monkey from .annotations import JSONTypes @@ -135,11 +137,13 @@ def _random_key(self) -> str: class _Encodable: 'Mixin class that implements JSON encoding and decoding.' + @final @staticmethod def _encode(value: JSONTypes) -> str: encoded = json.dumps(value, sort_keys=True) return encoded + @final @staticmethod def _decode(value: AnyStr) -> JSONTypes: try: @@ -179,6 +183,7 @@ def redis(self) -> Redis: def key(self) -> str: 'Redis key.' + @final @contextlib.contextmanager def __watch_keys(self, *keys: str, @@ -201,6 +206,7 @@ def __watch_keys(self, ) pipeline.execute() + @final def __context_managers(self, *others: Any, ) -> Generator[ContextManager[Pipeline], None, None]: @@ -216,6 +222,7 @@ def __context_managers(self, pipeline = containers[0].__watch_keys(*keys) yield pipeline + @final @contextlib.contextmanager def _watch(self, *others: Any,