Skip to content

fix(database): use naive datetime to match DB column type - #4

Merged
Jonathan0823 merged 1 commit into
mainfrom
fix/timestamp-timezone-mismatch
Apr 16, 2026
Merged

Jonathan0823 merged 1 commit into
mainfrom
fix/timestamp-timezone-mismatch

Conversation

@Jonathan0823

@Jonathan0823 Jonathan0823 commented Apr 16, 2026 •

Copy link
Copy Markdown
Owner

Related issue

  • None

Summary

  • Changed datetime.now(timezone.utc) to datetime.now() in database models and CRUD operations
  • Fixes asyncpg error: "can't subtract offset-naive and offset-aware datetimes" when creating polling targets

Root Cause

  • Database columns use TIMESTAMP WITHOUT TIME ZONE (naive)
  • App code was using timezone-aware datetimes (datetime.now(timezone.utc))
  • asyncpg rejected binding aware datetime into naive timestamp column

Scope

  • In scope: app/database/models.py, app/database/crud.py
  • Out of scope: style changes in metrics.py, modbus_client.py, new docs/docker-compose.yml

Testing

  • Created polling target via direct database call
    • Command: create_polling_target(session, target) with UTC-aware datetime
    • Result: FAILED before fix
  • Created polling target after fix
    • Command: create_polling_target(session, target) with naive datetime
    • Result: SUCCESS - created_at=2026-04-16 08:11:11.544190, tzinfo=None
  • Verified DB stores naive timestamps correctly

Reviewer Focus

  • Please review: app/database/crud.py, app/database/models.py
  • Risk areas: None - simple one-line change from aware to naive datetime
  • No migrations needed - existing DB data unaffected (already stored as naive)

Notes

  • Migrations: none required
  • Backward compatible: yes - existing data already stored as naive timestamps

Summary by CodeRabbit

  • Chores
    • Updated timestamp handling for device and polling target records in the database.

Changed datetime.now(timezone.utc) to datetime.now() in models and CRUD
to resolve asyncpg error: can't subtract offset-naive and offset-aware datetimes

The DB columns are TIMESTAMP WITHOUT TIME ZONE, so the app must use naive
datetimes to be compatible.
@coderabbitai

coderabbitai Bot commented Apr 16, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The changes switch from UTC-aware datetimes (datetime.now(timezone.utc)) to system-local/naive datetimes (datetime.now()) across database models and CRUD operations for device and polling target management. This affects timestamp defaults in ModbusDevice and PollingTarget models, as well as update operations in CRUD functions.

Changes

