Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions app/database/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from __future__ import annotations

from datetime import datetime, timezone
from datetime import datetime
from typing import List, Optional

from sqlalchemy.ext.asyncio import AsyncSession
Expand Down Expand Up @@ -81,7 +81,7 @@ async def update_device(
for key, value in update_data.items():
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.

await session.commit()
await session.refresh(device)
logger.info(
Expand Down Expand Up @@ -111,7 +111,7 @@ async def delete_device(session: AsyncSession, device_id: str) -> bool:
return False

device.is_active = False
device.updated_at = datetime.now(timezone.utc)
device.updated_at = datetime.now()
await session.commit()
logger.info(
"device_deleted",
Expand Down Expand Up @@ -139,7 +139,7 @@ async def activate_device(session: AsyncSession, device_id: str) -> bool:
return False

device.is_active = True
device.updated_at = datetime.now(timezone.utc)
device.updated_at = datetime.now()
await session.commit()
logger.info(
"device_activated",
Expand Down Expand Up @@ -251,7 +251,7 @@ async def update_polling_target(
for key, value in update_data.items():
setattr(target, key, value)

target.updated_at = datetime.now(timezone.utc)
target.updated_at = datetime.now()
await session.commit()
await session.refresh(target)
logger.info(
Expand Down Expand Up @@ -281,7 +281,7 @@ async def delete_polling_target(session: AsyncSession, target_id: int) -> bool:
return False

target.is_active = False
target.updated_at = datetime.now(timezone.utc)
target.updated_at = datetime.now()
await session.commit()
logger.info(
"polling_target_deleted",
Expand Down Expand Up @@ -309,7 +309,7 @@ async def activate_polling_target(session: AsyncSession, target_id: int) -> bool
return False

target.is_active = True
target.updated_at = datetime.now(timezone.utc)
target.updated_at = datetime.now()
await session.commit()
logger.info(
"polling_target_activated",
Expand Down
8 changes: 4 additions & 4 deletions app/database/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ class ModbusDevice(SQLModel, table=True):
max_retries: int = Field(default=5)
retry_delay: float = Field(default=0.1)
is_active: bool = Field(default=True, index=True)
created_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))
updated_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))
created_at: datetime = Field(default_factory=lambda: datetime.now())
updated_at: datetime = Field(default_factory=lambda: datetime.now())
Comment on lines +84 to +85

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.



# Pydantic schemas for API
Expand Down Expand Up @@ -151,8 +151,8 @@ class PollingTarget(SQLModel, table=True):
count: int = Field(default=1, ge=1, le=125)
is_active: bool = Field(default=True, index=True)
description: Optional[str] = Field(default=None, max_length=200)
created_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))
updated_at: datetime = Field(default_factory=lambda: datetime.now(timezone.utc))
created_at: datetime = Field(default_factory=lambda: datetime.now())
updated_at: datetime = Field(default_factory=lambda: datetime.now())


# Pydantic schemas for Polling API
Expand Down
Loading