fix: use timezone-aware datetimes (fixes #34)#37
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR configures all SQLAlchemy DateTime columns to be timezone-aware by specifying timezone=True, ensuring consistent UTC timestamp handling across supported databases, and updates the package version to 0.3.3. Entity Relationship diagram for updated DateTime columns with timezone awarenesserDiagram
CapturedRequest {
Float duration_ms
String client_ip
DateTime created_at_timezone_aware
}
CapturedQuery {
Integer rows_affected
String connection_name
DateTime created_at_timezone_aware
}
CapturedException {
Text exception_value
Text traceback
DateTime created_at_timezone_aware
}
Trace {
String service_name
String operation_name
DateTime start_time_timezone_aware
DateTime end_time_timezone_aware
Float duration_ms
Integer span_count
String status
JSON tags
DateTime created_at_timezone_aware
}
Span {
String operation_name
String service_name
String span_kind
DateTime start_time_timezone_aware
DateTime end_time_timezone_aware
Float duration_ms
String status
JSON tags
JSON logs
DateTime created_at_timezone_aware
}
SpanRelation {
String parent_span_id
String child_span_id
Integer depth
DateTime created_at_timezone_aware
}
BackgroundTask {
String status
DateTime start_time_timezone_aware
DateTime end_time_timezone_aware
Float duration_ms
Text error
DateTime created_at_timezone_aware
}
Class diagram for updated models with timezone-aware DateTime columnsclassDiagram
class CapturedRequest {
+duration_ms: float
+client_ip: str
+created_at: DateTime(timezone=True)
}
class CapturedQuery {
+rows_affected: int
+connection_name: str
+created_at: DateTime(timezone=True)
}
class CapturedException {
+exception_value: str
+traceback: str
+created_at: DateTime(timezone=True)
}
class Trace {
+service_name: str
+operation_name: str
+start_time: DateTime(timezone=True)
+end_time: DateTime(timezone=True)
+duration_ms: float
+span_count: int
+status: str
+tags: dict
+created_at: DateTime(timezone=True)
}
class Span {
+operation_name: str
+service_name: str
+span_kind: str
+start_time: DateTime(timezone=True)
+end_time: DateTime(timezone=True)
+duration_ms: float
+status: str
+tags: dict
+logs: dict
+created_at: DateTime(timezone=True)
}
class SpanRelation {
+parent_span_id: str
+child_span_id: str
+depth: int
+created_at: DateTime(timezone=True)
}
class BackgroundTask {
+status: str
+start_time: DateTime(timezone=True)
+end_time: DateTime(timezone=True)
+duration_ms: float
+error: str
+created_at: DateTime(timezone=True)
}
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="fastapi_radar/models.py">
<violation number="1" location="fastapi_radar/models.py:44">
The change to `DateTime(timezone=True)` does not work for the MySQL backend, contrary to the PR description. The SQLAlchemy documentation confirms the `timezone` flag is ignored by the MySQL dialect, meaning the original bug of storing naive datetimes will persist on MySQL deployments.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| client_ip = Column(String(50)) | ||
| created_at = Column( | ||
| DateTime, default=lambda: datetime.now(timezone.utc), index=True | ||
| DateTime(timezone=True), default=lambda: datetime.now(timezone.utc), index=True |
There was a problem hiding this comment.
The change to DateTime(timezone=True) does not work for the MySQL backend, contrary to the PR description. The SQLAlchemy documentation confirms the timezone flag is ignored by the MySQL dialect, meaning the original bug of storing naive datetimes will persist on MySQL deployments.
Prompt for AI agents
Address the following comment on fastapi_radar/models.py at line 44:
<comment>The change to `DateTime(timezone=True)` does not work for the MySQL backend, contrary to the PR description. The SQLAlchemy documentation confirms the `timezone` flag is ignored by the MySQL dialect, meaning the original bug of storing naive datetimes will persist on MySQL deployments.</comment>
<file context>
@@ -41,7 +41,7 @@ class CapturedRequest(Base):
client_ip = Column(String(50))
created_at = Column(
- DateTime, default=lambda: datetime.now(timezone.utc), index=True
+ DateTime(timezone=True), default=lambda: datetime.now(timezone.utc), index=True
)
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #34 - All DateTime columns now use timezone-aware datetimes to ensure correct timestamp handling across different timezones.
Changes
DateTime(timezone=True)datetime.now(timezone.utc)for UTC timestampsFix Details
The issue was that while Python code was creating timezone-aware datetime objects using
datetime.now(timezone.utc), SQLAlchemy's DateTime columns were not configured to preserve timezone information in the database. This caused timestamps to be stored as naive datetimes, leading to incorrect time display.By adding
timezone=Trueto all DateTime columns:Test plan
Breaking Changes
None - this is backward compatible as the change affects how data is stored, not the API.
Summary by Sourcery
Use timezone-aware datetimes in all SQLAlchemy models to fix issues with naive timestamps
Bug Fixes:
Build:
Summary by cubic
Make all database timestamps timezone-aware to fix incorrect time display across timezones (fixes #34). Set SQLAlchemy DateTime columns to DateTime(timezone=True) across models, keep UTC via datetime.now(timezone.utc), and bump the package version to 0.3.3.
Written for commit 1a6ba7a. Summary will update automatically on new commits.