Cohort / File(s) Summary
Datetime Handling Migration
app/database/models.py, app/database/crud.py
Replaced datetime.now(timezone.utc) with datetime.now() in timestamp defaults for ModbusDevice and PollingTarget models, and in CRUD operations (update_device, delete_device, activate_device, update_polling_target, delete_polling_target, activate_polling_target). Removed unused timezone import from datetime module.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hopping through the code with glee,
From UTC to local spree,
No more timezones to confuse,
Just datetime.now() we choose!
The rabbit's work is complete,
Simpler timestamps, oh what a treat! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: switching from timezone-aware to naive datetimes to match the database column type.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/timestamp-timezone-mismatch

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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/database/crud.py`:
- Line 84: Add a new helper function utc_now_naive() in app/database/models.py
that returns datetime.now(timezone.utc).replace(tzinfo=None), export it, then
update the created_at and updated_at Column defaults in models.py to use
utc_now_naive; next, import utc_now_naive into app/database/crud.py and replace
all usages of datetime.now() (specifically the assignments to device.updated_at
and the five other assignments referenced) with utc_now_naive() so both model
defaults and the six CRUD updates consistently use UTC-naive timestamps.

In `@app/database/models.py`:
- Around line 84-85: The created_at and updated_at Field default_factories use
datetime.now() which yields local-naive timestamps; replace them to use a
UTC-naive helper (e.g., utc_now()) that returns
datetime.now(timezone.utc).replace(tzinfo=None) and update the Field defaults
for created_at and updated_at to call that helper; also replace the six similar
occurrences in app/database/crud.py to use the same utc_now() helper so all
stored datetimes are UTC-naive and consistent across deployments.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9da8e29-0330-4098-8b08-667c0a849479

📥 Commits

Reviewing files that changed from the base of the PR and between cf8c01c and 0afd866.

📒 Files selected for processing (2)
  • app/database/crud.py
  • app/database/models.py

Comment thread app/database/crud.py
setattr(device, key, value)

device.updated_at = datetime.now(timezone.utc)
device.updated_at = datetime.now()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find app -type f -name "*.py" | head -20

Repository: Jonathan0823/nexusbus

Length of output: 519


🏁 Script executed:

ls -la app/database/ 2>/dev/null || echo "Directory not found"

Repository: Jonathan0823/nexusbus

Length of output: 416


🏁 Script executed:

head -30 app/database/models.py 2>/dev/null || echo "File not found"

Repository: Jonathan0823/nexusbus

Length of output: 973


🏁 Script executed:

wc -l app/database/crud.py 2>/dev/null || echo "File not found"

Repository: Jonathan0823/nexusbus

Length of output: 89


🏁 Script executed:

cat app/database/models.py

Repository: Jonathan0823/nexusbus

Length of output: 6901


🏁 Script executed:

cat app/database/crud.py

Repository: Jonathan0823/nexusbus

Length of output: 10021


🏁 Script executed:

rg -i "utc_now" app/ 2>/dev/null || echo "No utc_now functions found"

Repository: Jonathan0823/nexusbus

Length of output: 91


🏁 Script executed:

rg -i "def.*now" app/database/ 2>/dev/null || echo "No now functions found"

Repository: Jonathan0823/nexusbus

Length of output: 448


🏁 Script executed:

rg "timezone" app/database/ -A 2 -B 2 2>/dev/null || echo "No timezone usage found"

Repository: Jonathan0823/nexusbus

Length of output: 291


The suggested helper function utc_now_naive() does not exist in the codebase and must be created first.

The concern about datetime.now() using server-local time is valid and needs fixing. However, the suggested fix is incomplete—it references a helper that hasn't been defined yet and doesn't address the same issue in app/database/models.py where created_at and updated_at defaults also use datetime.now().

To fix this properly:

  1. Add a utc_now_naive() helper to app/database/models.py that returns datetime.now(timezone.utc).replace(tzinfo=None) (or equivalent)
  2. Update both models.py defaults and all six crud.py assignments (lines 84, 114, 142, 254, 284, 312) to use this helper
  3. Ensure the helper is exported and imported consistently
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/database/crud.py` at line 84, Add a new helper function utc_now_naive()
in app/database/models.py that returns
datetime.now(timezone.utc).replace(tzinfo=None), export it, then update the
created_at and updated_at Column defaults in models.py to use utc_now_naive;
next, import utc_now_naive into app/database/crud.py and replace all usages of
datetime.now() (specifically the assignments to device.updated_at and the five
other assignments referenced) with utc_now_naive() so both model defaults and
the six CRUD updates consistently use UTC-naive timestamps.

Comment thread app/database/models.py
Comment on lines +84 to +85
created_at: datetime = Field(default_factory=lambda: datetime.now())
updated_at: datetime = Field(default_factory=lambda: datetime.now())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check the file exists and read the relevant lines with context
cat -n app/database/models.py | head -100

Repository: Jonathan0823/nexusbus

Length of output: 4110


🏁 Script executed:

# Check git history for changes to these lines
git log --oneline -n 20 -- app/database/models.py

Repository: Jonathan0823/nexusbus

Length of output: 130


🏁 Script executed:

# Get the diff of the most recent changes to see what changed
git diff HEAD~1 HEAD -- app/database/models.py || git show HEAD:app/database/models.py | head -200

Repository: Jonathan0823/nexusbus

Length of output: 6897


🏁 Script executed:

# Search for database column definitions and response models
rg -n "TIMESTAMP|WITHOUT TIME ZONE|ModbusDeviceResponse|PollingTargetResponse" app/database/

Repository: Jonathan0823/nexusbus

Length of output: 195


🏁 Script executed:

# Check the previous commit to see what the original implementation was
git show 0afd866 | head -150

Repository: Jonathan0823/nexusbus

Length of output: 3409


🏁 Script executed:

# Look for database schema/migrations to understand column types
find . -type f -name "*.sql" -o -name "*migration*" -o -name "*schema*" | head -20

Repository: Jonathan0823/nexusbus

