Skip to content

refactor: replace datetime parsing with utility function for consistency across event handlers #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
CorieW wants to merge 2 commits into
base: @invertase/fix-timestamp-parsing
Choose a base branch
from
+13 −57
Draft

refactor: replace datetime parsing with utility function for consistency across event handlers #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b36564e
refactor: replace datetime parsing with utility function for consiste…
CorieW Apr 30, 2026
fb594f5
refactor: streamline timestamp handling in storage functions
CorieW Apr 30, 2026
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
6 changes: 1 addition & 5 deletions src/firebase_functions/eventarc_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
"""Cloud functions to handle Eventarc events."""

# pylint: disable=protected-access
import datetime as _dt
import functools as _functools
import typing as _typing

Expand Down Expand Up @@ -64,10 +63,7 @@ def on_custom_event_published_wrapped(raw: _ce.CloudEvent):
source=event_dict["source"],
specversion=event_dict["specversion"],
subject=event_dict["subject"] if "subject" in event_dict else None,
time=_dt.datetime.strptime(
event_dict["time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
),
time=_util.timestamp_conversion(event_dict["time"]),
type=event_dict["type"],
)
_with_init(func)(event)
Expand Down
21 changes: 2 additions & 19 deletions src/firebase_functions/pubsub_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
# pylint: disable=protected-access
import base64 as _base64
import dataclasses as _dataclasses
import datetime as _dt
import functools as _functools
import json as _json
import typing as _typing
Expand Down Expand Up @@ -105,25 +104,9 @@ def _message_handler(
data = event_dict["data"]
message_dict = data["message"]

# if no microseconds are present, we should set them to 0 to prevent parsing from failing
if "." not in event_dict["time"]:
event_dict["time"] = event_dict["time"].replace("Z", ".000000Z")
if "." not in message_dict["publish_time"]:
message_dict["publish_time"] = message_dict["publish_time"].replace("Z", ".000000Z")

time = _dt.datetime.strptime(
event_dict["time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
)

publish_time = _dt.datetime.strptime(
message_dict["publish_time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
)

# Convert the UTC string into a datetime object
event_dict["time"] = time
message_dict["publish_time"] = publish_time
event_dict["time"] = _util.timestamp_conversion(event_dict["time"])
message_dict["publish_time"] = _util.timestamp_conversion(message_dict["publish_time"])
Comment on lines +108 to +109
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

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

high

The replacement of manual parsing with _util.timestamp_conversion introduces a potential regression for Python versions prior to 3.11. The previous implementation (lines 109-112 in the original code) explicitly handled the 'Z' suffix by replacing it or using a format that worked. However, _util.timestamp_conversion uses strptime with the %z directive for second and microsecond precisions, which does not support the 'Z' suffix in Python < 3.11. Since this library likely supports Python 3.10, this change will cause a ValueError when processing timestamps ending in 'Z'. It is recommended to update the utility function _util.timestamp_conversion to handle the 'Z' suffix for all precisions.


# Pop unnecessary keys from the message data
# (we get these keys from the snake case alternatives that are provided)
Expand Down
7 changes: 2 additions & 5 deletions src/firebase_functions/remote_config_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def _config_handler(func: _C1, raw: _ce.CloudEvent) -> None:

config_data = ConfigUpdateData(
version_number=event_data["versionNumber"],
update_time=_dt.datetime.strptime(event_data["updateTime"], "%Y-%m-%dT%H:%M:%S.%f%z"),
update_time=_util.timestamp_conversion(event_data["updateTime"]),
update_user=ConfigUser(
name=event_data["updateUser"]["name"],
email=event_data["updateUser"]["email"],
Expand All @@ -182,10 +182,7 @@ def _config_handler(func: _C1, raw: _ce.CloudEvent) -> None:
source=event_dict["source"],
specversion=event_dict["specversion"],
subject=event_dict["subject"] if "subject" in event_dict else None,
time=_dt.datetime.strptime(
event_dict["time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
),
time=_util.timestamp_conversion(event_dict["time"]),
type=event_dict["type"],
)

Expand Down
23 changes: 5 additions & 18 deletions src/firebase_functions/scheduler_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,11 @@ def on_schedule_wrapped(request: _Request) -> _Response:
schedule_time = _dt.datetime.now(_timezone.utc)
else:
try:
# Try to parse with the stdlib which supports fractional
# seconds and offsets in Python 3.11+ via fromisoformat.
# Normalize RFC3339 'Z' to '+00:00' for fromisoformat.
iso_str = schedule_time_str
if iso_str.endswith("Z"):
iso_str = iso_str[:-1] + "+00:00"
schedule_time = _dt.datetime.fromisoformat(iso_str)
except ValueError:
# Fallback to strict parsing without fractional seconds
try:
schedule_time = _dt.datetime.strptime(
schedule_time_str,
"%Y-%m-%dT%H:%M:%S%z",
)
except ValueError as e:
# If all parsing fails, log and use current UTC time
_logging.exception(e)
schedule_time = _dt.datetime.now(_timezone.utc)
schedule_time = _util.timestamp_conversion(schedule_time_str)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

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

high

The refactor to use _util.timestamp_conversion removes the explicit normalization of the 'Z' suffix to '+00:00' (previously at lines 112-113). While datetime.fromisoformat and strptime with %z support 'Z' in Python 3.11+, they do not in Python 3.10. Since this library supports Python 3.10, this change introduces a regression where timestamps ending in 'Z' will fail to parse. It is recommended to update the utility function _util.timestamp_conversion to handle the 'Z' suffix for all precisions, or to retain the normalization here.

except ValueError as e:
# If parsing fails, log and use current UTC time.
_logging.exception(e)
schedule_time = _dt.datetime.now(_timezone.utc)
event = ScheduledEvent(
job_name=request.headers.get("X-CloudScheduler-JobName"),
schedule_time=schedule_time,
Expand Down
6 changes: 1 addition & 5 deletions src/firebase_functions/storage_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

# pylint: disable=protected-access
import dataclasses as _dataclasses
import datetime as _dt
import functools as _functools
import typing as _typing

Expand Down Expand Up @@ -250,10 +249,7 @@ def _message_handler(
source=event_attributes["source"],
specversion=event_attributes["specversion"],
subject=event_attributes["subject"] if "subject" in event_attributes else None,
time=_dt.datetime.strptime(
event_attributes["time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
),
time=_util.timestamp_conversion(event_attributes["time"]),
type=event_attributes["type"],
)

Expand Down
7 changes: 2 additions & 5 deletions src/firebase_functions/test_lab_fn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def _event_handler(func: _C1, raw: _ce.CloudEvent) -> None:
event_dict = {**event_data, **event_attributes}

test_lab_data = TestMatrixCompletedData(
create_time=_dt.datetime.strptime(event_data["createTime"], "%Y-%m-%dT%H:%M:%S.%f%z"),
create_time=_util.timestamp_conversion(event_data["createTime"]),
state=TestState(event_data["state"]),
invalid_matrix_details=event_data.get("invalidMatrixDetails"),
outcome_summary=OutcomeSummary(event_data["outcomeSummary"]),
Expand All @@ -237,10 +237,7 @@ def _event_handler(func: _C1, raw: _ce.CloudEvent) -> None:
source=event_dict["source"],
specversion=event_dict["specversion"],
subject=event_dict["subject"] if "subject" in event_dict else None,
time=_dt.datetime.strptime(
event_dict["time"],
"%Y-%m-%dT%H:%M:%S.%f%z",
),
time=_util.timestamp_conversion(event_dict["time"]),
type=event_dict["type"],
)

Expand Down
Loading