feat(lock_store): make locking backend overridable#6015
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a runtime-pluggable lock backend to lock_store: a ChangesPluggable Lock Backend System
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Allow the centralised lock factory to use a pluggable backend instead of the hardcoded Redis/file selection. Backends are resolved with precedence override > CREWAI_LOCK_FACTORY env > built-in default: - set_lock_backend()/reset_lock_backend() and a scoped lock_backend() context manager for programmatic overrides - CREWAI_LOCK_FACTORY="module:callable" env import-path, resolved lazily and cached, with clear errors on malformed or non-callable specs - LockBackend Protocol documenting the contract (raw name in, context manager out; backend owns its namespacing) Default Redis/file behavior is unchanged when nothing is overridden.
Replace the no-op `...` body with `raise NotImplementedError` to satisfy the CodeQL ineffectual-statement check while keeping the Protocol structural-typing only.
Keep the backend overridable via set_lock_backend/reset_lock_backend and the CREWAI_LOCK_FACTORY env path, but remove the scoped lock_backend() context manager. It was speculative surface and the only thread-unsafe piece (racy save/restore of the module global); nothing depends on it.
reset_lock_backend() was just set_lock_backend(None); callers use that directly. Clearing the override is documented on set_lock_backend.
Reduce the override surface to just set_lock_backend(): lock() uses the custom backend when one is set, otherwise the unchanged Redis/file default. Drop the CREWAI_LOCK_FACTORY env import-path, the runtime_checkable Protocol, the precedence resolver, and the getter — a custom backend is now any callable(name, *, timeout) -> context manager, registered in process.
mattatcha
force-pushed
the
matcha/overridable-lock-backend
branch
from
f8f382e to
986dfff
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 986dfff. Configure here.
greysonlalonde
requested changes
Jun 4, 2026
| Automatically namespaced to avoid collisions. | ||
| timeout: Maximum seconds to wait for the lock before raising. | ||
| """ | ||
| if _backend is not None: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai-core/src/crewai_core/lock_store.py (1)
37-39: ⚡ Quick winConsider a Protocol for stricter typing.
The
Callable[..., AbstractContextManager[None]]type accepts any arguments, which doesn't enforce the expectedbackend(name, *, timeout=...)signature documented in the comment. This loses IDE autocomplete and type-checker validation.🔍 Suggested Protocol-based type definition
-# A backend is called as ``backend(name, timeout=...)`` and returns a context -# manager that holds the lock while the ``with`` block runs. -LockBackend = Callable[..., AbstractContextManager[None]] +# A backend is called as ``backend(name, timeout=...)`` and returns a context +# manager that holds the lock while the ``with`` block runs. +from typing import Protocol + +class LockBackend(Protocol): + """Protocol for custom lock backends.""" + def __call__(self, name: str, *, timeout: float) -> AbstractContextManager[None]: ...Note: If you prefer the flexibility of accepting any signature, the current
Callable[..., ...]approach is acceptable and you can skip this change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai-core/src/crewai_core/lock_store.py` around lines 37 - 39, The current LockBackend alias uses Callable[..., AbstractContextManager[None]] which permits any signature; replace it with a Protocol that enforces the expected backend(name: str, *, timeout: float | None = ...) -> AbstractContextManager[None] signature so IDEs and type-checkers can validate and autocomplete; define a LockBackendProtocol (or similar) implementing __call__(self, name: str, *, timeout: float | None = ...) -> AbstractContextManager[None] and update the LockBackend type annotation to reference that Protocol instead of Callable[..., ...].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai-core/src/crewai_core/lock_store.py`:
- Around line 45-51: Update the docstring of set_lock_backend to state the
intended usage and thread-safety expectations: explain that set_lock_backend
replaces the global _backend for the whole process and is intended as a one-time
initialization (call before spawning threads or concurrent operations), that
changing _backend at runtime is not synchronized and may result in some threads
using the old backend while others use the new one, and that if dynamic runtime
switching is required callers should add their own synchronization (or we could
switch to protecting _backend with a threading.Lock). Refer to set_lock_backend
and the global _backend in the docstring so readers know exactly which symbol is
affected.
---
Nitpick comments:
In `@lib/crewai-core/src/crewai_core/lock_store.py`:
- Around line 37-39: The current LockBackend alias uses Callable[...,
AbstractContextManager[None]] which permits any signature; replace it with a
Protocol that enforces the expected backend(name: str, *, timeout: float | None
= ...) -> AbstractContextManager[None] signature so IDEs and type-checkers can
validate and autocomplete; define a LockBackendProtocol (or similar)
implementing __call__(self, name: str, *, timeout: float | None = ...) ->
AbstractContextManager[None] and update the LockBackend type annotation to
reference that Protocol instead of Callable[..., ...].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9eaa577-4744-46d4-81a4-206198c3c3c9
📒 Files selected for processing (2)
lib/crewai-core/src/crewai_core/lock_store.pylib/crewai/tests/utilities/test_lock_store.py
Read the module-global backend once into a local before the None check and the call, so a concurrent set_lock_backend(None) cannot make lock() invoke None.
The default namespaces the lock name; custom backends receive it verbatim. Correct the lock() docstring which implied namespacing always happens.
greysonlalonde
approved these changes
Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allow
crewai_core.lock_storeto use a custom locking backend instead of the built-in Redis/file selection, without touching call sites.Key Changes
set_lock_backend(backend)— register a custom backend; passNoneto restore the default.lock()uses the custom backend when one is set, otherwise the unchanged Redis/file logic.callable(name, *, timeout)returning a context manager that holds the lock for thewithblock.Linear Issues
Note
Low Risk
Additive API with unchanged default locking path; main caveat is unsynchronized backend swaps under concurrent lock use, which is documented for startup-only setup.
Overview
Adds a process-wide pluggable lock backend so callers can swap Redis/file locking for custom implementations (e.g. in-process locks in tests or another distributed service) without changing existing
lock()call sites.set_lock_backend(backend)registers a callable(name, *, timeout) -> context manager; passNoneto restore the built-in path.lock()snapshots the current backend before use so a concurrent swap cannot break acquisition, and when a custom backend is set it delegates directly with the raw lock name (the default path still namespaces names viacrewai:+ hash).Default Redis vs file selection is unchanged when no backend is registered. Tests add autouse reset of the backend plus coverage that overrides bypass
portalockerand clearing the backend restores file locking.Reviewed by Cursor Bugbot for commit b4c097d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit