timestamp cleanup #1508
timestamp cleanup #1508
Conversation
|
@coderabbitai review |
There was a problem hiding this comment.
Pull request overview
This pull request performs a comprehensive refactoring of timestamp handling across the NetAlertX codebase, centralizing all timestamp operations to use UTC and introducing a single source of truth function timeNowUTC(). The PR addresses timezone inconsistencies by migrating from multiple datetime functions (timeNowTZ, timeNowDB, timeNow) to a unified approach.
Changes:
- Refactored
datetime_utils.pywith newtimeNowUTC(as_string=True)function as the single source of truth for current time - Updated 25+ server modules and 40+ plugin files to use the new timestamp function
- Created VERSION-based database migration logic to convert existing local timestamps to UTC
- Added comprehensive test coverage with 25 new tests across two test files
- Updated frontend JavaScript to properly handle UTC timestamps from the database
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/utils/datetime_utils.py | Core refactoring - replaced timeNowTZ/timeNowDB/timeNow with timeNowUTC(), updated format functions to assume UTC DB storage |
| server/db/db_upgrade.py | Added UTC migration logic with VERSION detection and timezone offset calculations |
| server/database.py | Integrated migration call into database initialization |
| test/server/test_datetime_utils.py | New comprehensive unit tests for timeNowUTC() function |
| test/db/test_timestamp_migration.py | New tests for migration logic covering version detection and edge cases |
| test/api_endpoints/*.py | Updated test imports from timeNowDB/timeNowTZ to timeNowUTC |
| server/main.py, api.py, app_state.py, initialise.py, logger.py, scheduler.py | Updated all timestamp calls to use timeNowUTC() |
| server/scan/.py, models/.py, messaging/.py, api_server/.py | Replaced deprecated functions with timeNowUTC() |
| front/plugins/*.py | Updated all 40+ plugin files to use timeNowUTC() |
| front/js/common.js | Enhanced timestamp parsing to explicitly mark DB timestamps as UTC before conversion |
| .github/workflows/run-all-tests.yml | Added test/db/ to backend test suite |
| .github/skills/code-standards/SKILL.md | Updated documentation with UTC timestamp conventions |
| def get_timezone_offset(): | ||
| now = datetime.datetime.now(conf.tz) | ||
| offset_hours = now.utcoffset().total_seconds() / 3600 | ||
| now = timeNowUTC(as_string=False).replace(tzinfo=conf.tz) if conf.tz else timeNowUTC(as_string=False) | ||
| offset_hours = now.utcoffset().total_seconds() / 3600 if now.utcoffset() else 0 | ||
| offset_formatted = "{:+03d}:{:02d}".format(int(offset_hours), int((offset_hours % 1) * 60)) | ||
| return offset_formatted |
There was a problem hiding this comment.
The logic in get_timezone_offset() appears incorrect. The function replaces the tzinfo of a UTC datetime with conf.tz, which doesn't perform a timezone conversion - it just changes the timezone label without adjusting the actual time value. This will give an incorrect offset calculation.
The correct approach should be to convert the UTC time to the target timezone using astimezone(), then calculate the offset. For example:
- Create UTC datetime: timeNowUTC(as_string=False)
- Convert to target timezone: utc_time.astimezone(conf.tz)
- Get offset from the converted datetime
This will properly account for DST and give the correct offset between UTC and the target timezone.
| ensure_Settings(self.sql) | ||
|
|
||
| # Parameters tables setup | ||
| ensure_Parameters(self.sql) | ||
|
|
||
| # One-time UTC timestamp migration (must run after Parameters table exists) | ||
| migrate_timestamps_to_utc(self.sql) |
There was a problem hiding this comment.
Critical ordering issue: The migration function migrate_timestamps_to_utc() is called AFTER ensure_Settings(), which drops and recreates the Settings table (line 383: "DROP TABLE IF EXISTS Settings"). This means the VERSION value that the migration logic depends on to determine if migration is needed will always be empty/missing, causing the migration to be skipped even for upgrades from old versions.
The migration should be called BEFORE ensure_Settings(), or ensure_Settings() should preserve the VERSION value before dropping the table. Otherwise, all upgrades from pre-26.2.6 versions will skip the timestamp migration, leaving timestamps in local time format instead of UTC.
test/server/test_datetime_utils.py
Outdated
| def test_timeNowUTC_datetime_has_UTC_timezone(self): | ||
| """Test that datetime object has UTC timezone""" | ||
| result = timeNowUTC(as_string=False) | ||
| assert result.tzinfo is datetime.UTC or result.tzinfo is not None |
There was a problem hiding this comment.
The assertion assert result.tzinfo is datetime.UTC or result.tzinfo is not None is too weak. According to the function implementation, it should always return a datetime with datetime.UTC timezone, not just "any timezone". The assertion should be assert result.tzinfo is datetime.UTC to properly verify the UTC timezone requirement.
front/js/common.js
Outdated
| let isoStr = str.trim(); | ||
| if (!hasOffset) { | ||
| // Ensure proper ISO format before appending Z | ||
| // Replace space with 'T' if needed: "2026-02-11 11:37:02" → "2026-02-11T11:37:02Z" | ||
| isoStr = isoStr.replace(' ', 'T') + 'Z'; |
There was a problem hiding this comment.
The string replacement .replace(' ', 'T') only replaces the first occurrence of a space. While this should work for well-formed timestamps like "2026-02-11 11:37:02", if there are any leading spaces or malformed inputs with multiple spaces, this could create invalid ISO strings.
Consider using a more robust approach that specifically targets the space between date and time, or trim the string first and then replace. For example: isoStr.trim().replace(/^(\d{4}-\d{2}-\d{2}) (\d{2}:\d{2}:\d{2})$/, '$1T$2') + 'Z' would be more precise.
| """ | ||
| # --- Setup: create two sessions for the test MAC --- | ||
| now = timeNowTZ() | ||
| now = datetime.now() |
There was a problem hiding this comment.
Inconsistent datetime usage in test. This line uses datetime.now() directly instead of timeNowUTC(as_string=False), which contradicts the PR's goal of making timeNowUTC() the single source of truth for current time. This could lead to timezone-related test failures if the test runs in a different timezone than UTC.
Should be changed to: now = timeNowUTC(as_string=False)
| @@ -140,7 +140,7 @@ def test_delete_events_dynamic_days(client, api_token, test_mac): | |||
| # Count pre-existing events younger than 30 days for test_mac | |||
| # These will remain after delete operation | |||
| from datetime import datetime | |||
There was a problem hiding this comment.
Redundant import statement. The datetime module is already imported at the top of the file (line 5). This local import on line 142 is unnecessary and can cause confusion about which datetime is being used.
server/models/event_instance.py
Outdated
| eve_PendingAlertEmail, eve_PairEventRowid | ||
| ) VALUES (?,?,?,?,?,?,?) | ||
| """, (mac, ip, datetime.now(), eventType, info, | ||
| """, (mac, ip, timeNowUTC(as_string=False), eventType, info, |
There was a problem hiding this comment.
Type mismatch in database insertion. The code passes timeNowUTC(as_string=False) which returns a datetime object, but the database column expects a TEXT/string timestamp. According to the PR's pattern, database writes should use timeNowUTC() (or explicitly timeNowUTC(as_string=True)) to get the formatted string.
This should be: timeNowUTC() instead of timeNowUTC(as_string=False)
While SQLite can handle datetime objects and convert them, this breaks the consistency of the refactoring which aims to always use string timestamps for DB storage.
📝 WalkthroughWalkthroughReplaces legacy time helpers with a single UTC utility Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant DB as Database
participant Upgrader as DB Upgrade (migrate_timestamps_to_utc)
participant Config as Config/Timezone
App->>DB: Initialize DB (ensure tables)
DB->>Upgrader: migrate_timestamps_to_utc(sql)
Upgrader->>DB: Read Settings.VERSION
alt VERSION fresh/unknown or >= UTC-aware
Upgrader->>DB: Skip migration (return success)
else Old version detected
Upgrader->>DB: Sample timestamps from Devices
Upgrader->>Config: Read configured timezone (conf.tz)
Upgrader->>Upgrader: Compute local→UTC offset
loop For each table.column needing migration
Upgrader->>DB: Update non-NULL timestamps by adding offset
Upgrader->>Upgrader: Log progress/errors
end
Upgrader->>DB: Return success/failure
end
DB->>App: DB init complete (migration done or skipped)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
server/utils/datetime_utils.py (1)
82-87:⚠️ Potential issue | 🟠 Major
fromtimestamp()uses local timezone — inconsistent with UTC-first approach.
datetime.datetime.fromtimestamp(inputTimeStamp)interprets the epoch in the system's local timezone and returns a naive datetime in local time. In a UTC-first codebase, this is a latent mismatch: callers that later assume the result is UTC (as the rest of the module does for naive datetimes) will get wrong values on non-UTC servers.Suggested fix
if isinstance(inputTimeStamp, (int, float)): try: - return datetime.datetime.fromtimestamp(inputTimeStamp) + return datetime.datetime.fromtimestamp(inputTimeStamp, tz=datetime.UTC) except (OSError, OverflowError, ValueError): return Nonefront/plugins/freebox/freebox.py (1)
170-170:⚠️ Potential issue | 🟠 Major
datetime.fromtimestamp()returns local time, not UTC — inconsistent with the PR's UTC mandate.
datetime.fromtimestamp(ts)interprets the Unix timestamp in the system's local timezone. For UTC-only timestamps, use thetzparameter:🐛 Proposed fix
+import datetime as dt ... - watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0)).strftime(DATETIME_PATTERN), + watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0), tz=dt.timezone.utc).strftime(DATETIME_PATTERN),Or, since the
datetimemodule is already imported at line 7 asfrom datetime import datetime, you can use:- watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0)).strftime(DATETIME_PATTERN), + watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0), tz=__import__('datetime').timezone.utc).strftime(DATETIME_PATTERN),A cleaner approach — adjust the import at line 7:
-from datetime import datetime +import datetimeThen at line 170:
- watched4=datetime.fromtimestamp(ip.get("last_time_reachable", 0)).strftime(DATETIME_PATTERN), + watched4=datetime.datetime.fromtimestamp(ip.get("last_time_reachable", 0), tz=datetime.UTC).strftime(DATETIME_PATTERN),server/models/notification_instance.py (1)
286-296:⚠️ Potential issue | 🟠 MajorRemove
tz_offsetparameter from Device Down alert time comparisons.
datetime('now')in SQLite returns UTC. Sinceeve_DateTimeis stored in UTC (viatimeNowUTC()), applyingtz_offsetincorrectly shifts the comparison threshold. The offset was compensating for local-time storage that no longer applies after the UTC migration.This issue exists in two locations:
server/models/notification_instance.pyline 293server/messaging/reporting.pyline 130Suggested fix for notification_instance.py
- AND eve_DateTime < datetime('now', ?, ?) + AND eve_DateTime < datetime('now', ?) """, - (f"-{minutes} minutes", tz_offset), + (f"-{minutes} minutes",),front/plugins/_publisher_mqtt/mqtt.py (1)
622-637:⚠️ Potential issue | 🟠 MajorBug:
prepTimeStampincorrectly treats naive UTC timestamps as local time.The database stores all timestamps in UTC format (as documented in
timeNowUTC()). WhenprepTimeStampreceives a naive datetime string from the database (lines 507-508 call withdevice["devLastConnection"]anddevice["devFirstConnection"]), line 629 incorrectly appliesconf.tz.localize(), which treats it as if it were in the user's configured timezone rather than UTC. This adds an incorrect timezone offset.For example, a UTC timestamp
2024-01-01 12:00:00becomes EST -05:00 instead of UTC +00:00. The fallback path on line 634 correctly returns a UTC-aware datetime, exposing the inconsistency.Replace line 629 with
parsed_datetime = parsed_datetime.replace(tzinfo=datetime.timezone.utc)and addimport datetime as dt_moduleat the top of the file, or usedatetime.timezone.utcdirectly if importing the module.Proposed fix
+import datetime as dt_module + def prepTimeStamp(datetime_str): try: parsed_datetime = datetime.fromisoformat(datetime_str) if parsed_datetime.tzinfo is None: - parsed_datetime = conf.tz.localize(parsed_datetime) + parsed_datetime = parsed_datetime.replace(tzinfo=dt_module.timezone.utc) except ValueError: mylog('verbose', [f"[{pluginName}] Timestamp conversion failed of string '{datetime_str}'"]) parsed_datetime = timeNowUTC(as_string=False) return parsed_datetime.isoformat()
🤖 Fix all issues with AI agents
In @.github/skills/code-standards/SKILL.md:
- Around line 50-55: The Markdown lines beginning with `#` (the explanatory
comments about UTC/timeNowUTC()) are being rendered as headings; update the
SKILL.md content so those lines are not interpreted as H1 headers — either
remove the leading `#` characters or place the explanatory lines inside a fenced
code block or blockquote; ensure the guidance still references the canonical
function name timeNowUTC() and the rule that all database timestamps must be
stored in UTC so readers can find the SINGLE SOURCE OF TRUTH for current time.
In `@front/js/common.js`:
- Around line 455-468: The current logic unconditionally appends 'Z' to any
timestamp lacking an offset (using hasOffset and isoStr), which breaks non-ISO
inputs like RFC2822; change the behavior so you only append 'Z' when the input
is ISO-shaped: first test str against an ISO pattern (e.g.,
/^\d{4}-\d{2}-\d{2}(?:[ T]\d{2}:\d{2}:\d{2}(?:\.\d+)?)?$/) and only then perform
the space→'T' replacement and append 'Z' to isoStr; leave other formats (e.g.,
RFC2822) untouched so they are parsed as-is by new Date().
In `@server/database.py`:
- Around line 191-192: The call to migrate_timestamps_to_utc in initDB currently
ignores its boolean return; update initDB to check
migrate_timestamps_to_utc(self.sql) and handle a False result by preventing the
subsequent commit (e.g., rollback the transaction, log an error via the existing
logger, and propagate/raise an exception or return an error status from initDB)
so the DB is not left in a partially-migrated state; reference the
migrate_timestamps_to_utc function and the initDB commit path to locate where to
insert the check and the rollback/abort logic.
In `@server/db/db_upgrade.py`:
- Around line 646-650: The DST handling computes offset_hours from the current
timezone offset (using tz, now_local, and offset_hours) which misapplies DST to
historical timestamps; add a log message in the same block where offset_hours is
computed (the tz/now_local/offset_hours code in db_upgrade.py) that clearly
documents this limitation and warns that historical timestamps may be off by up
to ±1 hour for DST-observing zones (mention conf.tz or tz name), so
callers/operators are aware of the caveat.
🧹 Nitpick comments (10)
server/utils/datetime_utils.py (2)
188-190: Use!rconversion flag per Ruff RUF010.Suggested fix
- return f"invalid:{repr(date_str)}" + return f"invalid:{date_str!r}"Also applies to the similar pattern on line 207:
- return f"invalid:{repr(date_str)} e: {e}" + return f"invalid:{date_str!r} e: {e}"
192-204:format_datemirrorsformat_date_iso— consistent UTC conversion pattern.The handling of
conf.tzas string vs. tzinfo (lines 198-201) duplicates the pattern fromformat_date_iso(line 133). Consider extracting a small helper likeget_target_tz()to reduce duplication, but this is not blocking.front/plugins/internet_ip/script.py (1)
77-77: Redundantstr()wrapper —timeNowUTC()already returns a string.
timeNowUTC()with the defaultas_string=Truereturns astr, making thestr()call unnecessary.Suggested fix
- append_line_to_file(logPath + '/IP_changes.log', '[' + str(timeNowUTC()) + ']\t' + new_internet_IP + '\n') + append_line_to_file(logPath + '/IP_changes.log', '[' + timeNowUTC() + ']\t' + new_internet_IP + '\n')server/api_server/sync_endpoint.py (1)
25-25: Timestamps are passed explicitly, butwrite_notificationalready defaults totimeNowUTC().Since
write_notificationusestimeNowUTC()as its default whentimestampisNone, these explicit passes are redundant. Not a bug, but you could simplify by omitting the timestamp argument entirely.Also applies to: 31-31, 71-71, 76-76
server/db/db_upgrade.py (1)
517-568:current_offset_secondsis computed but never used for detection logic.Lines 524–540 compute
current_offset_secondsfrom the configured timezone, but the actual decision (Lines 556–568) relies solely on whether timezone markers (+,Z) exist in the sampled strings, or defaults toFalse. The offset variable is only logged on Line 566. If it's intentionally informational-only, that's fine, but if the intent was to use it for smarter heuristics, it's incomplete.test/db/test_timestamp_migration.py (1)
122-139: Consider verifying actual timestamp content after migration.
test_migrate_old_version_triggers_migrationonly assertsresult is Truebut doesn't verify that the timestamp values in theDevicestable were actually modified. Adding a check that reads back the migrated timestamps and compares them to the expected shifted values would strengthen confidence in the migration logic.server/scan/session_events.py (1)
174-229: Pre-existing:startTimeis interpolated via f-strings into SQL.
startTimefromtimeNowUTC()is safe (controlled format), but theinsert_eventsfunction uses f-string interpolation for all its SQL inserts (Lines 177, 190, 207, 221) whileinsertOnlineHistoryon Line 272 correctly uses parameterized queries. Consider migrating these to parameterized queries for consistency and defense-in-depth.Not introduced by this PR, so deferring is fine.
server/logger.py (1)
127-127: Log timestamps are now UTC with no timezone indicator.The
file_printfunction now emitsHH:MM:SSin UTC. Since there's no "Z" or "UTC" suffix, operators may interpret these as local time. Consider appending a "Z" suffix or a brief indicator so log readers know the time is UTC.💡 Suggested tweak
- result = timeNowUTC(as_string=False).strftime("%H:%M:%S") + " " + result = timeNowUTC(as_string=False).strftime("%H:%M:%S") + "Z "server/models/event_instance.py (2)
113-121: Dead branch: bothif/elsepaths are identical.
ensure_datetimealready handlesstr,datetime, andNoneinputs, so theisinstancecheck on line 118 is redundant — both branches execute the same call.♻️ Simplify
def createEvent(self, mac: str, ip: str, event_type: str = "Device Down", additional_info: str = "", pending_alert: int = 1, event_time: datetime | None = None): - if isinstance(event_time, str): - start_time = ensure_datetime(event_time) - else: - start_time = ensure_datetime(event_time) + start_time = ensure_datetime(event_time)
102-109: Two deletion methods with different time sources.
delete_older_than(line 102) computes the cutoff in Python viatimeNowUTC, whiledeleteEventsOlderThan(line 160) delegates to SQLite'sdate('now', ...). Both happen to use UTC (SQLite'sdate('now')is UTC), so results are consistent — but having two methods for the same operation with different implementations is a maintenance risk.Consider consolidating into one, or having
deleteEventsOlderThandelegate todelete_older_than.Also applies to: 160-172
| const hasOffset = /Z$/i.test(str.trim()) || | ||
| /GMT[+-]\d{2,4}/.test(str) || | ||
| /[+-]\d{2}:?\d{2}$/.test(str.trim()) || | ||
| /\([^)]+\)$/.test(str.trim()); | ||
|
|
||
| // ⚠️ CRITICAL: All DB timestamps are stored in UTC without timezone markers. | ||
| // If no offset is present, we must explicitly mark it as UTC by appending 'Z' | ||
| // so JavaScript doesn't interpret it as local browser time. | ||
| let isoStr = str.trim(); | ||
| if (!hasOffset) { | ||
| // Ensure proper ISO format before appending Z | ||
| // Replace space with 'T' if needed: "2026-02-11 11:37:02" → "2026-02-11T11:37:02Z" | ||
| isoStr = isoStr.trim().replace(/^(\d{4}-\d{2}-\d{2}) (\d{2}:\d{2}:\d{2})$/, '$1T$2') + 'Z'; | ||
| } |
There was a problem hiding this comment.
Appending Z to non-ISO strings (e.g., RFC2822) may break parsing.
When case 5 (RFC2822, line 425) matches a string like "25 Aug 2025 13:45:22" without an offset, hasOffset is false and Z gets appended, producing "25 Aug 2025 13:45:22Z". This is not a valid format for new Date() in all browsers and will likely yield NaN/"Failed conversion".
Consider limiting the Z-append to ISO-shaped strings only:
Proposed fix
let isoStr = str.trim();
if (!hasOffset) {
- // Ensure proper ISO format before appending Z
- // Replace space with 'T' if needed: "2026-02-11 11:37:02" → "2026-02-11T11:37:02Z"
- isoStr = isoStr.trim().replace(/^(\d{4}-\d{2}-\d{2}) (\d{2}:\d{2}:\d{2})$/, '$1T$2') + 'Z';
+ // Only force UTC for ISO-shaped strings (DB timestamps)
+ if (/^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}(:\d{2})?$/.test(isoStr)) {
+ isoStr = isoStr.replace(' ', 'T') + 'Z';
+ }
}🤖 Prompt for AI Agents
In `@front/js/common.js` around lines 455 - 468, The current logic unconditionally
appends 'Z' to any timestamp lacking an offset (using hasOffset and isoStr),
which breaks non-ISO inputs like RFC2822; change the behavior so you only append
'Z' when the input is ISO-shaped: first test str against an ISO pattern (e.g.,
/^\d{4}-\d{2}-\d{2}(?:[ T]\d{2}:\d{2}:\d{2}(?:\.\d+)?)?$/) and only then perform
the space→'T' replacement and append 'Z' to isoStr; leave other formats (e.g.,
RFC2822) untouched so they are parsed as-is by new Date().
| # One-time UTC timestamp migration (must run after Parameters table exists) | ||
| migrate_timestamps_to_utc(self.sql) |
There was a problem hiding this comment.
Return value of migrate_timestamps_to_utc is silently discarded.
Per the function's contract, it returns False on top-level failure (while internally catching per-table errors). If it returns False, initDB will proceed to commit, potentially leaving the database with inconsistent (mixed local/UTC) timestamps. Consider checking the return value:
💡 Suggested fix
# One-time UTC timestamp migration (must run after Parameters table exists)
- migrate_timestamps_to_utc(self.sql)
+ if not migrate_timestamps_to_utc(self.sql):
+ mylog("minimal", ["[Database] - WARNING: UTC timestamp migration failed"])📝 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.
| # One-time UTC timestamp migration (must run after Parameters table exists) | |
| migrate_timestamps_to_utc(self.sql) | |
| # One-time UTC timestamp migration (must run after Parameters table exists) | |
| if not migrate_timestamps_to_utc(self.sql): | |
| mylog("minimal", ["[Database] - WARNING: UTC timestamp migration failed"]) |
🤖 Prompt for AI Agents
In `@server/database.py` around lines 191 - 192, The call to
migrate_timestamps_to_utc in initDB currently ignores its boolean return; update
initDB to check migrate_timestamps_to_utc(self.sql) and handle a False result by
preventing the subsequent commit (e.g., rollback the transaction, log an error
via the existing logger, and propagate/raise an exception or return an error
status from initDB) so the DB is not left in a partially-migrated state;
reference the migrate_timestamps_to_utc function and the initDB commit path to
locate where to insert the check and the rollback/abort logic.
| if tz: | ||
| now_local = dt.datetime.now(tz) | ||
| offset_hours = (now_local.utcoffset().total_seconds()) / 3600 | ||
| else: | ||
| offset_hours = 0 |
There was a problem hiding this comment.
DST offset applied uniformly to all historical timestamps.
offset_hours is computed from the current UTC offset of conf.tz. For timezones with DST (e.g., CET alternating between UTC+1 and UTC+2), timestamps stored during a different DST period will be shifted by the wrong amount. This is inherently hard to fix perfectly, but worth documenting as a known limitation — affected timestamps could be off by up to 1 hour.
Consider adding a log line noting this caveat.
🤖 Prompt for AI Agents
In `@server/db/db_upgrade.py` around lines 646 - 650, The DST handling computes
offset_hours from the current timezone offset (using tz, now_local, and
offset_hours) which misapplies DST to historical timestamps; add a log message
in the same block where offset_hours is computed (the tz/now_local/offset_hours
code in db_upgrade.py) that clearly documents this limitation and warns that
historical timestamps may be off by up to ±1 hour for DST-observing zones
(mention conf.tz or tz name), so callers/operators are aware of the caveat.
1 participant
✅ Step 1: Refactored datetime_utils.py with timeNowUTC(as_string=True)
✅ Step 2: Updated 25+ server modules
✅ Step 3: Updated 40+ plugin files
✅ Step 4: Created VERSION-based UTC migration logic
✅ Step 5: Updated 4 existing test files
✅ Step 6: Created 25 comprehensive new tests
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests