Skip to content

fix: add UTC timezone suffix to datetime serializers#12995

Open
luke-mino-altherr wants to merge 1 commit intoComfy-Org:masterfrom
luke-mino-altherr:luke-mino-altherr/unify-timestamps-task3
Open

fix: add UTC timezone suffix to datetime serializers#12995
luke-mino-altherr wants to merge 1 commit intoComfy-Org:masterfrom
luke-mino-altherr:luke-mino-altherr/unify-timestamps-task3

Conversation

@luke-mino-altherr
Copy link
Contributor

Datetime fields in asset API responses are serialized via .isoformat() without timezone info, producing strings like "2026-03-16T10:30:00.000000". Per the ECMAScript spec, new Date() parses these as local time, causing offset errors when the browser timezone ≠ UTC.

Changes:

  • get_utc_now() now returns timezone-aware datetime (also the Python 3.12+ recommended replacement for deprecated datetime.utcnow())
  • All field_serializer methods in schemas_out.py attach +00:00 to legacy naive datetimes
  • to_dict() in models.py applies the same pattern

Output changes from "2026-03-16T10:30:00.000000" to "2026-03-16T10:30:00+00:00" which is valid RFC 3339 and parsed correctly by JavaScript.

@luke-mino-altherr
Copy link
Contributor Author

Part of the Unify Timestamps effort. Related PRs:

@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39690a28-e608-4340-b320-c44803a13980

📥 Commits

Reviewing files that changed from the base of the PR and between 1eb1bac and 7da9f22.

📒 Files selected for processing (3)
  • app/assets/api/schemas_out.py
  • app/assets/helpers.py
  • app/database/models.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/assets/api/schemas_out.py
  • app/assets/helpers.py

📝 Walkthrough

Walkthrough

The PR adds explicit timezone handling: API schema serializers and the database model assign UTC (timezone.utc) to datetime values that lack tzinfo before calling isoformat(). The helper docstring was updated to clarify usage for DB columns declared with timezone=False; its implementation remains unchanged (it still returns a UTC time with tzinfo stripped). No public/exported function or class signatures were changed.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding UTC timezone suffix to datetime serializers to fix JavaScript parsing issues.
Description check ✅ Passed The description clearly explains the problem (naive datetime strings parsed as local time by JavaScript), the solution (adding UTC timezone info), and the expected output format change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
app/assets/api/schemas_out.py (1)

21-27: LGTM with optional DRY suggestion.

The serialization logic is correct. Consider extracting the repeated pattern into a module-level helper to reduce duplication across the three serializer methods.

,

♻️ Optional: Extract shared helper
def _serialize_dt(v: datetime | None) -> str | None:
    if v is None:
        return None
    if v.tzinfo is None:
        v = v.replace(tzinfo=timezone.utc)
    return v.isoformat()

Then each serializer becomes:

 `@field_serializer`("created_at", "updated_at", "last_access_time")
 def _serialize_datetime(self, v: datetime | None, _info):
-    if v is None:
-        return None
-    if v.tzinfo is None:
-        v = v.replace(tzinfo=timezone.utc)
-    return v.isoformat()
+    return _serialize_dt(v)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/assets/api/schemas_out.py` around lines 21 - 27, The datetime serializer
_serialize_datetime repeats logic that can be reused; extract a module-level
helper (e.g., _serialize_dt) that accepts datetime | None and returns str |
None, performing the None check, ensuring tzinfo (replace with timezone.utc when
missing) and calling isoformat, then update _serialize_datetime (and any other
serializers for "created_at", "updated_at", "last_access_time") to delegate to
that helper to remove duplication while keeping behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/assets/api/schemas_out.py`:
- Around line 21-27: The datetime serializer _serialize_datetime repeats logic
that can be reused; extract a module-level helper (e.g., _serialize_dt) that
accepts datetime | None and returns str | None, performing the None check,
ensuring tzinfo (replace with timezone.utc when missing) and calling isoformat,
then update _serialize_datetime (and any other serializers for "created_at",
"updated_at", "last_access_time") to delegate to that helper to remove
duplication while keeping behavior identical.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 34f3dfd3-8eba-49b7-9f1d-319a485d4f8c

📥 Commits

Reviewing files that changed from the base of the PR and between 593be20 and 1eb1bac.

📒 Files selected for processing (3)
  • app/assets/api/schemas_out.py
  • app/assets/helpers.py
  • app/database/models.py

@luke-mino-altherr luke-mino-altherr force-pushed the luke-mino-altherr/unify-timestamps-task3 branch from 1eb1bac to 7da9f22 Compare March 16, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant