fix: replace deprecated datetime.utcnow() with datetime.now(timezone.utc) in memory module#5877
fix: replace deprecated datetime.utcnow() with datetime.now(timezone.utc) in memory module#5877Ghraven wants to merge 2 commits into
Conversation
…utc) datetime.utcnow() has been deprecated since Python 3.12 because it returns a naive datetime that can be ambiguous. Replace all usages in the memory module with datetime.now(timezone.utc), which returns a timezone-aware datetime object. Files changed: - lib/crewai/src/crewai/memory/types.py - lib/crewai/src/crewai/memory/storage/lancedb_storage.py - lib/crewai/src/crewai/memory/encoding_flow.py - lib/crewai/src/crewai/memory/unified_memory.py - lib/crewai/tests/memory/test_unified_memory.py
📝 WalkthroughWalkthroughThis PR migrates the crewAI memory subsystem from naive UTC timestamps ( ChangesTimezone-aware UTC datetime migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/memory/storage/lancedb_storage.py (1)
268-275:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winNormalize legacy naive timestamps in
_row_to_record’s_parse_dtto UTC-aware datetimes.In
lib/crewai/src/crewai/memory/storage/lancedb_storage.py(lines 268-275),_parse_dtreturns parsed datetimes as-is; legacycreated_at/last_accessedstrings saved without an offset produce naivedatetimes. Later code (e.g.,datetime.now(timezone.utc) - record.created_atin scoring) can raiseTypeError: offset-naive and offset-aware. Coercetzinfo is Nonevalues totimezone.utcand normalize aware values to UTC before returning.Suggested fix
def _row_to_record(self, row: dict[str, Any]) -> MemoryRecord: def _parse_dt(val: Any) -> datetime: if val is None: return datetime.now(timezone.utc) - if isinstance(val, datetime): - return val - s = str(val) - return datetime.fromisoformat(s.replace("Z", "+00:00")) + if isinstance(val, datetime): + dt = val + else: + s = str(val) + dt = datetime.fromisoformat(s.replace("Z", "+00:00")) + if dt.tzinfo is None: + return dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc)🤖 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/src/crewai/memory/storage/lancedb_storage.py` around lines 268 - 275, In _row_to_record’s helper _parse_dt, ensure any parsed datetime is timezone-aware in UTC: if val is None return datetime.now(timezone.utc); if val is a datetime and tzinfo is None set tzinfo=timezone.utc; if the datetime is aware convert it to UTC (astimezone(timezone.utc)); when parsing strings, produce an aware UTC datetime (e.g., after fromisoformat ensure tzinfo is not None and normalize to UTC). Update _parse_dt so created_at and last_accessed are always UTC-aware datetimes to avoid offset-naive vs offset-aware errors.
🤖 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.
Outside diff comments:
In `@lib/crewai/src/crewai/memory/storage/lancedb_storage.py`:
- Around line 268-275: In _row_to_record’s helper _parse_dt, ensure any parsed
datetime is timezone-aware in UTC: if val is None return
datetime.now(timezone.utc); if val is a datetime and tzinfo is None set
tzinfo=timezone.utc; if the datetime is aware convert it to UTC
(astimezone(timezone.utc)); when parsing strings, produce an aware UTC datetime
(e.g., after fromisoformat ensure tzinfo is not None and normalize to UTC).
Update _parse_dt so created_at and last_accessed are always UTC-aware datetimes
to avoid offset-naive vs offset-aware errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31135e58-7402-4de0-a5af-32ebce4ab811
📒 Files selected for processing (5)
lib/crewai/src/crewai/memory/encoding_flow.pylib/crewai/src/crewai/memory/storage/lancedb_storage.pylib/crewai/src/crewai/memory/types.pylib/crewai/src/crewai/memory/unified_memory.pylib/crewai/tests/memory/test_unified_memory.py
|
Thanks for the review. I addressed the timestamp normalization issue by coercing legacy naive datetimes to UTC and normalizing aware datetimes back to UTC before building MemoryRecords. I also added a regression test covering both an offset-less legacy timestamp and an offset-aware timestamp.\n\nLocal verification: python -m compileall -q lib/crewai/src/crewai/memory/storage/lancedb_storage.py lib/crewai/tests/memory/test_unified_memory.py and git diff --check pass. I was not able to run the focused pytest locally because this lightweight checkout/environment is missing project test dependencies (json_repair after disabling unavailable default pytest plugins), but the test is included for CI to run in the normal repo environment. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/memory/storage/lancedb_storage.py (1)
535-546:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
created_atinget_scope_info()before comparison.
get_scope_info()can compare naive and aware datetimes directly (Line 543/Line 545), which will raiseTypeErroron mixed legacy/new rows. Reuse the same UTC-normalization path used by_row_to_recordbefore min/max comparisons.Suggested fix
@@ - created = row.get("created_at") - if created: - dt = ( - datetime.fromisoformat(str(created).replace("Z", "+00:00")) - if isinstance(created, str) - else created - ) - if isinstance(dt, datetime): - if oldest is None or dt < oldest: - oldest = dt - if newest is None or dt > newest: - newest = dt + created = row.get("created_at") + if created: + if isinstance(created, datetime): + dt = created + else: + dt = datetime.fromisoformat(str(created).replace("Z", "+00:00")) + dt = dt.replace(tzinfo=timezone.utc) if dt.tzinfo is None else dt.astimezone(timezone.utc) + if oldest is None or dt < oldest: + oldest = dt + if newest is None or dt > newest: + newest = dt🤖 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/src/crewai/memory/storage/lancedb_storage.py` around lines 535 - 546, Normalize the created_at value in get_scope_info() before comparing to oldest/newest: reuse the same UTC-normalization logic used in _row_to_record by parsing string timestamps the same way and converting any naive datetime to an aware UTC datetime (so all datetimes are timezone-aware) before doing the min/max comparisons; update the handling of the local variable created (and dt) so oldest and newest are compared against consistently UTC-aware datetimes instead of mixing naive and aware datetimes.
🤖 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/tests/memory/test_unified_memory.py`:
- Around line 147-148: The test currently asserts equality of record.created_at
and record.last_accessed but doesn't ensure their tzinfo is normalized to
timezone.utc; update the normalization test to explicitly assert that
record.created_at.tzinfo is timezone.utc and that record.last_accessed.tzinfo is
timezone.utc (referencing the variables record.created_at and
record.last_accessed in the test_unified_memory.py test) so the contract
enforces UTC-aware datetimes.
---
Outside diff comments:
In `@lib/crewai/src/crewai/memory/storage/lancedb_storage.py`:
- Around line 535-546: Normalize the created_at value in get_scope_info() before
comparing to oldest/newest: reuse the same UTC-normalization logic used in
_row_to_record by parsing string timestamps the same way and converting any
naive datetime to an aware UTC datetime (so all datetimes are timezone-aware)
before doing the min/max comparisons; update the handling of the local variable
created (and dt) so oldest and newest are compared against consistently
UTC-aware datetimes instead of mixing naive and aware datetimes.
🪄 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: 56056237-f6d0-4f44-a0d5-9435243e3e5e
📒 Files selected for processing (2)
lib/crewai/src/crewai/memory/storage/lancedb_storage.pylib/crewai/tests/memory/test_unified_memory.py
| assert record.created_at == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) | ||
| assert record.last_accessed == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) |
There was a problem hiding this comment.
Assert tzinfo explicitly in normalization test.
Current equality checks can still pass when the value is offset-aware but not normalized to timezone.utc. Add direct tzinfo assertions to enforce the contract.
Suggested fix
assert record.created_at == datetime(2026, 1, 1, 12, tzinfo=timezone.utc)
assert record.last_accessed == datetime(2026, 1, 1, 12, tzinfo=timezone.utc)
+ assert record.created_at.tzinfo == timezone.utc
+ assert record.last_accessed.tzinfo == timezone.utc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert record.created_at == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) | |
| assert record.last_accessed == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) | |
| assert record.created_at == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) | |
| assert record.last_accessed == datetime(2026, 1, 1, 12, tzinfo=timezone.utc) | |
| assert record.created_at.tzinfo == timezone.utc | |
| assert record.last_accessed.tzinfo == timezone.utc |
🤖 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/tests/memory/test_unified_memory.py` around lines 147 - 148, The
test currently asserts equality of record.created_at and record.last_accessed
but doesn't ensure their tzinfo is normalized to timezone.utc; update the
normalization test to explicitly assert that record.created_at.tzinfo is
timezone.utc and that record.last_accessed.tzinfo is timezone.utc (referencing
the variables record.created_at and record.last_accessed in the
test_unified_memory.py test) so the contract enforces UTC-aware datetimes.
1 participant
Summary
Replace all
datetime.utcnow()calls withdatetime.now(timezone.utc)in the memory module to resolve the Python 3.12 deprecation warning.Problem
datetime.utcnow()has been deprecated since Python 3.12 because it returns a naivedatetimeobject that does not carry timezone information, making it ambiguous and error-prone when comparing with timezone-aware timestamps.Before (deprecated):
After (recommended):
Changes
types.pydefault_factoryforcreated_atandlast_accessedfields; updatedcompute_composite_score()lancedb_storage.pysave(),_now(), andtouch_records()encoding_flow.pyexecute_plans()unified_memory.pyupdate()test_unified_memory.pydatetime.now(timezone.utc)Verification
datetime.now(timezone.utc)returns the exact same instant asdatetime.utcnow(), but as a timezone-aware objectfrom datetime import datetimetofrom datetime import datetime, timezonein all affected filesutcnowcalls in the memory moduleSummary by CodeRabbit