codeant-ai-for-open-source[bot] commented on code in PR #37014:
URL: https://github.com/apache/superset/pull/37014#discussion_r2676697513
##########
superset/models/helpers.py:
##########
@@ -2387,6 +2387,17 @@ def get_time_filter( # pylint:
disable=too-many-arguments
)
)
+ # Apply timezone offset to convert local time boundaries to UTC
+ # The offset is in hours (e.g., 6 for UTC+6)
+ # If offset is set, we subtract it from the datetime to convert to UTC
+ # Example: User selects "Last day" which gives midnight local time
+ # If offset=6 (UTC+6), we subtract 6 hours to get UTC time
+ offset_hours = getattr(self, "offset", 0) or 0
+ if offset_hours and start_dttm:
Review Comment:
**Suggestion:** The value returned by `self.offset` may not be an integer
(it could be a string or None) and passing a non-numeric value to
`timedelta(hours=...)` will raise a TypeError; coerce the offset to an int and
fallback to 0 on failure. [type error]
**Severity Level:** Minor ⚠️
```suggestion
try:
offset_hours = int(offset_hours)
except (TypeError, ValueError):
# If offset cannot be converted to int, treat it as no offset
offset_hours = 0
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current code assumes self.offset is numeric. If a subclass or consumer
accidentally provides a string (e.g. "6") or other non-numeric value,
timedelta(hours=offset_hours) will raise TypeError. Coercing to int with a safe
fallback prevents an unexpected runtime crash. The change is local and
defensive.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 2396:2396
**Comment:**
*Type Error: The value returned by `self.offset` may not be an integer
(it could be a string or None) and passing a non-numeric value to
`timedelta(hours=...)` will raise a TypeError; coerce the offset to an int and
fallback to 0 on failure.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset/models/helpers.py:
##########
@@ -2387,6 +2387,17 @@ def get_time_filter( # pylint:
disable=too-many-arguments
)
)
+ # Apply timezone offset to convert local time boundaries to UTC
+ # The offset is in hours (e.g., 6 for UTC+6)
+ # If offset is set, we subtract it from the datetime to convert to UTC
+ # Example: User selects "Last day" which gives midnight local time
+ # If offset=6 (UTC+6), we subtract 6 hours to get UTC time
+ offset_hours = getattr(self, "offset", 0) or 0
+ if offset_hours and start_dttm:
+ start_dttm = start_dttm - timedelta(hours=offset_hours)
+ if offset_hours and end_dttm:
+ end_dttm = end_dttm - timedelta(hours=offset_hours)
Review Comment:
**Suggestion:** The same issues apply to `end_dttm`: subtracting a timedelta
from a string or from a timezone-aware datetime can raise errors or
double-adjust the timezone; parse strings and convert tz-aware datetimes to UTC
instead of applying the numeric offset. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Accept string inputs as datetimes
if isinstance(end_dttm, str):
end_dttm = dateutil.parser.parse(end_dttm)
# If datetime is timezone-aware, convert directly to UTC (do not
apply offset again)
if getattr(end_dttm, "tzinfo", None):
end_dttm = end_dttm.astimezone(pytz.utc)
else:
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Same rationale as for start_dttm: string inputs would raise on subtraction
and tz-aware datetimes should be converted to UTC instead of blindly
subtracting hours. Handling end_dttm the same way avoids asymmetric behavior
between start and end.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 2399:2399
**Comment:**
*Logic Error: The same issues apply to `end_dttm`: subtracting a
timedelta from a string or from a timezone-aware datetime can raise errors or
double-adjust the timezone; parse strings and convert tz-aware datetimes to UTC
instead of applying the numeric offset.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset/models/helpers.py:
##########
@@ -2387,6 +2387,17 @@ def get_time_filter( # pylint:
disable=too-many-arguments
)
)
+ # Apply timezone offset to convert local time boundaries to UTC
+ # The offset is in hours (e.g., 6 for UTC+6)
+ # If offset is set, we subtract it from the datetime to convert to UTC
+ # Example: User selects "Last day" which gives midnight local time
+ # If offset=6 (UTC+6), we subtract 6 hours to get UTC time
+ offset_hours = getattr(self, "offset", 0) or 0
+ if offset_hours and start_dttm:
+ start_dttm = start_dttm - timedelta(hours=offset_hours)
Review Comment:
**Suggestion:** Subtracting a timedelta from a string or applying a numeric
offset to a timezone-aware datetime will raise or double-adjust the value;
parse string inputs to datetime and, if the datetime already has tzinfo,
convert it to UTC instead of applying the numeric offset. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Accept string inputs as datetimes
if isinstance(start_dttm, str):
start_dttm = dateutil.parser.parse(start_dttm)
# If datetime is timezone-aware, convert directly to UTC (do not
apply offset again)
if getattr(start_dttm, "tzinfo", None):
start_dttm = start_dttm.astimezone(pytz.utc)
else:
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Subtracting a timedelta from a string will raise, and subtracting hours from
a tz-aware datetime can double-adjust the timestamp or be incorrect. Parsing
string inputs and converting tz-aware datetimes properly to UTC (via
astimezone) addresses both problems. The project already imports
dateutil.parser and pytz, so the suggested code is implementable and fixes real
edge-cases.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/models/helpers.py
**Line:** 2397:2397
**Comment:**
*Logic Error: Subtracting a timedelta from a string or applying a
numeric offset to a timezone-aware datetime will raise or double-adjust the
value; parse string inputs to datetime and, if the datetime already has tzinfo,
convert it to UTC instead of applying the numeric offset.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