Length of output: 94


🏁 Script executed:

# Check if there are any alembic migrations or database config
fd -e py -e sql | grep -E "(migration|alembic|schema)" | head -20

Repository: Jonathan0823/nexusbus

Length of output: 231


🏁 Script executed:

# Search for where created_at/updated_at might be used to understand impact
rg "created_at|updated_at" -B 2 -A 2 app/ --max-count=30

Repository: Jonathan0823/nexusbus

Length of output: 2760


🏁 Script executed:

# Check the migration files to understand database column definitions
cat migrations/001_initial_setup.py | head -100

Repository: Jonathan0823/nexusbus

Length of output: 2429


🏁 Script executed:

# Check the CRUD operations to see how updated_at is being used
cat app/database/crud.py | head -80

Repository: Jonathan0823/nexusbus

Length of output: 2426


🏁 Script executed:

# Check if there's any documentation about timestamp expectations
find . -type f -name "*.md" | xargs grep -l "timestamp\|timezone\|utc" 2>/dev/null | head -5

Repository: Jonathan0823/nexusbus

Length of output: 209


🏁 Script executed:

# Look for any tests that might clarify expected behavior
find tests -name "*.py" 2>/dev/null | xargs grep -l "created_at\|updated_at" 2>/dev/null | head -3

Repository: Jonathan0823/nexusbus

Length of output: 47


🏁 Script executed:

# Search for the database schema definition to confirm TIMESTAMP WITHOUT TIME ZONE
rg "TIMESTAMP" . --type sql --type py

Repository: Jonathan0823/nexusbus

Length of output: 47


🏁 Script executed:

# Check if there's any existing helper function for datetime handling
rg "utc|UTC|timezone" app/database/ -B 2 -A 2

Repository: Jonathan0823/nexusbus

Length of output: 291


🏁 Script executed:

# Verify the imports at the top of models.py to understand what's available
head -10 app/database/models.py

Repository: Jonathan0823/nexusbus

Length of output: 295


🏁 Script executed:

# Check app/database/crud.py to see all uses of datetime.now() that were changed
rg "datetime\.now\(\)" app/database/crud.py -B 2 -A 2

Repository: Jonathan0823/nexusbus

Length of output: 899


🏁 Script executed:

# Look at the full git commit to see the exact change from UTC to local time
git show 0afd866 app/database/models.py | head -80

Repository: Jonathan0823/nexusbus

Length of output: 2719


🏁 Script executed:

# Check if there are any tests that verify timestamp behavior
find . -name "*.py" -path "*/test*" -exec grep -l "datetime\|timestamp" {} \;

Repository: Jonathan0823/nexusbus

Length of output: 91


🏁 Script executed:

# Verify the database schema files to understand TIMESTAMP WITHOUT TIME ZONE usage
find . -name "*.py" -path "*migration*" -exec grep -l "TIMESTAMP" {} \;

Repository: Jonathan0823/nexusbus

Length of output: 47


🏁 Script executed:

# Check the test file that references datetime
cat tests/unit/test_cache.py

Repository: Jonathan0823/nexusbus

Length of output: 6101


🏁 Script executed:

# Look for any datetime serialization or comparison in the codebase
rg "strftime|isoformat|fromisoformat" app/ --type py

Repository: Jonathan0823/nexusbus

Length of output: 654


🏁 Script executed:

# Check if there's any timezone-related configuration in the app
rg "TZ|timezone|UTC|utc" app/ --type py -B 1 -A 1 | head -50

Repository: Jonathan0823/nexusbus

Length of output: 2367


🏁 Script executed:

# Check if there are any other places where datetime is created for the database
rg "datetime\.now\(\)" app/ --type py -B 2 -A 2

Repository: Jonathan0823/nexusbus

Length of output: 2328


🏁 Script executed:

# Verify the database schema definition in migrations
cat migrations/001_initial_setup.py | grep -A 50 "modbus_devices"

Repository: Jonathan0823/nexusbus

Length of output: 2042


🏁 Script executed:

# Check cache.py to see how it properly uses UTC
cat app/core/cache.py | head -50

Repository: Jonathan0823/nexusbus

Length of output: 1570


🏁 Script executed:

# Verify the actual database schema to confirm TIMESTAMP WITHOUT TIME ZONE
cat migrations/base.py | head -80

