fix: replaced deprecated datetime.utc() with datetime.now(timezone.utc)#2075
Conversation
Signed-off-by: vchen7629 <chenvincent7629@gmail.com>
github-actions
Bot
added
backend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes replace ChangesTimezone-aware timestamp migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ 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-actions
Bot
added
bug
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/services/knowledge_filter_service.py (1)
354-359: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
timezonefrom local import.The local import
from datetime import datetime, timezoneon line 354 importstimezone, but it is never used —UTCcomes from the module-level import on line 1. This is a newly introduced unused import.♻️ Proposed fix
- from datetime import datetime, timezone + from datetime import datetime🤖 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 `@src/services/knowledge_filter_service.py` around lines 354 - 359, Remove the unused timezone symbol from the local datetime import in the update body logic, retaining only datetime; continue using the module-level UTC referenced by the relevant subscription update method.src/db/repositories/api_key_repo.py (1)
36-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the inline datetime import to the top level.
UTCis already imported at the top level (line 7), anddatetimeis only needed inrevoke. Movingdatetimeto the top-level import eliminates the inline import and the unusedtimezoneit pulls in.♻️ Proposed refactor
from datetime import UTC +from datetime import UTC, datetimeasync def revoke(self, key_id: str) -> None: - from datetime import datetime, timezone - row = await self.session.get(ApiKey, key_id) if row: row.revoked = True row.revoked_at = datetime.now(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 `@src/db/repositories/api_key_repo.py` around lines 36 - 41, Move the datetime import to the module-level imports in the API key repository, retaining only datetime and removing the inline import of datetime and timezone from revoke. Keep revoke using datetime.now(UTC) when setting revoked_at.
🤖 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 `@alembic/versions/0002_seed_roles_permissions.py`:
- Around line 15-16: Remove the unused timezone and Union imports from the
module’s import section, retaining only the datetime symbols referenced by the
seed logic so Ruff no longer reports F401.
In `@src/api/knowledge_filter.py`:
- Line 3: Remove the unused timezone symbol from the datetime import in the
knowledge filter module, keeping only the imports required by datetime.now(UTC).
In `@src/api/settings/helpers.py`:
- Line 165: Remove the unused timezone symbol from the datetime import in the
settings helper module, leaving only the imported datetime symbol while
retaining use of the module-level UTC constant.
In `@src/connectors/onedrive/connector.py`:
- Line 407: Remove the unused timezone symbol from the datetime import in the
affected module, retaining only the imports used by the connector, including
UTC.
In `@src/connectors/sharepoint/connector.py`:
- Line 442: Remove the unused timezone symbol from the datetime import in the
SharePoint connector, keeping only the datetime components used by the relevant
method, including UTC.
In `@src/db/models/audit_log.py`:
- Line 1: Remove the unused timezone symbol from the datetime import in the
audit log module, keeping only the datetime symbols actually referenced,
including UTC.
- Around line 12-24: Update the timestamp field `ts` in the audit log model and
all other timestamp model fields introduced in this PR to use timezone-aware
database columns by configuring their SQLAlchemy `DateTime` type with
`timezone=True`, while retaining the UTC-aware default factory such as
`datetime.now(UTC)`.
---
Nitpick comments:
In `@src/db/repositories/api_key_repo.py`:
- Around line 36-41: Move the datetime import to the module-level imports in the
API key repository, retaining only datetime and removing the inline import of
datetime and timezone from revoke. Keep revoke using datetime.now(UTC) when
setting revoked_at.
In `@src/services/knowledge_filter_service.py`:
- Around line 354-359: Remove the unused timezone symbol from the local datetime
import in the update body logic, retaining only datetime; continue using the
module-level UTC referenced by the relevant subscription update method.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34e5d5a2-c870-4837-baba-f5fed2b49f16
📒 Files selected for processing (23)
alembic/versions/0002_seed_roles_permissions.pysrc/api/knowledge_filter.pysrc/api/settings/helpers.pysrc/connectors/onedrive/connector.pysrc/connectors/sharepoint/connector.pysrc/db/migrations_runtime.pysrc/db/models/api_key.pysrc/db/models/audit_log.pysrc/db/models/conversation.pysrc/db/models/migration_status.pysrc/db/models/role.pysrc/db/models/session_ownership.pysrc/db/models/user.pysrc/db/models/user_role.pysrc/db/models/workspace_config.pysrc/db/repositories/api_key_repo.pysrc/db/repositories/conversation_repo.pysrc/db/repositories/workspace_config_repo.pysrc/services/api_key_service.pysrc/services/auth_service.pysrc/services/knowledge_filter_service.pysrc/services/session_ownership_service.pysrc/session_manager.py
| ts: datetime = Field(default_factory=lambda: datetime.now(UTC), index=True) | ||
| actor_user_id: str | None = Field( | ||
| default=None, foreign_key="users.id", max_length=64, index=True | ||
| ) | ||
| actor_api_key_id: Optional[str] = Field(default=None, max_length=64) | ||
| actor_api_key_id: str | None = Field(default=None, max_length=64) | ||
| event: str = Field(max_length=128, index=True) | ||
| target_type: Optional[str] = Field(default=None, max_length=64) | ||
| target_id: Optional[str] = Field(default=None, max_length=128) | ||
| audit_metadata: Optional[dict] = Field( | ||
| target_type: str | None = Field(default=None, max_length=64) | ||
| target_id: str | None = Field(default=None, max_length=128) | ||
| audit_metadata: dict | None = Field( | ||
| default=None, sa_column=Column("metadata", JSON, nullable=True) | ||
| ) | ||
| ip: Optional[str] = Field(default=None, max_length=64) | ||
| user_agent: Optional[str] = Field(default=None, max_length=512) | ||
| ip: str | None = Field(default=None, max_length=64) | ||
| user_agent: str | None = Field(default=None, max_length=512) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for timezone=True in model definitions
rg -n 'timezone.*True' src/db/models/ --type=py
# Check for custom SQLAlchemy DateTime types
rg -n 'class.*DateTime' src/db/ --type=py
# Check for database URL configuration
rg -n 'DATABASE_URL|database_url|db_url' src/config/ --type=py
# Check if timezone is used beyond imports in all changed files
for f in src/db/models/audit_log.py src/db/models/conversation.py src/db/models/role.py src/db/models/workspace_config.py src/db/repositories/api_key_repo.py src/db/repositories/conversation_repo.py src/db/repositories/workspace_config_repo.py; do
echo "=== $f ==="
rg -n 'timezone' "$f" | rg -v 'from datetime import'
doneRepository: langflow-ai/openrag
Length of output: 448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- audit_log.py outline ---'
ast-grep outline src/db/models/audit_log.py --view expanded || true
echo '--- audit_log.py lines ---'
nl -ba src/db/models/audit_log.py | sed -n '1,220p'
echo '--- other model files with timestamp fields ---'
for f in src/db/models/conversation.py src/db/models/role.py src/db/models/workspace_config.py; do
echo "=== $f ==="
nl -ba "$f" | sed -n '1,220p'
done
echo '--- search for DateTime/Timezone column declarations ---'
rg -n 'DateTime|timezone\s*=' src -g '*.py' || true
echo '--- search for engine/backend config ---'
rg -n 'create_engine|sqlite|postgres|postgresql|mysql|DATABASE_URL|database_url|db_url|engine' src -g '*.py' || trueRepository: langflow-ai/openrag
Length of output: 960
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- audit_log.py ---'
sed -n '1,220p' src/db/models/audit_log.py
echo '--- conversation.py ---'
sed -n '1,220p' src/db/models/conversation.py
echo '--- role.py ---'
sed -n '1,220p' src/db/models/role.py
echo '--- workspace_config.py ---'
sed -n '1,260p' src/db/models/workspace_config.py
echo '--- explicit DateTime/timezone usage in src ---'
rg -n 'DateTime|timezone\s*=' src -g '*.py' || true
echo '--- DB/backend config mentions ---'
rg -n 'DATABASE_URL|database_url|db_url|sqlite|postgresql|postgres|mysql|create_engine|engine' src -g '*.py' || trueRepository: langflow-ai/openrag
Length of output: 12999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- timestamp field usages ---'
rg -n '\b(ts|created_at|last_activity|updated_at)\b' src -g '*.py' || true
echo '--- datetime comparisons / UTC now usage ---'
rg -n 'datetime\.now\(UTC\)|UTC\)|<|>|<=|>=|replace\(tzinfo|astimezone|timezone' src/db src/services src/app -g '*.py' || true
echo '--- db engine file ---'
sed -n '1,220p' src/db/engine.pyRepository: langflow-ai/openrag
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- timestamp field usages ---'
rg -n '\b(ts|created_at|last_activity|updated_at)\b' src -g '*.py' || true
echo '--- datetime comparisons / UTC now usage ---'
rg -n 'datetime\.now\(UTC\)|astimezone|replace\(tzinfo|timezone' src/db src/services src/app -g '*.py' || true
echo '--- db engine file ---'
sed -n '1,220p' src/db/engine.pyRepository: langflow-ai/openrag
Length of output: 20789
Use timezone-aware columns for the UTC timestamp fields
In src/db/models/audit_log.py and the other timestamp models in this PR, datetime.now(UTC) still maps to the default naive DateTime. Add timezone=True on those DB-backed timestamp columns so rows round-trip with tzinfo intact and don’t diverge on serialization or comparisons.
🤖 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 `@src/db/models/audit_log.py` around lines 12 - 24, Update the timestamp field
`ts` in the audit log model and all other timestamp model fields introduced in
this PR to use timezone-aware database columns by configuring their SQLAlchemy
`DateTime` type with `timezone=True`, while retaining the UTC-aware default
factory such as `datetime.now(UTC)`.
There was a problem hiding this comment.
This might be out of scope for this PR
There was a problem hiding this comment.
@Vchen7629, that's fair — adding timezone=True to all the DB-backed timestamp columns is a broader change than what this PR set out to do (which is fixing the datetime.utcnow() removal in Python 3.13). I'll leave it as-is here.
Would you like me to open a follow-up issue to track adding timezone=True to the timestamp columns, so it doesn't get lost?
Signed-off-by: vchen7629 <chenvincent7629@gmail.com>
Signed-off-by: vchen7629 <chenvincent7629@gmail.com>
github-actions
Bot
added
bug
|
review suggestion. File: src/db/migrations_runtime.py Legacy JSON files may contain naive ISO strings (written by the old datetime.utcnow().isoformat() code before this fix shipped). Parsing them with fromisoformat() and mixing with new aware rows risks a TypeError if anything later compares them. Normalize on parse: def _parse_legacy_dt(value: str | None) -> datetime | None: Then use _parse_legacy_dt(data.get("created_at")) etc. in place of the current inline try/except blocks (lines ~215–262). This makes every value entering the DB tz-aware regardless of when the source JSON was written, closing off the mismatch at the one place legacy data is ingested. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd migrations_runtime.py
sed -n '190,270p' src/db/migrations_runtime.pyLength of output: 3392
Your def _parse_legacy_dt(value: str | None) -> datetime | None:
"""Parse a legacy ISO datetime string, coercing naive values to UTC."""
if not value:
return None
try:
dt = datetime.fromisoformat(value)
except (TypeError, ValueError):
return None
return dt if dt.tzinfo is not None else dt.replace(tzinfo=UTC)And then replace the four blocks, e.g.: created = _parse_legacy_dt(data.get("created_at"))
last = _parse_legacy_dt(data.get("last_accessed"))
...
created = _parse_legacy_dt(meta.get("created_at"))
last = _parse_legacy_dt(meta.get("last_activity"))One small tweak: I'd narrow the caught exception to I'll flag this as a follow-up suggestion on the PR. ✏️ Learnings added
✅ Action performedReview finished.
|
github-actions
Bot
added
bug
github-actions
Bot
added
bug
2 participants
Summary
Fixes #1552 . Replaces all 33 deprecated datetime.utcnow() calls with timezone-aware datetime.now(timezone.utc) across the codebase, fixing a AttributeError at runtime since utcnow() was removed in Python 3.13 which is used in the backend base image.
Changes
Summary by CodeRabbit