-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/redis set functions #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a4f508e
e643def
70ba98c
1d6c804
edb5a9a
8de15b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Python bytecode / caches | ||
| __pycache__/ | ||
| *.py[cod] | ||
| *$py.class | ||
| *.so | ||
| .Python | ||
|
|
||
| # Virtualenvs | ||
| .venv/ | ||
| venv/ | ||
| env/ | ||
| ENV/ | ||
|
|
||
| # Build / dist artifacts | ||
| build/ | ||
| dist/ | ||
| *.egg-info/ | ||
| .eggs/ | ||
| wheels/ | ||
| pip-wheel-metadata/ | ||
|
|
||
| # Test / type-check / lint caches | ||
| .pytest_cache/ | ||
| .mypy_cache/ | ||
| .ruff_cache/ | ||
| .tox/ | ||
| .nox/ | ||
| .coverage | ||
| .coverage.* | ||
| htmlcov/ | ||
| coverage.xml | ||
|
|
||
| # Local-only dev / smoke-test scripts (kept out of source control) | ||
| scripts/ | ||
|
|
||
| # Editor / IDE | ||
| .vscode/ | ||
| .idea/ | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # OS | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Secrets / env | ||
| .env | ||
| .env.* | ||
| !.env.example |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,8 +27,25 @@ async def exists(self, key: str) -> bool: | |||||||||||||||||
| """Return ``True`` if *key* exists.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def expire(self, key: str, seconds: int) -> bool: | ||||||||||||||||||
| """Set a timeout on *key*. Returns ``True`` if the timeout was set.""" | ||||||||||||||||||
| async def expire( | ||||||||||||||||||
| self, | ||||||||||||||||||
| key: str, | ||||||||||||||||||
| seconds: int, | ||||||||||||||||||
| nx: bool = False, | ||||||||||||||||||
| xx: bool = False, | ||||||||||||||||||
| ) -> bool: | ||||||||||||||||||
| """Set a timeout on *key*. Returns ``True`` if the timeout was set. | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| key: Target key. | ||||||||||||||||||
| seconds: TTL in seconds. | ||||||||||||||||||
| nx: Only set the TTL if the key has no existing TTL. | ||||||||||||||||||
| xx: Only set the TTL if the key already has a TTL. | ||||||||||||||||||
|
|
||||||||||||||||||
| ``nx`` and ``xx`` are mutually exclusive. Backends that don't support | ||||||||||||||||||
| these flags natively should emulate them via ``ttl(key)`` and document | ||||||||||||||||||
| that the operation is not atomic. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def ttl(self, key: str) -> int: | ||||||||||||||||||
|
|
@@ -54,6 +71,39 @@ async def hgetall(self, key: str) -> dict[bytes, bytes]: | |||||||||||||||||
| async def hdel(self, key: str, *fields: str) -> int: | ||||||||||||||||||
| """Delete fields from the hash at *key*. Returns number of fields removed.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def sadd(self, key: str, *members: bytes | str) -> int: | ||||||||||||||||||
| """Add one or more *members* to the set at *key*. | ||||||||||||||||||
|
|
||||||||||||||||||
| Returns the number of members that were newly added (i.e. not already | ||||||||||||||||||
| present). This "was-new" signal is the foundation of unique-element | ||||||||||||||||||
| deduplication patterns (e.g. DAU/MAU tracking). | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def srem(self, key: str, *members: bytes | str) -> int: | ||||||||||||||||||
| """Remove one or more *members* from the set at *key*. Returns the number removed.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def scard(self, key: str) -> int: | ||||||||||||||||||
| """Return the number of elements in the set at *key*.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def sismember(self, key: str, member: bytes | str) -> bool: | ||||||||||||||||||
| """Return ``True`` if *member* is in the set at *key*.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def smembers(self, key: str) -> "set[bytes]": | ||||||||||||||||||
| """Return all members of the set at *key*.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def sinter(self, *keys: str) -> "set[bytes]": | ||||||||||||||||||
| """Return the members common to all sets at *keys* (set intersection). | ||||||||||||||||||
|
|
||||||||||||||||||
| With a single key this is equivalent to :meth:`smembers`. A missing key | ||||||||||||||||||
| is treated as an empty set, so any missing key yields an empty result. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
| @abstractmethod | ||||||||||||||||||
| async def lpush(self, key: str, *values: bytes | str) -> int: | ||||||||||||||||||
| """Prepend values to the list at *key*. Returns new list length.""" | ||||||||||||||||||
|
|
@@ -102,6 +152,27 @@ async def setex(self, key: str, value: bytes | str, ttl: int) -> None: | |||||||||||||||||
| """Atomic set-with-TTL. Default delegates to ``set(key, value, ttl=ttl)``.""" | ||||||||||||||||||
| await self.set(key, value, ttl=ttl) | ||||||||||||||||||
|
|
||||||||||||||||||
| @asynccontextmanager | ||||||||||||||||||
| async def pipeline(self): | ||||||||||||||||||
| """Batch multiple commands. | ||||||||||||||||||
|
|
||||||||||||||||||
| Usage: | ||||||||||||||||||
| async with cache.pipeline() as pipe: | ||||||||||||||||||
| pipe.sadd("k", "m") | ||||||||||||||||||
| pipe.expire("k", 60) | ||||||||||||||||||
| # commands execute on context exit | ||||||||||||||||||
|
|
||||||||||||||||||
| The default implementation queues calls and replays them sequentially | ||||||||||||||||||
| on exit — it provides no atomicity and no round-trip savings. Redis | ||||||||||||||||||
| backends override this with a true server-side pipeline. Callers that | ||||||||||||||||||
| depend on atomicity or batching performance must check the backend. | ||||||||||||||||||
| """ | ||||||||||||||||||
| pipe = _SequentialPipeline(self) | ||||||||||||||||||
| try: | ||||||||||||||||||
| yield pipe | ||||||||||||||||||
| finally: | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Use
Suggested change
|
||||||||||||||||||
| await pipe.execute() | ||||||||||||||||||
|
|
||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still unresolved: the base pipeline() runs pipe.execute() in a finally block, so a partially-constructed pipeline is silently replayed even if the caller's with-block raised an exception. If pipe.execute() itself then raises, the original exception is replaced. Suggested fix:
Suggested change
Actually the full fix is to change the try/finally to try/except/else: This ensures execute() only runs when the with-block exits cleanly. |
||||||||||||||||||
| async def health_check(self) -> bool: | ||||||||||||||||||
| """Return True if the cache server is reachable.""" | ||||||||||||||||||
| try: | ||||||||||||||||||
|
|
@@ -116,6 +187,33 @@ async def __aexit__(self, exc_type, exc, tb) -> None: | |||||||||||||||||
| await self.close() | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class _SequentialPipeline: | ||||||||||||||||||
| """Default pipeline that records calls and replays them sequentially on execute. | ||||||||||||||||||
|
|
||||||||||||||||||
| Provides no atomicity and no round-trip savings — exists so the | ||||||||||||||||||
| ``pipeline()`` API works on every backend. Redis backends bypass this with | ||||||||||||||||||
| a true server-side pipeline. | ||||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
| def __init__(self, backend: "CacheBackend") -> None: | ||||||||||||||||||
| self._backend = backend | ||||||||||||||||||
| self._ops: list[tuple[str, tuple, dict]] = [] | ||||||||||||||||||
|
|
||||||||||||||||||
| def __getattr__(self, name: str): | ||||||||||||||||||
| def queue(*args, **kwargs): | ||||||||||||||||||
| self._ops.append((name, args, kwargs)) | ||||||||||||||||||
| return self | ||||||||||||||||||
| return queue | ||||||||||||||||||
|
|
||||||||||||||||||
| async def execute(self) -> list: | ||||||||||||||||||
| results = [] | ||||||||||||||||||
| ops, self._ops = self._ops, [] | ||||||||||||||||||
| for name, args, kwargs in ops: | ||||||||||||||||||
| method = getattr(self._backend, name) | ||||||||||||||||||
| results.append(await method(*args, **kwargs)) | ||||||||||||||||||
| return results | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class _RedisMixin: | ||||||||||||||||||
| """Concrete Redis implementation shared by all Redis-backed cache backends. | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -148,9 +246,55 @@ async def exists(self, key: str) -> bool: | |||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def expire(self, key: str, seconds: int) -> bool: | ||||||||||||||||||
| async def expire( | ||||||||||||||||||
| self, | ||||||||||||||||||
| key: str, | ||||||||||||||||||
| seconds: int, | ||||||||||||||||||
| nx: bool = False, | ||||||||||||||||||
| xx: bool = False, | ||||||||||||||||||
| ) -> bool: | ||||||||||||||||||
| if nx and xx: | ||||||||||||||||||
| raise ValueError("expire() flags `nx` and `xx` are mutually exclusive") | ||||||||||||||||||
| try: | ||||||||||||||||||
| return bool(await self._client.expire(key, seconds, nx=nx, xx=xx)) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def sadd(self, key: str, *members: bytes | str) -> int: | ||||||||||||||||||
| try: | ||||||||||||||||||
| return await self._client.sadd(key, *members) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def srem(self, key: str, *members: bytes | str) -> int: | ||||||||||||||||||
| try: | ||||||||||||||||||
| return await self._client.srem(key, *members) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correctness bug — return set(await self._client.sinter(keys))redis-py’s
Suggested change
This mirrors how every other variadic Redis command in this file is called (e.g. |
||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def scard(self, key: str) -> int: | ||||||||||||||||||
| try: | ||||||||||||||||||
| return await self._client.scard(key) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def sismember(self, key: str, member: bytes | str) -> bool: | ||||||||||||||||||
| try: | ||||||||||||||||||
| return bool(await self._client.sismember(key, member)) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def smembers(self, key: str) -> "set[bytes]": | ||||||||||||||||||
| try: | ||||||||||||||||||
| return await self._client.smembers(key) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
| async def sinter(self, *keys: str) -> "set[bytes]": | ||||||||||||||||||
| if not keys: | ||||||||||||||||||
| raise ValueError("sinter() requires at least one key") | ||||||||||||||||||
| try: | ||||||||||||||||||
| return bool(await self._client.expire(key, seconds)) | ||||||||||||||||||
| return set(await self._client.sinter(*keys)) | ||||||||||||||||||
| except RedisError as e: | ||||||||||||||||||
| raise CacheError(str(e)) from e | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring says "results available via
await pipe.execute()if called explicitly inside the block" but_SequentialPipeline.execute()resetsself._ops = []— so calling it inside theasync withblock and then having thefinallybranch call it again will replay an empty list and return[]the second time. That is fine for correctness but the comment implies callers can inspect incremental results mid-block, which they cannot.Suggestion: drop the parenthetical about calling
execute()explicitly, or document that calling it inside the block consumes the queued ops and thefinallybranch becomes a no-op: