Fix lock_store crash when redis package is not installed (#4943)
Some checks failed
CodeQL Advanced / Analyze (actions) (push) Has been cancelled
CodeQL Advanced / Analyze (python) (push) Has been cancelled
Nightly Canary Release / Check for new commits (push) Has been cancelled
Nightly Canary Release / Build nightly packages (push) Has been cancelled
Nightly Canary Release / Publish nightly to PyPI (push) Has been cancelled
Mark stale issues and pull requests / stale (push) Has been cancelled

* Fix lock_store crash when redis package is not installed

`REDIS_URL` being set was enough to trigger a Redis lock, which would
raise `ImportError` if the `redis` package wasn't available. Added
`_redis_available()` to guard on both the env var and the import.

* Simplify tests

* Simplify tests #2
This commit is contained in:
Vini Brasil
2026-03-18 15:05:41 -03:00
committed by GitHub
parent 6a6adaf2da
commit 6b262f5a6d
2 changed files with 85 additions and 3 deletions

View File

@@ -1,7 +1,7 @@
"""Centralised lock factory.
If ``REDIS_URL`` is set, locks are distributed via ``portalocker.RedisLock``. Otherwise, falls
back to the standard ``portalocker.Lock``.
If ``REDIS_URL`` is set and the ``redis`` package is installed, locks are distributed via
``portalocker.RedisLock``. Otherwise, falls back to the standard ``portalocker.Lock``.
"""
from __future__ import annotations
@@ -30,6 +30,18 @@ _REDIS_URL: str | None = os.environ.get("REDIS_URL")
_DEFAULT_TIMEOUT: Final[int] = 120
def _redis_available() -> bool:
"""Return True if redis is installed and REDIS_URL is set."""
if not _REDIS_URL:
return False
try:
import redis # noqa: F401
return True
except ImportError:
return False
@lru_cache(maxsize=1)
def _redis_connection() -> redis.Redis:
"""Return a cached Redis connection, creating one on first call."""
@@ -51,7 +63,7 @@ def lock(name: str, *, timeout: float = _DEFAULT_TIMEOUT) -> Iterator[None]:
"""
channel = f"crewai:{md5(name.encode(), usedforsecurity=False).hexdigest()}"
if _REDIS_URL:
if _redis_available():
with portalocker.RedisLock(
channel=channel,
connection=_redis_connection(),

View File

@@ -0,0 +1,70 @@
"""Tests for lock_store.
We verify our own logic: the _redis_available guard and which portalocker
backend is selected. We trust portalocker to handle actual locking mechanics.
"""
from __future__ import annotations
import sys
from unittest import mock
import pytest
import crewai.utilities.lock_store as lock_store
from crewai.utilities.lock_store import lock
@pytest.fixture(autouse=True)
def no_redis_url(monkeypatch):
monkeypatch.setattr(lock_store, "_REDIS_URL", None)
# ---------------------------------------------------------------------------
# _redis_available
# ---------------------------------------------------------------------------
def test_redis_not_available_without_url():
assert lock_store._redis_available() is False
def test_redis_not_available_when_package_missing(monkeypatch):
monkeypatch.setattr(lock_store, "_REDIS_URL", "redis://localhost:6379")
monkeypatch.setitem(sys.modules, "redis", None) # None → ImportError on import
assert lock_store._redis_available() is False
def test_redis_available_with_url_and_package(monkeypatch):
monkeypatch.setattr(lock_store, "_REDIS_URL", "redis://localhost:6379")
monkeypatch.setitem(sys.modules, "redis", mock.MagicMock())
assert lock_store._redis_available() is True
# ---------------------------------------------------------------------------
# lock strategy selection
# ---------------------------------------------------------------------------
def test_uses_file_lock_when_redis_unavailable():
with mock.patch("portalocker.Lock") as mock_lock:
with lock("file_test"):
pass
mock_lock.assert_called_once()
assert "crewai:" in mock_lock.call_args.args[0]
def test_uses_redis_lock_when_redis_available(monkeypatch):
fake_conn = mock.MagicMock()
monkeypatch.setattr(lock_store, "_redis_available", mock.Mock(return_value=True))
monkeypatch.setattr(lock_store, "_redis_connection", mock.Mock(return_value=fake_conn))
with mock.patch("portalocker.RedisLock") as mock_redis_lock:
with lock("redis_test"):
pass
mock_redis_lock.assert_called_once()
kwargs = mock_redis_lock.call_args.kwargs
assert kwargs["channel"].startswith("crewai:")
assert kwargs["connection"] is fake_conn