Repository: Jonathan0823/nexusbus

Length of output: 2503


🏁 Script executed:

# Check if there's any SQLAlchemy type definition for the datetime columns
rg "DateTime\|TIMESTAMP" migrations/ app/ --type py -B 2 -A 2

Repository: Jonathan0823/nexusbus

Length of output: 47


🏁 Script executed:

# Check the complete CRUD file to understand the full context
wc -l app/database/crud.py

Repository: Jonathan0823/nexusbus

Length of output: 89


🏁 Script executed:

# Search for any datetime comparisons that might be causing the asyncpg error
rg "created_at|updated_at" app/database/crud.py -B 3 -A 3

Repository: Jonathan0823/nexusbus

Length of output: 1275


🏁 Script executed:

# Get a clearer picture: count total occurrences of datetime.now() without timezone in database code
rg "datetime\.now\(\)" app/database/ --type py | wc -l

Repository: Jonathan0823/nexusbus

Length of output: 67


🏁 Script executed:

# Double-check: are there any database schema definitions showing TIMESTAMP WITHOUT TIME ZONE explicitly?
find . -type f \( -name "*.sql" -o -name "*.py" \) -exec grep -l "TIMESTAMP WITHOUT TIME ZONE\|without time zone" {} \;

Repository: Jonathan0823/nexusbus

Length of output: 47


Use UTC-naive timestamps, not local-naive, to preserve correct timestamp semantics.

datetime.now() creates local-wall-clock naive datetimes, which means on a non-UTC deployment (e.g., Asia/Bangkok +07:00), created_at and updated_at will be stored as local time instead of UTC—changing what the values semantically represent. The DB requires naive datetimes, but that doesn't mean you should use local time. Extract a UTC helper that returns datetime.now(timezone.utc).replace(tzinfo=None) so stored timestamps remain consistent across deployments and DST changes.

Suggested fix
 from datetime import datetime, timezone
 from typing import Optional, Tuple
 
 VALID_FRAMERS: Tuple[str, ...] = ("RTU", "SOCKET", "ASCII")
 VALID_REGISTER_TYPES: Tuple[str, ...] = ("holding", "input", "coil", "discrete")
+
+
+def utc_now_naive() -> datetime:
+    """Return the current UTC time without timezone info for TIMESTAMP WITHOUT TIME ZONE columns."""
+    return datetime.now(timezone.utc).replace(tzinfo=None)
 
 
 class ModbusDevice(SQLModel, table=True):
     ...
-    created_at: datetime = Field(default_factory=lambda: datetime.now())
-    updated_at: datetime = Field(default_factory=lambda: datetime.now())
+    created_at: datetime = Field(default_factory=utc_now_naive)
+    updated_at: datetime = Field(default_factory=utc_now_naive)
 
 
 class PollingTarget(SQLModel, table=True):
     ...
-    created_at: datetime = Field(default_factory=lambda: datetime.now())
-    updated_at: datetime = Field(default_factory=lambda: datetime.now())
+    created_at: datetime = Field(default_factory=utc_now_naive)
+    updated_at: datetime = Field(default_factory=utc_now_naive)

Also update app/database/crud.py (6 occurrences):

-device.updated_at = datetime.now()
+device.updated_at = utc_now_naive()
📝 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.

Suggested change
created_at: datetime = Field(default_factory=lambda: datetime.now())
updated_at: datetime = Field(default_factory=lambda: datetime.now())
created_at: datetime = Field(default_factory=utc_now_naive)
updated_at: datetime = Field(default_factory=utc_now_naive)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/database/models.py` around lines 84 - 85, The created_at and updated_at
Field default_factories use datetime.now() which yields local-naive timestamps;
replace them to use a UTC-naive helper (e.g., utc_now()) that returns
datetime.now(timezone.utc).replace(tzinfo=None) and update the Field defaults
for created_at and updated_at to call that helper; also replace the six similar
occurrences in app/database/crud.py to use the same utc_now() helper so all
stored datetimes are UTC-naive and consistent across deployments.

@Jonathan0823
Jonathan0823 merged commit 0cded64 into main Apr 16, 2026
3 checks passed
@Jonathan0823
Jonathan0823 deleted the fix/timestamp-timezone-mismatch branch May 11, 2026 09:24
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