Skip to content

Commit cfd907a

Browse files
committed
feat: require timezone-aware datetime for first_run_at
Change from warning to error when first_run_at is timezone-naive. This prevents bugs from Python's datetime.timestamp() treating naive datetimes as local time instead of UTC. - Require explicit timezone on first_run_at datetime - Remove warning, raise ValueError instead - Update tests to match new behavior - Aligns with Python best practices and Temporal SDK approach Signed-off-by: Tim Li <ltim@uber.com>
1 parent dd43278 commit cfd907a

File tree

2 files changed

+15
-39
lines changed

2 files changed

+15
-39
lines changed

cadence/client.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import os
22
import socket
33
import uuid
4-
import warnings
54
from datetime import datetime, timedelta
65
from typing import TypedDict, Unpack, Any, cast, Union
76

@@ -59,14 +58,14 @@ def _validate_and_apply_defaults(options: StartWorkflowOptions) -> StartWorkflow
5958
elif task_timeout <= timedelta(0):
6059
raise ValueError("task_start_to_close_timeout must be greater than 0")
6160

62-
# Warn if first_run_at is timezone-naive, and validate it's not before Unix epoch
61+
# Validate first_run_at (must be timezone-aware and not before Unix epoch)
6362
first_run_at = options.get("first_run_at")
6463
if first_run_at is not None:
64+
# Require timezone-aware datetime to prevent ambiguity
6565
if first_run_at.tzinfo is None:
66-
warnings.warn(
67-
"first_run_at is timezone-naive; it will be treated as UTC",
68-
UserWarning,
69-
stacklevel=3,
66+
raise ValueError(
67+
"first_run_at must be timezone-aware. "
68+
"Use datetime.now(timezone.utc) or datetime(..., tzinfo=timezone.utc)"
7069
)
7170
# Validate timestamp is not before Unix epoch (matching Go client behavior)
7271
if first_run_at.timestamp() < 0:

tests/cadence/test_client_workflow.py

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import pytest
22
import uuid
3-
import warnings
43
from datetime import timedelta, datetime, timezone
54
from unittest.mock import AsyncMock, Mock, PropertyMock
65

@@ -266,50 +265,28 @@ async def test_build_request_with_first_run_at_timezone_aware(self, mock_client)
266265
tzinfo=None
267266
) # ToDatetime returns naive UTC
268267

269-
@pytest.mark.asyncio
270-
async def test_build_request_with_first_run_at_naive(self, mock_client):
271-
"""Test building request with timezone-naive first_run_at."""
272-
client = Client(domain="test-domain", target="localhost:7933")
273-
274-
first_run = datetime(2024, 6, 1, 12, 0, 0) # Naive datetime
275-
276-
options = StartWorkflowOptions(
277-
task_list="test-task-list",
278-
execution_start_to_close_timeout=timedelta(minutes=10),
279-
task_start_to_close_timeout=timedelta(seconds=30),
280-
first_run_at=first_run,
281-
)
282-
283-
request = client._build_start_workflow_request("TestWorkflow", (), options)
284-
285-
assert request.HasField("first_run_at")
286-
287-
def test_first_run_at_naive_datetime_warns(self):
288-
"""Test that timezone-naive first_run_at produces warning."""
268+
def test_first_run_at_naive_datetime_raises_error(self):
269+
"""Test that timezone-naive first_run_at raises ValueError."""
289270
options = StartWorkflowOptions(
290271
task_list="test-task-list",
291272
execution_start_to_close_timeout=timedelta(minutes=10),
292273
first_run_at=datetime(2024, 1, 1, 12, 0, 0), # Naive
293274
)
294-
with pytest.warns(UserWarning, match="timezone-naive"):
275+
with pytest.raises(ValueError, match="must be timezone-aware"):
295276
_validate_and_apply_defaults(options)
296277

297-
def test_first_run_at_aware_datetime_no_warning(self):
298-
"""Test that timezone-aware first_run_at produces no warning."""
278+
def test_first_run_at_aware_datetime_is_valid(self):
279+
"""Test that timezone-aware first_run_at is valid."""
299280
options = StartWorkflowOptions(
300281
task_list="test-task-list",
301282
execution_start_to_close_timeout=timedelta(minutes=10),
302283
first_run_at=datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc),
303284
)
304-
# Should not warn - use warnings.catch_warnings to verify
305-
with warnings.catch_warnings(record=True) as w:
306-
warnings.simplefilter("always")
307-
_validate_and_apply_defaults(options)
308-
# Filter for our specific warning
309-
relevant_warnings = [
310-
warning for warning in w if "timezone-naive" in str(warning.message)
311-
]
312-
assert len(relevant_warnings) == 0
285+
# Should not raise
286+
validated = _validate_and_apply_defaults(options)
287+
assert validated["first_run_at"] == datetime(
288+
2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc
289+
)
313290

314291
def test_first_run_at_before_epoch_raises_error(self):
315292
"""Test that first_run_at before Unix epoch raises ValueError."""

0 commit comments

Comments
 (0)