From db9e0a00cea2187859839a9cc044dd11114b0829 Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sat, 4 Oct 2025 13:15:42 +0530 Subject: [PATCH 01/14] feat: add timezone support for CRON triggers (#438) Allow users to specify timezone when setting up CRON triggers to enable scheduling in local time zones with automatic DST handling. Changes: - Add timezone field to CronTrigger and DatabaseTriggers models - Implement timezone-aware cron scheduling using Python's zoneinfo - Validate timezone using IANA timezone database - Update Python SDK to support timezone parameter - Add comprehensive documentation with examples - Default to UTC for backward compatibility The timezone field is optional and defaults to "UTC". All trigger times are internally stored in UTC while croniter calculations respect the specified timezone, ensuring correct scheduling across time zones and DST transitions. Signed-off-by: Sparsh --- docs/docs/exosphere/triggers.md | 36 +++++- python-sdk/exospherehost/models.py | 3 +- python-sdk/exospherehost/statemanager.py | 3 +- state-manager/app/models/db/trigger.py | 1 + state-manager/app/models/trigger_models.py | 13 +- state-manager/app/tasks/trigger_cron.py | 16 ++- state-manager/app/tasks/verify_graph.py | 31 +++-- .../tests/unit/models/test_trigger_models.py | 118 ++++++++++++++++++ 8 files changed, 203 insertions(+), 18 deletions(-) create mode 100644 state-manager/tests/unit/models/test_trigger_models.py diff --git a/docs/docs/exosphere/triggers.md b/docs/docs/exosphere/triggers.md index 67de4cfd..aa220cd8 100644 --- a/docs/docs/exosphere/triggers.md +++ b/docs/docs/exosphere/triggers.md @@ -61,13 +61,15 @@ Define triggers in your graph template: { "type": "CRON", "value": { - "expression": "0 9 * * 1-5" + "expression": "0 9 * * 1-5", + "timezone": "America/New_York" } }, { "type": "CRON", "value": { - "expression": "0 0 * * 0" + "expression": "0 0 * * 0", + "timezone": "UTC" } } ], @@ -77,6 +79,8 @@ Define triggers in your graph template: } ``` +**Note:** The `timezone` field is optional and defaults to `"UTC"` if not specified. Use IANA timezone names (e.g., `"America/New_York"`, `"Europe/London"`, `"Asia/Tokyo"`). + ### Python SDK Example ```python @@ -109,8 +113,8 @@ async def create_scheduled_graph(): # Define triggers for automatic execution triggers = [ - CronTrigger(expression="0 2 * * *"), # Daily at 2:00 AM - CronTrigger(expression="0 */4 * * *") # Every 4 hours + CronTrigger(expression="0 2 * * *", timezone="America/New_York"), # Daily at 2:00 AM EST/EDT + CronTrigger(expression="0 */4 * * *", timezone="UTC") # Every 4 hours UTC ] # Create the graph with triggers @@ -158,7 +162,7 @@ asyncio.run(create_scheduled_graph()) 1. **Avoid Peak Times**: Schedule resource-intensive workflows during off-peak hours 2. **Stagger Executions**: If you have multiple graphs, stagger their execution times -3. **Consider Time Zones**: Cron expressions use server time (UTC by default) +3. **Consider Time Zones**: Specify the `timezone` parameter to ensure your cron expressions run at the correct local time. If not specified, defaults to UTC. 4. **Resource Planning**: Ensure your infrastructure can handle scheduled workloads ### Error Handling @@ -191,11 +195,31 @@ result = await state_manager.upsert_graph( ) ``` +## Timezone Support + +Triggers now support specifying a timezone for cron expressions, allowing you to schedule jobs in your local timezone: + +```python +# Schedule a report to run at 9 AM New York time (handles DST automatically) +CronTrigger(expression="0 9 * * 1-5", timezone="America/New_York") + +# Schedule a job at 5 PM London time +CronTrigger(expression="0 17 * * *", timezone="Europe/London") + +# Schedule using UTC (default) +CronTrigger(expression="0 12 * * *", timezone="UTC") +``` + +**Important Notes:** +- Use IANA timezone names (e.g., `"America/New_York"`, `"Europe/London"`, `"Asia/Tokyo"`) +- Timezones automatically handle Daylight Saving Time (DST) transitions +- If no timezone is specified, defaults to `"UTC"` +- All trigger times are internally stored in UTC for consistency + ## Limitations - **CRON Only**: Currently only cron-based scheduling is supported - **No Manual Override**: Scheduled executions cannot be manually cancelled once triggered -- **Time Zone**: All cron expressions are evaluated in server time (UTC) - **Minimum Interval**: Avoid scheduling more frequently than every minute ## Next Steps diff --git a/python-sdk/exospherehost/models.py b/python-sdk/exospherehost/models.py index 92ed3381..ecb7c17d 100644 --- a/python-sdk/exospherehost/models.py +++ b/python-sdk/exospherehost/models.py @@ -160,4 +160,5 @@ def validate_default_values(cls, v: dict[str, str]) -> dict[str, str]: return normalized_dict class CronTrigger(BaseModel): - expression: str = Field(..., description="Cron expression for scheduling automatic graph execution. Uses standard 5-field format: minute hour day-of-month month day-of-week. Example: '0 9 * * 1-5' for weekdays at 9 AM.") \ No newline at end of file + expression: str = Field(..., description="Cron expression for scheduling automatic graph execution. Uses standard 5-field format: minute hour day-of-month month day-of-week. Example: '0 9 * * 1-5' for weekdays at 9 AM.") + timezone: str = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC'). Defaults to 'UTC'.") \ No newline at end of file diff --git a/python-sdk/exospherehost/statemanager.py b/python-sdk/exospherehost/statemanager.py index 13cf6890..eed01414 100644 --- a/python-sdk/exospherehost/statemanager.py +++ b/python-sdk/exospherehost/statemanager.py @@ -173,7 +173,8 @@ async def upsert_graph(self, graph_name: str, graph_nodes: list[GraphNodeModel], { "type": "CRON", "value": { - "expression": trigger.expression + "expression": trigger.expression, + "timezone": trigger.timezone } } for trigger in triggers diff --git a/state-manager/app/models/db/trigger.py b/state-manager/app/models/db/trigger.py index 26fe199f..d32ffa64 100644 --- a/state-manager/app/models/db/trigger.py +++ b/state-manager/app/models/db/trigger.py @@ -9,6 +9,7 @@ class DatabaseTriggers(Document): type: TriggerTypeEnum = Field(..., description="Type of the trigger") expression: Optional[str] = Field(default=None, description="Expression of the trigger") + timezone: Optional[str] = Field(default="UTC", description="Timezone for the trigger") graph_name: str = Field(..., description="Name of the graph") namespace: str = Field(..., description="Namespace of the graph") trigger_time: datetime = Field(..., description="Trigger time of the trigger") diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index a8dbddb0..8ce31f4e 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -1,7 +1,8 @@ from pydantic import BaseModel, Field, field_validator, model_validator from enum import Enum from croniter import croniter -from typing import Self +from typing import Self, Optional +from zoneinfo import available_timezones class TriggerTypeEnum(str, Enum): CRON = "CRON" @@ -15,6 +16,7 @@ class TriggerStatusEnum(str, Enum): class CronTrigger(BaseModel): expression: str = Field(..., description="Cron expression for the trigger") + timezone: Optional[str] = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC')") @field_validator("expression") @classmethod @@ -23,6 +25,15 @@ def validate_expression(cls, v: str) -> str: raise ValueError("Invalid cron expression") return v + @field_validator("timezone") + @classmethod + def validate_timezone(cls, v: Optional[str]) -> str: + if v is None: + return "UTC" + if v not in available_timezones(): + raise ValueError(f"Invalid timezone: {v}. Must be a valid IANA timezone (e.g., 'America/New_York', 'Europe/London', 'UTC')") + return v + class Trigger(BaseModel): type: TriggerTypeEnum = Field(..., description="Type of the trigger") value: dict = Field(default_factory=dict, description="Value of the trigger") diff --git a/state-manager/app/tasks/trigger_cron.py b/state-manager/app/tasks/trigger_cron.py index ab0771bb..acf9552e 100644 --- a/state-manager/app/tasks/trigger_cron.py +++ b/state-manager/app/tasks/trigger_cron.py @@ -8,6 +8,7 @@ from pymongo import ReturnDocument from pymongo.errors import DuplicateKeyError from app.config.settings import get_settings +from zoneinfo import ZoneInfo import croniter import asyncio @@ -42,15 +43,26 @@ async def mark_as_failed(trigger: DatabaseTriggers): async def create_next_triggers(trigger: DatabaseTriggers, cron_time: datetime): assert trigger.expression is not None - iter = croniter.croniter(trigger.expression, trigger.trigger_time) + + # Use the trigger's timezone, defaulting to UTC if not specified + tz = ZoneInfo(trigger.timezone or "UTC") + + # Convert trigger_time to the specified timezone for croniter + trigger_time_tz = trigger.trigger_time.replace(tzinfo=ZoneInfo("UTC")).astimezone(tz) + iter = croniter.croniter(trigger.expression, trigger_time_tz) while True: - next_trigger_time = iter.get_next(datetime) + # Get next trigger time in the specified timezone + next_trigger_time_tz = iter.get_next(datetime) + + # Convert back to UTC for storage + next_trigger_time = next_trigger_time_tz.astimezone(ZoneInfo("UTC")).replace(tzinfo=None) try: await DatabaseTriggers( type=TriggerTypeEnum.CRON, expression=trigger.expression, + timezone=trigger.timezone, graph_name=trigger.graph_name, namespace=trigger.namespace, trigger_time=next_trigger_time, diff --git a/state-manager/app/tasks/verify_graph.py b/state-manager/app/tasks/verify_graph.py index 7fbb021d..373eea7e 100644 --- a/state-manager/app/tasks/verify_graph.py +++ b/state-manager/app/tasks/verify_graph.py @@ -3,6 +3,7 @@ from datetime import datetime from json_schema_to_pydantic import create_model +from zoneinfo import ZoneInfo from app.models.db.graph_template_model import GraphTemplate from app.models.graph_template_validation_status import GraphTemplateValidationStatus @@ -101,20 +102,36 @@ async def verify_inputs(graph_template: GraphTemplate, registered_nodes: list[Re return errors async def create_crons(graph_template: GraphTemplate): - expressions_to_create = set([trigger.value["expression"] for trigger in graph_template.triggers if trigger.type == TriggerTypeEnum.CRON]) + # Build a map of (expression, timezone) -> trigger for deduplication + triggers_to_create = {} + for trigger in graph_template.triggers: + if trigger.type == TriggerTypeEnum.CRON: + expression = trigger.value["expression"] + timezone = trigger.value.get("timezone", "UTC") + triggers_to_create[(expression, timezone)] = trigger + + current_time = datetime.now(ZoneInfo("UTC")).replace(tzinfo=None) - current_time = datetime.now() - new_db_triggers = [] - for expression in expressions_to_create: - iter = croniter.croniter(expression, current_time) + for (expression, timezone), trigger in triggers_to_create.items(): + # Use the trigger's timezone, defaulting to UTC + tz = ZoneInfo(timezone) + + # Get current time in the specified timezone + current_time_tz = current_time.replace(tzinfo=ZoneInfo("UTC")).astimezone(tz) + iter = croniter.croniter(expression, current_time_tz) + + # Get next trigger time in the specified timezone + next_trigger_time_tz = iter.get_next(datetime) + + # Convert back to UTC for storage (remove timezone info for storage) + next_trigger_time = next_trigger_time_tz.astimezone(ZoneInfo("UTC")).replace(tzinfo=None) - next_trigger_time = iter.get_next(datetime) - new_db_triggers.append( DatabaseTriggers( type=TriggerTypeEnum.CRON, expression=expression, + timezone=timezone, graph_name=graph_template.name, namespace=graph_template.namespace, trigger_status=TriggerStatusEnum.PENDING, diff --git a/state-manager/tests/unit/models/test_trigger_models.py b/state-manager/tests/unit/models/test_trigger_models.py new file mode 100644 index 00000000..e214d2a6 --- /dev/null +++ b/state-manager/tests/unit/models/test_trigger_models.py @@ -0,0 +1,118 @@ +import pytest +from pydantic import ValidationError +from app.models.trigger_models import CronTrigger, Trigger, TriggerTypeEnum + + +class TestCronTrigger: + """Test cases for CronTrigger model""" + + def test_valid_cron_trigger_with_timezone(self): + """Test creating a valid cron trigger with timezone""" + trigger = CronTrigger(expression="0 9 * * *", timezone="America/New_York") + assert trigger.expression == "0 9 * * *" + assert trigger.timezone == "America/New_York" + + def test_valid_cron_trigger_without_timezone(self): + """Test creating a valid cron trigger without timezone defaults to UTC""" + trigger = CronTrigger(expression="0 9 * * *") + assert trigger.expression == "0 9 * * *" + assert trigger.timezone == "UTC" + + def test_valid_cron_trigger_with_utc_timezone(self): + """Test creating a valid cron trigger with UTC timezone""" + trigger = CronTrigger(expression="0 9 * * *", timezone="UTC") + assert trigger.expression == "0 9 * * *" + assert trigger.timezone == "UTC" + + def test_valid_cron_trigger_with_europe_london(self): + """Test creating a valid cron trigger with Europe/London timezone""" + trigger = CronTrigger(expression="0 17 * * *", timezone="Europe/London") + assert trigger.expression == "0 17 * * *" + assert trigger.timezone == "Europe/London" + + def test_valid_cron_trigger_with_asia_tokyo(self): + """Test creating a valid cron trigger with Asia/Tokyo timezone""" + trigger = CronTrigger(expression="30 8 * * 1-5", timezone="Asia/Tokyo") + assert trigger.expression == "30 8 * * 1-5" + assert trigger.timezone == "Asia/Tokyo" + + def test_invalid_cron_expression(self): + """Test creating a cron trigger with invalid expression""" + with pytest.raises(ValidationError) as exc_info: + CronTrigger(expression="invalid cron", timezone="UTC") + + errors = exc_info.value.errors() + assert len(errors) == 1 + assert "Invalid cron expression" in str(errors[0]["ctx"]["error"]) + + def test_invalid_timezone(self): + """Test creating a cron trigger with invalid timezone""" + with pytest.raises(ValidationError) as exc_info: + CronTrigger(expression="0 9 * * *", timezone="Invalid/Timezone") + + errors = exc_info.value.errors() + assert len(errors) == 1 + assert "Invalid timezone" in str(errors[0]["ctx"]["error"]) + assert "Invalid/Timezone" in str(errors[0]["ctx"]["error"]) + + def test_none_timezone_defaults_to_utc(self): + """Test that None timezone defaults to UTC""" + trigger = CronTrigger(expression="0 9 * * *", timezone=None) + assert trigger.timezone == "UTC" + + def test_complex_cron_expression_with_timezone(self): + """Test complex cron expression with timezone""" + trigger = CronTrigger(expression="0 0 1,15 * *", timezone="America/Los_Angeles") + assert trigger.expression == "0 0 1,15 * *" + assert trigger.timezone == "America/Los_Angeles" + + def test_every_15_minutes_cron_with_timezone(self): + """Test every 15 minutes cron with timezone""" + trigger = CronTrigger(expression="*/15 * * * *", timezone="Europe/Paris") + assert trigger.expression == "*/15 * * * *" + assert trigger.timezone == "Europe/Paris" + + +class TestTrigger: + """Test cases for Trigger model""" + + def test_valid_trigger_with_cron_and_timezone(self): + """Test creating a valid trigger with CRON type and timezone""" + trigger = Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ) + assert trigger.type == TriggerTypeEnum.CRON + assert trigger.value["expression"] == "0 9 * * *" + assert trigger.value["timezone"] == "America/New_York" + + def test_valid_trigger_with_cron_without_timezone(self): + """Test creating a valid trigger with CRON type without timezone""" + trigger = Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *"} + ) + assert trigger.type == TriggerTypeEnum.CRON + assert trigger.value["expression"] == "0 9 * * *" + + def test_invalid_trigger_with_invalid_cron_expression(self): + """Test creating a trigger with invalid cron expression""" + with pytest.raises(ValidationError) as exc_info: + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "invalid cron"} + ) + + errors = exc_info.value.errors() + assert len(errors) > 0 + + def test_invalid_trigger_with_invalid_timezone(self): + """Test creating a trigger with invalid timezone""" + with pytest.raises(ValidationError) as exc_info: + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "Invalid/Zone"} + ) + + errors = exc_info.value.errors() + assert len(errors) > 0 \ No newline at end of file From 93071da38425a6ff2370c3a313c7a28b3b4c7e95 Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sat, 4 Oct 2025 13:55:24 +0530 Subject: [PATCH 02/14] test: add comprehensive tests for timezone functionality Add unit tests to improve code coverage for timezone support: - Tests for CronTrigger model timezone validation - Tests for create_crons function with various timezone scenarios - Tests for timezone deduplication logic - Tests for default UTC timezone behavior All tests use mocks to avoid database dependencies and rely on environment variables set by CI workflow. Signed-off-by: Sparsh --- .../unit/tasks/test_create_crons_coverage.py | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) create mode 100644 state-manager/tests/unit/tasks/test_create_crons_coverage.py diff --git a/state-manager/tests/unit/tasks/test_create_crons_coverage.py b/state-manager/tests/unit/tasks/test_create_crons_coverage.py new file mode 100644 index 00000000..75f7382d --- /dev/null +++ b/state-manager/tests/unit/tasks/test_create_crons_coverage.py @@ -0,0 +1,236 @@ +""" +Tests for create_crons function to improve code coverage. +These are pure unit tests that mock DatabaseTriggers to avoid database dependency. +Environment variables are provided by CI (see .github/workflows/test-state-manager.yml). +""" +import pytest +from unittest.mock import MagicMock, AsyncMock, patch +from datetime import datetime + +# Environment variables are set by CI workflow +# For local testing, these should be set before running tests +# See: .github/workflows/test-state-manager.yml + +from app.tasks.verify_graph import create_crons +from app.models.trigger_models import Trigger, TriggerTypeEnum + + +@pytest.mark.asyncio +async def test_create_crons_with_america_new_york_timezone(): + """Test create_crons processes America/New_York timezone correctly""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ) + ] + + # Mock DatabaseTriggers class and insert_many method + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + # Mock instances that will be created + mock_instance = MagicMock() + mock_db_class.return_value = mock_instance + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Verify DatabaseTriggers was instantiated with correct parameters + assert mock_db_class.called + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "America/New_York" + assert call_kwargs['expression'] == "0 9 * * *" + assert call_kwargs['graph_name'] == "test_graph" + assert call_kwargs['namespace'] == "test_ns" + + # Verify insert_many was called with the list of triggers + assert mock_db_class.insert_many.called + + +@pytest.mark.asyncio +async def test_create_crons_with_default_utc_timezone(): + """Test create_crons uses UTC as default when timezone not specified""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *"} # No timezone specified + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Verify default UTC timezone was used + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "UTC" + assert mock_db_class.insert_many.called + + +@pytest.mark.asyncio +async def test_create_crons_with_europe_london_timezone(): + """Test create_crons handles Europe/London timezone""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 17 * * *", "timezone": "Europe/London"} + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "Europe/London" + assert call_kwargs['expression'] == "0 17 * * *" + assert mock_db_class.insert_many.called + + +@pytest.mark.asyncio +async def test_create_crons_with_multiple_different_timezones(): + """Test create_crons handles multiple triggers with different timezones""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ), + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 17 * * *", "timezone": "Europe/London"} + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Should be called twice (one for each trigger) + assert mock_db_class.call_count == 2 + + # Verify insert_many was called once with list of 2 triggers + assert mock_db_class.insert_many.call_count == 1 + insert_call_args = mock_db_class.insert_many.call_args[0][0] + assert len(insert_call_args) == 2 + + +@pytest.mark.asyncio +async def test_create_crons_skips_insert_when_no_triggers(): + """Test create_crons doesn't call insert_many when no CRON triggers exist""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Verify insert_many was NOT called (no triggers to insert) + assert not mock_db_class.insert_many.called + # DatabaseTriggers should not have been instantiated + assert mock_db_class.call_count == 0 + + +@pytest.mark.asyncio +async def test_create_crons_deduplicates_same_expression_and_timezone(): + """Test create_crons deduplicates triggers with same expression and timezone""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ), + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Should only create one DatabaseTriggers instance (deduplicated) + assert mock_db_class.call_count == 1 + + # insert_many should be called with list of 1 + insert_call_args = mock_db_class.insert_many.call_args[0][0] + assert len(insert_call_args) == 1 + + +@pytest.mark.asyncio +async def test_create_crons_keeps_same_expression_different_timezones(): + """Test create_crons keeps triggers with same expression but different timezones""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ), + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "Europe/London"} + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Should create two DatabaseTriggers instances (different timezones) + assert mock_db_class.call_count == 2 + + # insert_many should be called with list of 2 + insert_call_args = mock_db_class.insert_many.call_args[0][0] + assert len(insert_call_args) == 2 + + +@pytest.mark.asyncio +async def test_create_crons_trigger_time_is_datetime(): + """Test that trigger_time is set to a datetime object""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": "America/New_York"} + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Verify trigger_time is a datetime object + call_kwargs = mock_db_class.call_args[1] + assert isinstance(call_kwargs['trigger_time'], datetime) \ No newline at end of file From 770eb4e7d3ffacd871655e0d6acfc5f694e1a02a Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sat, 4 Oct 2025 14:03:08 +0530 Subject: [PATCH 03/14] test: add tests for trigger_cron timezone functionality Add unit tests for create_next_triggers function to improve coverage: - Test timezone handling (America/New_York, UTC, Europe/London) - Test None timezone defaults to UTC - Test DuplicateKeyError handling - Test datetime object validation - Test multiple trigger creation These tests cover the new timezone-aware trigger scheduling logic. Signed-off-by: Sparsh --- ...crons_coverage.py => test_create_crons.py} | 0 .../tests/unit/tasks/test_trigger_cron.py | 188 ++++++++++++++++++ 2 files changed, 188 insertions(+) rename state-manager/tests/unit/tasks/{test_create_crons_coverage.py => test_create_crons.py} (100%) create mode 100644 state-manager/tests/unit/tasks/test_trigger_cron.py diff --git a/state-manager/tests/unit/tasks/test_create_crons_coverage.py b/state-manager/tests/unit/tasks/test_create_crons.py similarity index 100% rename from state-manager/tests/unit/tasks/test_create_crons_coverage.py rename to state-manager/tests/unit/tasks/test_create_crons.py diff --git a/state-manager/tests/unit/tasks/test_trigger_cron.py b/state-manager/tests/unit/tasks/test_trigger_cron.py new file mode 100644 index 00000000..6c734a01 --- /dev/null +++ b/state-manager/tests/unit/tasks/test_trigger_cron.py @@ -0,0 +1,188 @@ +""" +Tests for trigger_cron functions to improve code coverage. +These are pure unit tests that mock database operations. +Environment variables are provided by CI (see .github/workflows/test-state-manager.yml). +""" +import pytest +from unittest.mock import MagicMock, AsyncMock, patch +from datetime import datetime + +from app.tasks.trigger_cron import create_next_triggers + + +@pytest.mark.asyncio +async def test_create_next_triggers_with_america_new_york_timezone(): + """Test create_next_triggers processes America/New_York timezone correctly""" + trigger = MagicMock() + trigger.expression = "0 9 * * *" + trigger.timezone = "America/New_York" + trigger.trigger_time = datetime(2025, 10, 4, 13, 0, 0) # Naive UTC time + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Verify DatabaseTriggers was instantiated with timezone + assert mock_db_class.called + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "America/New_York" + assert call_kwargs['expression'] == "0 9 * * *" + + +@pytest.mark.asyncio +async def test_create_next_triggers_with_utc_timezone(): + """Test create_next_triggers with UTC timezone""" + trigger = MagicMock() + trigger.expression = "0 9 * * *" + trigger.timezone = "UTC" + trigger.trigger_time = datetime(2025, 10, 4, 9, 0, 0) + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Verify timezone was passed correctly + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "UTC" + + +@pytest.mark.asyncio +async def test_create_next_triggers_with_none_timezone_defaults_to_utc(): + """Test create_next_triggers with None timezone defaults to UTC""" + trigger = MagicMock() + trigger.expression = "0 9 * * *" + trigger.timezone = None + trigger.trigger_time = datetime(2025, 10, 4, 9, 0, 0) + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Verify None timezone is passed through (will default to UTC in ZoneInfo call) + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] is None + + +@pytest.mark.asyncio +async def test_create_next_triggers_with_europe_london_timezone(): + """Test create_next_triggers with Europe/London timezone""" + trigger = MagicMock() + trigger.expression = "0 17 * * *" + trigger.timezone = "Europe/London" + trigger.trigger_time = datetime(2025, 10, 4, 16, 0, 0) # UTC time + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Verify Europe/London timezone was used + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "Europe/London" + + +@pytest.mark.asyncio +async def test_create_next_triggers_handles_duplicate_key_error(): + """Test create_next_triggers handles DuplicateKeyError gracefully""" + from pymongo.errors import DuplicateKeyError + + trigger = MagicMock() + trigger.expression = "0 9 * * *" + trigger.timezone = "America/New_York" + trigger.trigger_time = datetime(2025, 10, 4, 13, 0, 0) + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + # First call raises DuplicateKeyError, second succeeds + mock_instance.insert = AsyncMock(side_effect=[ + DuplicateKeyError("Duplicate"), + None + ]) + mock_db_class.return_value = mock_instance + + with patch('app.tasks.trigger_cron.logger') as mock_logger: + # Should not raise exception + await create_next_triggers(trigger, cron_time) + + # Verify error was logged + assert mock_logger.error.called + error_msg = mock_logger.error.call_args[0][0] + assert "Duplicate trigger found" in error_msg + + +@pytest.mark.asyncio +async def test_create_next_triggers_trigger_time_is_datetime(): + """Test that next trigger_time is a datetime object""" + trigger = MagicMock() + trigger.expression = "0 9 * * *" + trigger.timezone = "America/New_York" + trigger.trigger_time = datetime(2025, 10, 4, 13, 0, 0) + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 6, 0, 0, 0) + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Verify trigger_time is a datetime + call_kwargs = mock_db_class.call_args[1] + assert isinstance(call_kwargs['trigger_time'], datetime) + + +@pytest.mark.asyncio +async def test_create_next_triggers_creates_multiple_triggers(): + """Test create_next_triggers creates multiple future triggers""" + trigger = MagicMock() + trigger.expression = "0 */6 * * *" # Every 6 hours + trigger.timezone = "UTC" + trigger.trigger_time = datetime(2025, 10, 4, 0, 0, 0) + trigger.graph_name = "test_graph" + trigger.namespace = "test_namespace" + + cron_time = datetime(2025, 10, 5, 0, 0, 0) # 24 hours later + + with patch('app.tasks.trigger_cron.DatabaseTriggers') as mock_db_class: + mock_instance = MagicMock() + mock_instance.insert = AsyncMock() + mock_db_class.return_value = mock_instance + + await create_next_triggers(trigger, cron_time) + + # Should create multiple triggers (every 6 hours until past cron_time) + assert mock_db_class.call_count >= 4 # At least 4 triggers in 24 hours \ No newline at end of file From 4b065e4d50a975983035511c5d6273dd447c5b85 Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sat, 4 Oct 2025 14:32:36 +0530 Subject: [PATCH 04/14] fix: validate timezone through CronTrigger model to prevent None from reaching ZoneInfo When clients send {"timezone": null}, the raw dictionary value is None, which would cause ZoneInfo(None) to raise an exception. Instead, validate each trigger.value through CronTrigger.model_validate() to normalize None to "UTC" via the field validator. This ensures: - None timezone values are normalized to "UTC" before ZoneInfo creation - No None values are persisted to the database - Invalid timezones are caught during validation - All timezone handling uses the validated, normalized value Added test to verify null timezone normalization behavior. Signed-off-by: Sparsh --- state-manager/app/tasks/verify_graph.py | 17 ++++++------ .../tests/unit/models/test_trigger_models.py | 10 ++++--- .../tests/unit/tasks/test_create_crons.py | 26 ++++++++++++++++++- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/state-manager/app/tasks/verify_graph.py b/state-manager/app/tasks/verify_graph.py index 373eea7e..6dc4f11e 100644 --- a/state-manager/app/tasks/verify_graph.py +++ b/state-manager/app/tasks/verify_graph.py @@ -102,19 +102,20 @@ async def verify_inputs(graph_template: GraphTemplate, registered_nodes: list[Re return errors async def create_crons(graph_template: GraphTemplate): - # Build a map of (expression, timezone) -> trigger for deduplication + # Build a map of (expression, timezone) -> validated CronTrigger for deduplication triggers_to_create = {} for trigger in graph_template.triggers: if trigger.type == TriggerTypeEnum.CRON: - expression = trigger.value["expression"] - timezone = trigger.value.get("timezone", "UTC") - triggers_to_create[(expression, timezone)] = trigger + # Validate through CronTrigger model to normalize timezone (None -> "UTC") + from app.models.trigger_models import CronTrigger + cron_trigger = CronTrigger.model_validate(trigger.value) + triggers_to_create[(cron_trigger.expression, cron_trigger.timezone)] = cron_trigger current_time = datetime.now(ZoneInfo("UTC")).replace(tzinfo=None) new_db_triggers = [] - for (expression, timezone), trigger in triggers_to_create.items(): - # Use the trigger's timezone, defaulting to UTC + for (expression, timezone), cron_trigger in triggers_to_create.items(): + # Use the validated timezone (guaranteed to be valid IANA timezone, never None) tz = ZoneInfo(timezone) # Get current time in the specified timezone @@ -130,8 +131,8 @@ async def create_crons(graph_template: GraphTemplate): new_db_triggers.append( DatabaseTriggers( type=TriggerTypeEnum.CRON, - expression=expression, - timezone=timezone, + expression=cron_trigger.expression, + timezone=cron_trigger.timezone, graph_name=graph_template.name, namespace=graph_template.namespace, trigger_status=TriggerStatusEnum.PENDING, diff --git a/state-manager/tests/unit/models/test_trigger_models.py b/state-manager/tests/unit/models/test_trigger_models.py index e214d2a6..c9b5da43 100644 --- a/state-manager/tests/unit/models/test_trigger_models.py +++ b/state-manager/tests/unit/models/test_trigger_models.py @@ -43,7 +43,9 @@ def test_invalid_cron_expression(self): errors = exc_info.value.errors() assert len(errors) == 1 - assert "Invalid cron expression" in str(errors[0]["ctx"]["error"]) + # Check the error message (Pydantic v2 doesn't always populate ctx) + error_msg = errors[0].get("msg") or str(errors[0]) + assert "Invalid cron expression" in error_msg def test_invalid_timezone(self): """Test creating a cron trigger with invalid timezone""" @@ -52,8 +54,10 @@ def test_invalid_timezone(self): errors = exc_info.value.errors() assert len(errors) == 1 - assert "Invalid timezone" in str(errors[0]["ctx"]["error"]) - assert "Invalid/Timezone" in str(errors[0]["ctx"]["error"]) + # Check the error message (Pydantic v2 doesn't always populate ctx) + error_msg = errors[0].get("msg") or str(errors[0]) + assert "Invalid timezone" in error_msg + assert "Invalid/Timezone" in error_msg def test_none_timezone_defaults_to_utc(self): """Test that None timezone defaults to UTC""" diff --git a/state-manager/tests/unit/tasks/test_create_crons.py b/state-manager/tests/unit/tasks/test_create_crons.py index 75f7382d..7097d69f 100644 --- a/state-manager/tests/unit/tasks/test_create_crons.py +++ b/state-manager/tests/unit/tasks/test_create_crons.py @@ -233,4 +233,28 @@ async def test_create_crons_trigger_time_is_datetime(): # Verify trigger_time is a datetime object call_kwargs = mock_db_class.call_args[1] - assert isinstance(call_kwargs['trigger_time'], datetime) \ No newline at end of file + assert isinstance(call_kwargs['trigger_time'], datetime) + +@pytest.mark.asyncio +async def test_create_crons_with_null_timezone_normalizes_to_utc(): + """Test create_crons with null/None timezone normalizes to UTC via model validation""" + graph_template = MagicMock() + graph_template.name = "test_graph" + graph_template.namespace = "test_ns" + graph_template.triggers = [ + Trigger( + type=TriggerTypeEnum.CRON, + value={"expression": "0 9 * * *", "timezone": None} # Explicit None + ) + ] + + with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: + mock_db_class.return_value = MagicMock() + mock_db_class.insert_many = AsyncMock() + + await create_crons(graph_template) + + # Verify timezone was normalized to "UTC" (not None) + call_kwargs = mock_db_class.call_args[1] + assert call_kwargs['timezone'] == "UTC" + assert call_kwargs['timezone'] is not None From 9d553228da69082b54f72a5353e33a84222beff0 Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sat, 4 Oct 2025 15:27:18 +0530 Subject: [PATCH 05/14] perf: cache timezone objects at module level for better performance Cache frequently instantiated timezone objects (UTC and available timezones) at module level to avoid repeated overhead: - Cache available_timezones() set in trigger_models.py to prevent repeated filesystem queries during timezone validation - Cache ZoneInfo("UTC") constant in trigger_cron.py and verify_graph.py to avoid repeated ZoneInfo instantiation in hot paths These optimizations reduce unnecessary overhead in functions that are called frequently, particularly in cron trigger scheduling operations. Signed-off-by: Sparsh --- state-manager/app/models/trigger_models.py | 5 ++++- state-manager/app/tasks/trigger_cron.py | 7 +++++-- state-manager/app/tasks/verify_graph.py | 9 ++++++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index 8ce31f4e..3594e410 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -4,6 +4,9 @@ from typing import Self, Optional from zoneinfo import available_timezones +# Cache available timezones at module level to avoid repeated filesystem queries +_AVAILABLE_TIMEZONES = available_timezones() + class TriggerTypeEnum(str, Enum): CRON = "CRON" @@ -30,7 +33,7 @@ def validate_expression(cls, v: str) -> str: def validate_timezone(cls, v: Optional[str]) -> str: if v is None: return "UTC" - if v not in available_timezones(): + if v not in _AVAILABLE_TIMEZONES: raise ValueError(f"Invalid timezone: {v}. Must be a valid IANA timezone (e.g., 'America/New_York', 'Europe/London', 'UTC')") return v diff --git a/state-manager/app/tasks/trigger_cron.py b/state-manager/app/tasks/trigger_cron.py index acf9552e..53c6f7d4 100644 --- a/state-manager/app/tasks/trigger_cron.py +++ b/state-manager/app/tasks/trigger_cron.py @@ -12,6 +12,9 @@ import croniter import asyncio +# Cache UTC timezone at module level to avoid repeated instantiation +UTC = ZoneInfo("UTC") + logger = LogsManager().get_logger() async def get_due_triggers(cron_time: datetime) -> DatabaseTriggers | None: @@ -48,7 +51,7 @@ async def create_next_triggers(trigger: DatabaseTriggers, cron_time: datetime): tz = ZoneInfo(trigger.timezone or "UTC") # Convert trigger_time to the specified timezone for croniter - trigger_time_tz = trigger.trigger_time.replace(tzinfo=ZoneInfo("UTC")).astimezone(tz) + trigger_time_tz = trigger.trigger_time.replace(tzinfo=UTC).astimezone(tz) iter = croniter.croniter(trigger.expression, trigger_time_tz) while True: @@ -56,7 +59,7 @@ async def create_next_triggers(trigger: DatabaseTriggers, cron_time: datetime): next_trigger_time_tz = iter.get_next(datetime) # Convert back to UTC for storage - next_trigger_time = next_trigger_time_tz.astimezone(ZoneInfo("UTC")).replace(tzinfo=None) + next_trigger_time = next_trigger_time_tz.astimezone(UTC).replace(tzinfo=None) try: await DatabaseTriggers( diff --git a/state-manager/app/tasks/verify_graph.py b/state-manager/app/tasks/verify_graph.py index 6dc4f11e..41476073 100644 --- a/state-manager/app/tasks/verify_graph.py +++ b/state-manager/app/tasks/verify_graph.py @@ -12,6 +12,9 @@ from app.models.trigger_models import TriggerStatusEnum, TriggerTypeEnum from app.models.db.trigger import DatabaseTriggers +# Cache UTC timezone at module level to avoid repeated instantiation +UTC = ZoneInfo("UTC") + logger = LogsManager().get_logger() async def verify_node_exists(graph_template: GraphTemplate, registered_nodes: list[RegisteredNode]) -> list[str]: @@ -111,7 +114,7 @@ async def create_crons(graph_template: GraphTemplate): cron_trigger = CronTrigger.model_validate(trigger.value) triggers_to_create[(cron_trigger.expression, cron_trigger.timezone)] = cron_trigger - current_time = datetime.now(ZoneInfo("UTC")).replace(tzinfo=None) + current_time = datetime.now(UTC).replace(tzinfo=None) new_db_triggers = [] for (expression, timezone), cron_trigger in triggers_to_create.items(): @@ -119,14 +122,14 @@ async def create_crons(graph_template: GraphTemplate): tz = ZoneInfo(timezone) # Get current time in the specified timezone - current_time_tz = current_time.replace(tzinfo=ZoneInfo("UTC")).astimezone(tz) + current_time_tz = current_time.replace(tzinfo=UTC).astimezone(tz) iter = croniter.croniter(expression, current_time_tz) # Get next trigger time in the specified timezone next_trigger_time_tz = iter.get_next(datetime) # Convert back to UTC for storage (remove timezone info for storage) - next_trigger_time = next_trigger_time_tz.astimezone(ZoneInfo("UTC")).replace(tzinfo=None) + next_trigger_time = next_trigger_time_tz.astimezone(UTC).replace(tzinfo=None) new_db_triggers.append( DatabaseTriggers( From 421c4bf36591860733845649c16a7a19fcf3287e Mon Sep 17 00:00:00 2001 From: Sparsh Date: Sun, 5 Oct 2025 14:08:39 +0530 Subject: [PATCH 06/14] refactor: use discriminated unions for Trigger model to eliminate redundant validation Changes: - Refactored Trigger.value from dict to typed TriggerValue union - Added discriminated union support using Literal type for CronTrigger.type - Removed redundant CronTrigger.model_validate() call in verify_graph.py - Updated all tests to include type field in trigger value dicts Benefits: - Single validation at request time (no re-validation needed) - Type safety with IDE autocomplete and static analysis - Cleaner code: trigger.value.expression vs trigger.value["expression"] - Extensible: easy to add new trigger types to the union - All 38 trigger-related tests passing Also bumped python-sdk version to 0.0.3b2 --- python-sdk/exospherehost/_version.py | 2 +- state-manager/app/models/trigger_models.py | 25 +++++++++------- state-manager/app/tasks/verify_graph.py | 7 ++--- .../tests/unit/models/test_trigger_models.py | 15 +++++----- .../tests/unit/tasks/test_create_crons.py | 30 ++++++++++++------- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/python-sdk/exospherehost/_version.py b/python-sdk/exospherehost/_version.py index 77621c99..5d12bc38 100644 --- a/python-sdk/exospherehost/_version.py +++ b/python-sdk/exospherehost/_version.py @@ -1 +1 @@ -version = "0.0.3b1" \ No newline at end of file +version = "0.0.3b2" \ No newline at end of file diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index 3594e410..55fbd510 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -1,7 +1,7 @@ -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import BaseModel, Field, field_validator from enum import Enum from croniter import croniter -from typing import Self, Optional +from typing import Union, Annotated, Optional, Literal from zoneinfo import available_timezones # Cache available timezones at module level to avoid repeated filesystem queries @@ -18,6 +18,7 @@ class TriggerStatusEnum(str, Enum): TRIGGERING = "TRIGGERING" class CronTrigger(BaseModel): + type: Literal[TriggerTypeEnum.CRON] = Field(default=TriggerTypeEnum.CRON, description="Type of the trigger") expression: str = Field(..., description="Cron expression for the trigger") timezone: Optional[str] = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC')") @@ -37,14 +38,16 @@ def validate_timezone(cls, v: Optional[str]) -> str: raise ValueError(f"Invalid timezone: {v}. Must be a valid IANA timezone (e.g., 'America/New_York', 'Europe/London', 'UTC')") return v +# Union type for all trigger types - add new trigger types here +TriggerValue = Annotated[Union[CronTrigger], Field(discriminator="type")] + class Trigger(BaseModel): + """ + Extensible trigger model using discriminated unions. + To add a new trigger type: + 1. Add the enum value to TriggerTypeEnum + 2. Create a new trigger class (e.g., WebhookTrigger) with type field + 3. Add it to the TriggerValue Union + """ type: TriggerTypeEnum = Field(..., description="Type of the trigger") - value: dict = Field(default_factory=dict, description="Value of the trigger") - - @model_validator(mode="after") - def validate_trigger(self) -> Self: - if self.type == TriggerTypeEnum.CRON: - CronTrigger.model_validate(self.value) - else: - raise ValueError(f"Unsupported trigger type: {self.type}") - return self \ No newline at end of file + value: TriggerValue = Field(..., description="Value of the trigger") \ No newline at end of file diff --git a/state-manager/app/tasks/verify_graph.py b/state-manager/app/tasks/verify_graph.py index 41476073..498f3756 100644 --- a/state-manager/app/tasks/verify_graph.py +++ b/state-manager/app/tasks/verify_graph.py @@ -105,13 +105,12 @@ async def verify_inputs(graph_template: GraphTemplate, registered_nodes: list[Re return errors async def create_crons(graph_template: GraphTemplate): - # Build a map of (expression, timezone) -> validated CronTrigger for deduplication + # Build a map of (expression, timezone) -> CronTrigger for deduplication triggers_to_create = {} for trigger in graph_template.triggers: if trigger.type == TriggerTypeEnum.CRON: - # Validate through CronTrigger model to normalize timezone (None -> "UTC") - from app.models.trigger_models import CronTrigger - cron_trigger = CronTrigger.model_validate(trigger.value) + # trigger.value is already a validated CronTrigger instance + cron_trigger = trigger.value triggers_to_create[(cron_trigger.expression, cron_trigger.timezone)] = cron_trigger current_time = datetime.now(UTC).replace(tzinfo=None) diff --git a/state-manager/tests/unit/models/test_trigger_models.py b/state-manager/tests/unit/models/test_trigger_models.py index c9b5da43..f5a049f6 100644 --- a/state-manager/tests/unit/models/test_trigger_models.py +++ b/state-manager/tests/unit/models/test_trigger_models.py @@ -84,27 +84,28 @@ def test_valid_trigger_with_cron_and_timezone(self): """Test creating a valid trigger with CRON type and timezone""" trigger = Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} ) assert trigger.type == TriggerTypeEnum.CRON - assert trigger.value["expression"] == "0 9 * * *" - assert trigger.value["timezone"] == "America/New_York" + assert trigger.value.expression == "0 9 * * *" + assert trigger.value.timezone == "America/New_York" def test_valid_trigger_with_cron_without_timezone(self): """Test creating a valid trigger with CRON type without timezone""" trigger = Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *"} + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *"} ) assert trigger.type == TriggerTypeEnum.CRON - assert trigger.value["expression"] == "0 9 * * *" + assert trigger.value.expression == "0 9 * * *" + assert trigger.value.timezone == "UTC" # Should default to UTC def test_invalid_trigger_with_invalid_cron_expression(self): """Test creating a trigger with invalid cron expression""" with pytest.raises(ValidationError) as exc_info: Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "invalid cron"} + value={"type": TriggerTypeEnum.CRON, "expression": "invalid cron"} ) errors = exc_info.value.errors() @@ -115,7 +116,7 @@ def test_invalid_trigger_with_invalid_timezone(self): with pytest.raises(ValidationError) as exc_info: Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "Invalid/Zone"} + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Invalid/Zone"} ) errors = exc_info.value.errors() diff --git a/state-manager/tests/unit/tasks/test_create_crons.py b/state-manager/tests/unit/tasks/test_create_crons.py index 7097d69f..92c6a294 100644 --- a/state-manager/tests/unit/tasks/test_create_crons.py +++ b/state-manager/tests/unit/tasks/test_create_crons.py @@ -14,6 +14,14 @@ from app.tasks.verify_graph import create_crons from app.models.trigger_models import Trigger, TriggerTypeEnum +# Helper function to create trigger value dict +def make_trigger_value(expression: str, timezone: str = None): + """Helper to create trigger value dict with required 'type' field""" + result = {"type": TriggerTypeEnum.CRON, "expression": expression} + if timezone is not None: + result["timezone"] = timezone + return result + @pytest.mark.asyncio async def test_create_crons_with_america_new_york_timezone(): @@ -24,7 +32,7 @@ async def test_create_crons_with_america_new_york_timezone(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -58,7 +66,7 @@ async def test_create_crons_with_default_utc_timezone(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *"} # No timezone specified + value=make_trigger_value("0 9 * * *") # No timezone specified ) ] @@ -83,7 +91,7 @@ async def test_create_crons_with_europe_london_timezone(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 17 * * *", "timezone": "Europe/London"} + value=make_trigger_value("0 17 * * *", "Europe/London") ) ] @@ -108,11 +116,11 @@ async def test_create_crons_with_multiple_different_timezones(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 17 * * *", "timezone": "Europe/London"} + value=make_trigger_value("0 17 * * *", "Europe/London") ) ] @@ -159,11 +167,11 @@ async def test_create_crons_deduplicates_same_expression_and_timezone(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -190,11 +198,11 @@ async def test_create_crons_keeps_same_expression_different_timezones(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "Europe/London"} + value=make_trigger_value("0 9 * * *", "Europe/London") ) ] @@ -221,7 +229,7 @@ async def test_create_crons_trigger_time_is_datetime(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": "America/New_York"} + value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -244,7 +252,7 @@ async def test_create_crons_with_null_timezone_normalizes_to_utc(): graph_template.triggers = [ Trigger( type=TriggerTypeEnum.CRON, - value={"expression": "0 9 * * *", "timezone": None} # Explicit None + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": None} # Explicit None ) ] From 077ca505a62eafee62ac0bc61ffa2285c6d1c0aa Mon Sep 17 00:00:00 2001 From: Sparsh Date: Mon, 6 Oct 2025 13:11:07 +0530 Subject: [PATCH 07/14] refactor: remove redundant type field from Trigger model Remove the duplicate type field from the Trigger model to eliminate redundancy with the discriminated union's type field in CronTrigger. The type is now accessed via trigger.value.type instead of trigger.type. Changes: - Remove type field from Trigger model - Update trigger.type references to trigger.value.type in verify_graph.py and upsert_graph_template.py - Update tests to use new API and add tests for duplicate trigger handling - Add docstring note about accessing type via trigger.value.type This simplifies the Trigger model API while maintaining full functionality through the discriminated union pattern. Signed-off-by: Sparsh --- .../app/controller/upsert_graph_template.py | 2 +- state-manager/app/models/trigger_models.py | 3 +- state-manager/app/tasks/verify_graph.py | 2 +- .../tests/unit/models/test_trigger_models.py | 60 ++++++++++++++++--- 4 files changed, 57 insertions(+), 10 deletions(-) diff --git a/state-manager/app/controller/upsert_graph_template.py b/state-manager/app/controller/upsert_graph_template.py index 72e70e29..125bae53 100644 --- a/state-manager/app/controller/upsert_graph_template.py +++ b/state-manager/app/controller/upsert_graph_template.py @@ -66,7 +66,7 @@ async def upsert_graph_template(namespace_name: str, graph_name: str, body: Upse DatabaseTriggers.graph_name == graph_name, DatabaseTriggers.trigger_status == TriggerStatusEnum.PENDING, DatabaseTriggers.type == TriggerTypeEnum.CRON, - In(DatabaseTriggers.expression, [trigger.value["expression"] for trigger in old_triggers if trigger.type == TriggerTypeEnum.CRON]) + In(DatabaseTriggers.expression, [trigger.value["expression"] for trigger in old_triggers if trigger.value.type == TriggerTypeEnum.CRON]) ).delete_many() background_tasks.add_task(verify_graph, graph_template) diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index 55fbd510..4fea2468 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -48,6 +48,7 @@ class Trigger(BaseModel): 1. Add the enum value to TriggerTypeEnum 2. Create a new trigger class (e.g., WebhookTrigger) with type field 3. Add it to the TriggerValue Union + + Note: Access trigger type via trigger.value.type """ - type: TriggerTypeEnum = Field(..., description="Type of the trigger") value: TriggerValue = Field(..., description="Value of the trigger") \ No newline at end of file diff --git a/state-manager/app/tasks/verify_graph.py b/state-manager/app/tasks/verify_graph.py index 498f3756..8e8fd411 100644 --- a/state-manager/app/tasks/verify_graph.py +++ b/state-manager/app/tasks/verify_graph.py @@ -108,7 +108,7 @@ async def create_crons(graph_template: GraphTemplate): # Build a map of (expression, timezone) -> CronTrigger for deduplication triggers_to_create = {} for trigger in graph_template.triggers: - if trigger.type == TriggerTypeEnum.CRON: + if trigger.value.type == TriggerTypeEnum.CRON: # trigger.value is already a validated CronTrigger instance cron_trigger = trigger.value triggers_to_create[(cron_trigger.expression, cron_trigger.timezone)] = cron_trigger diff --git a/state-manager/tests/unit/models/test_trigger_models.py b/state-manager/tests/unit/models/test_trigger_models.py index f5a049f6..99369824 100644 --- a/state-manager/tests/unit/models/test_trigger_models.py +++ b/state-manager/tests/unit/models/test_trigger_models.py @@ -83,20 +83,18 @@ class TestTrigger: def test_valid_trigger_with_cron_and_timezone(self): """Test creating a valid trigger with CRON type and timezone""" trigger = Trigger( - type=TriggerTypeEnum.CRON, value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} ) - assert trigger.type == TriggerTypeEnum.CRON + assert trigger.value.type == TriggerTypeEnum.CRON assert trigger.value.expression == "0 9 * * *" assert trigger.value.timezone == "America/New_York" def test_valid_trigger_with_cron_without_timezone(self): """Test creating a valid trigger with CRON type without timezone""" trigger = Trigger( - type=TriggerTypeEnum.CRON, value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *"} ) - assert trigger.type == TriggerTypeEnum.CRON + assert trigger.value.type == TriggerTypeEnum.CRON assert trigger.value.expression == "0 9 * * *" assert trigger.value.timezone == "UTC" # Should default to UTC @@ -104,7 +102,6 @@ def test_invalid_trigger_with_invalid_cron_expression(self): """Test creating a trigger with invalid cron expression""" with pytest.raises(ValidationError) as exc_info: Trigger( - type=TriggerTypeEnum.CRON, value={"type": TriggerTypeEnum.CRON, "expression": "invalid cron"} ) @@ -115,9 +112,58 @@ def test_invalid_trigger_with_invalid_timezone(self): """Test creating a trigger with invalid timezone""" with pytest.raises(ValidationError) as exc_info: Trigger( - type=TriggerTypeEnum.CRON, value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Invalid/Zone"} ) errors = exc_info.value.errors() - assert len(errors) > 0 \ No newline at end of file + assert len(errors) > 0 + + def test_duplicate_triggers_with_identical_values(self): + """Test creating two triggers with identical values""" + # Create first trigger + trigger1 = Trigger( + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} + ) + + # Create second trigger with identical values + trigger2 = Trigger( + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} + ) + + # Both triggers should be created successfully + assert trigger1.value.type == TriggerTypeEnum.CRON + assert trigger2.value.type == TriggerTypeEnum.CRON + assert trigger1.value.expression == "0 9 * * *" + assert trigger2.value.expression == "0 9 * * *" + assert trigger1.value.timezone == "America/New_York" + assert trigger2.value.timezone == "America/New_York" + + # Test equality - Pydantic models with same values should be equal + assert trigger1 == trigger2 + + # Test that they are different objects in memory + assert trigger1 is not trigger2 + + # Test model_dump produces identical output + assert trigger1.model_dump() == trigger2.model_dump() + + def test_duplicate_triggers_different_timezones(self): + """Test creating two triggers with same expression but different timezones""" + trigger1 = Trigger( + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} + ) + + trigger2 = Trigger( + value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Europe/London"} + ) + + # Both triggers should be created successfully + assert trigger1.value.type == TriggerTypeEnum.CRON + assert trigger2.value.type == TriggerTypeEnum.CRON + + # They should NOT be equal due to different timezones + assert trigger1 != trigger2 + + # Verify the difference is in timezone + assert trigger1.value.timezone == "America/New_York" + assert trigger2.value.timezone == "Europe/London" \ No newline at end of file From 8142f52e568af4807f873fdf21d37d81451988fd Mon Sep 17 00:00:00 2001 From: Sparsh Date: Mon, 6 Oct 2025 13:30:59 +0530 Subject: [PATCH 08/14] refactor: simplify CronTrigger timezone field and add deduplication tests Change timezone field from Optional[str] to str since the default="UTC" ensures it's never None at runtime. This makes the type annotation accurately reflect the actual behavior. Changes: - Change CronTrigger.timezone from Optional[str] to str - Remove None check from timezone validator (Pydantic default handles it) - Remove Optional import (no longer needed) - Rename test_duplicate_triggers_with_identical_values to test_trigger_model_equality_with_identical_values to clarify it tests Pydantic model equality, not business logic deduplication - Remove test_none_timezone_defaults_to_utc (timezone=None no longer valid) - Add integration tests for trigger deduplication in create_crons function - test_create_crons_deduplicates_identical_triggers - test_create_crons_keeps_triggers_with_different_timezones - test_create_crons_keeps_triggers_with_different_expressions - test_create_crons_complex_deduplication_scenario The integration tests verify actual database behavior to ensure the deduplication logic in verify_graph.py correctly creates only one DatabaseTriggers row per (expression, timezone) pair. Signed-off-by: Sparsh --- state-manager/app/models/trigger_models.py | 8 +- .../tests/unit/models/test_trigger_models.py | 13 +- .../tests/unit/tasks/test_create_crons.py | 33 --- .../test_trigger_deduplication.py | 265 ++++++++++++++++++ 4 files changed, 274 insertions(+), 45 deletions(-) create mode 100644 state-manager/tests/unit/with_database/test_trigger_deduplication.py diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index 4fea2468..a31693b7 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -1,7 +1,7 @@ from pydantic import BaseModel, Field, field_validator from enum import Enum from croniter import croniter -from typing import Union, Annotated, Optional, Literal +from typing import Union, Annotated, Literal from zoneinfo import available_timezones # Cache available timezones at module level to avoid repeated filesystem queries @@ -20,7 +20,7 @@ class TriggerStatusEnum(str, Enum): class CronTrigger(BaseModel): type: Literal[TriggerTypeEnum.CRON] = Field(default=TriggerTypeEnum.CRON, description="Type of the trigger") expression: str = Field(..., description="Cron expression for the trigger") - timezone: Optional[str] = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC')") + timezone: str = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC')") @field_validator("expression") @classmethod @@ -31,9 +31,7 @@ def validate_expression(cls, v: str) -> str: @field_validator("timezone") @classmethod - def validate_timezone(cls, v: Optional[str]) -> str: - if v is None: - return "UTC" + def validate_timezone(cls, v: str) -> str: if v not in _AVAILABLE_TIMEZONES: raise ValueError(f"Invalid timezone: {v}. Must be a valid IANA timezone (e.g., 'America/New_York', 'Europe/London', 'UTC')") return v diff --git a/state-manager/tests/unit/models/test_trigger_models.py b/state-manager/tests/unit/models/test_trigger_models.py index 99369824..293b9c8b 100644 --- a/state-manager/tests/unit/models/test_trigger_models.py +++ b/state-manager/tests/unit/models/test_trigger_models.py @@ -59,11 +59,6 @@ def test_invalid_timezone(self): assert "Invalid timezone" in error_msg assert "Invalid/Timezone" in error_msg - def test_none_timezone_defaults_to_utc(self): - """Test that None timezone defaults to UTC""" - trigger = CronTrigger(expression="0 9 * * *", timezone=None) - assert trigger.timezone == "UTC" - def test_complex_cron_expression_with_timezone(self): """Test complex cron expression with timezone""" trigger = CronTrigger(expression="0 0 1,15 * *", timezone="America/Los_Angeles") @@ -118,8 +113,12 @@ def test_invalid_trigger_with_invalid_timezone(self): errors = exc_info.value.errors() assert len(errors) > 0 - def test_duplicate_triggers_with_identical_values(self): - """Test creating two triggers with identical values""" + def test_trigger_model_equality_with_identical_values(self): + """Test Pydantic model equality for triggers with identical values. + + Note: This test verifies model-level equality only, not deduplication logic. + The actual deduplication of triggers is handled in verify_graph.py's create_crons function. + """ # Create first trigger trigger1 = Trigger( value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"} diff --git a/state-manager/tests/unit/tasks/test_create_crons.py b/state-manager/tests/unit/tasks/test_create_crons.py index 92c6a294..1ec136df 100644 --- a/state-manager/tests/unit/tasks/test_create_crons.py +++ b/state-manager/tests/unit/tasks/test_create_crons.py @@ -31,7 +31,6 @@ async def test_create_crons_with_america_new_york_timezone(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -65,7 +64,6 @@ async def test_create_crons_with_default_utc_timezone(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *") # No timezone specified ) ] @@ -90,7 +88,6 @@ async def test_create_crons_with_europe_london_timezone(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 17 * * *", "Europe/London") ) ] @@ -115,11 +112,9 @@ async def test_create_crons_with_multiple_different_timezones(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 17 * * *", "Europe/London") ) ] @@ -166,11 +161,9 @@ async def test_create_crons_deduplicates_same_expression_and_timezone(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -197,11 +190,9 @@ async def test_create_crons_keeps_same_expression_different_timezones(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ), Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "Europe/London") ) ] @@ -228,7 +219,6 @@ async def test_create_crons_trigger_time_is_datetime(): graph_template.namespace = "test_ns" graph_template.triggers = [ Trigger( - type=TriggerTypeEnum.CRON, value=make_trigger_value("0 9 * * *", "America/New_York") ) ] @@ -243,26 +233,3 @@ async def test_create_crons_trigger_time_is_datetime(): call_kwargs = mock_db_class.call_args[1] assert isinstance(call_kwargs['trigger_time'], datetime) -@pytest.mark.asyncio -async def test_create_crons_with_null_timezone_normalizes_to_utc(): - """Test create_crons with null/None timezone normalizes to UTC via model validation""" - graph_template = MagicMock() - graph_template.name = "test_graph" - graph_template.namespace = "test_ns" - graph_template.triggers = [ - Trigger( - type=TriggerTypeEnum.CRON, - value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": None} # Explicit None - ) - ] - - with patch('app.tasks.verify_graph.DatabaseTriggers') as mock_db_class: - mock_db_class.return_value = MagicMock() - mock_db_class.insert_many = AsyncMock() - - await create_crons(graph_template) - - # Verify timezone was normalized to "UTC" (not None) - call_kwargs = mock_db_class.call_args[1] - assert call_kwargs['timezone'] == "UTC" - assert call_kwargs['timezone'] is not None diff --git a/state-manager/tests/unit/with_database/test_trigger_deduplication.py b/state-manager/tests/unit/with_database/test_trigger_deduplication.py new file mode 100644 index 00000000..9a6afe51 --- /dev/null +++ b/state-manager/tests/unit/with_database/test_trigger_deduplication.py @@ -0,0 +1,265 @@ +""" +Integration tests for trigger deduplication logic in create_crons. +These tests verify that duplicate triggers with identical expression and timezone +result in only one DatabaseTriggers row being created. +""" +import pytest + +from app.models.db.graph_template_model import GraphTemplate +from app.models.graph_template_validation_status import GraphTemplateValidationStatus +from app.models.node_template_model import NodeTemplate +from app.models.trigger_models import Trigger, TriggerTypeEnum +from app.models.db.trigger import DatabaseTriggers +from app.tasks.verify_graph import create_crons + + +@pytest.mark.asyncio +async def test_create_crons_deduplicates_identical_triggers(app_started): + """Test that create_crons deduplicates triggers with identical expression and timezone + + This integration test verifies the actual database behavior, not mocks. + It creates a GraphTemplate with duplicate CRON triggers and verifies that only + one DatabaseTriggers row exists per (expression, timezone) pair. + """ + # Clean up any existing triggers for this test + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_dedup_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + # Create a graph template with duplicate triggers + graph_template = GraphTemplate( + name="test_dedup_graph", + namespace="test_namespace", + nodes=[ + NodeTemplate( + node_name="test_node", + namespace="test_namespace", + identifier="test_node", + inputs={}, + next_nodes=None, + unites=None + ) + ], + validation_status=GraphTemplateValidationStatus.PENDING, + triggers=[ + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + ] + ) + + # Call create_crons + await create_crons(graph_template) + + # Query the database to verify only one trigger was created + triggers = await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_dedup_graph", + DatabaseTriggers.namespace == "test_namespace", + DatabaseTriggers.expression == "0 9 * * *", + DatabaseTriggers.timezone == "America/New_York" + ).to_list() + + # Assert only one trigger exists (deduplication worked) + assert len(triggers) == 1 + assert triggers[0].expression == "0 9 * * *" + assert triggers[0].timezone == "America/New_York" + + # Clean up + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_dedup_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + +@pytest.mark.asyncio +async def test_create_crons_keeps_triggers_with_different_timezones(app_started): + """Test that create_crons keeps triggers with same expression but different timezones + + This verifies that triggers are only deduplicated when BOTH expression AND timezone match. + """ + # Clean up any existing triggers for this test + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_timezone_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + # Create a graph template with same expression, different timezones + graph_template = GraphTemplate( + name="test_timezone_graph", + namespace="test_namespace", + nodes=[ + NodeTemplate( + node_name="test_node", + namespace="test_namespace", + identifier="test_node", + inputs={}, + next_nodes=None, + unites=None + ) + ], + validation_status=GraphTemplateValidationStatus.PENDING, + triggers=[ + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Europe/London"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Asia/Tokyo"}), + ] + ) + + # Call create_crons + await create_crons(graph_template) + + # Query the database to verify all three triggers were created (different timezones) + triggers = await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_timezone_graph", + DatabaseTriggers.namespace == "test_namespace", + DatabaseTriggers.expression == "0 9 * * *" + ).to_list() + + # Assert three triggers exist (one for each timezone) + assert len(triggers) == 3 + + timezones = {t.timezone for t in triggers} + assert timezones == {"America/New_York", "Europe/London", "Asia/Tokyo"} + + # Clean up + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_timezone_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + +@pytest.mark.asyncio +async def test_create_crons_keeps_triggers_with_different_expressions(app_started): + """Test that create_crons keeps triggers with different expressions + + This verifies basic functionality - triggers with different expressions should all be created. + """ + # Clean up any existing triggers for this test + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_expr_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + # Create a graph template with different expressions + graph_template = GraphTemplate( + name="test_expr_graph", + namespace="test_namespace", + nodes=[ + NodeTemplate( + node_name="test_node", + namespace="test_namespace", + identifier="test_node", + inputs={}, + next_nodes=None, + unites=None + ) + ], + validation_status=GraphTemplateValidationStatus.PENDING, + triggers=[ + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "UTC"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 12 * * *", "timezone": "UTC"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), + ] + ) + + # Call create_crons + await create_crons(graph_template) + + # Query the database to verify all three triggers were created + triggers = await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_expr_graph", + DatabaseTriggers.namespace == "test_namespace" + ).to_list() + + # Assert three triggers exist (all different expressions) + assert len(triggers) == 3 + + expressions = {t.expression for t in triggers} + assert expressions == {"0 9 * * *", "0 12 * * *", "*/15 * * * *"} + + # Clean up + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_expr_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + +@pytest.mark.asyncio +async def test_create_crons_complex_deduplication_scenario(app_started): + """Test complex deduplication scenario with mix of duplicates and unique triggers + + This tests a realistic scenario where a graph template has: + - Some duplicate triggers (same expression + timezone) + - Some unique triggers + - Triggers with same expression but different timezone + """ + # Clean up any existing triggers for this test + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_complex_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() + + # Create a graph template with a complex mix of triggers + graph_template = GraphTemplate( + name="test_complex_graph", + namespace="test_namespace", + nodes=[ + NodeTemplate( + node_name="test_node", + namespace="test_namespace", + identifier="test_node", + inputs={}, + next_nodes=None, + unites=None + ) + ], + validation_status=GraphTemplateValidationStatus.PENDING, + triggers=[ + # Three duplicates - should result in 1 DB row + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + + # Same expression, different timezone - should result in 1 DB row + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Europe/London"}), + + # Two duplicates of a different expression - should result in 1 DB row + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), + + # Unique trigger - should result in 1 DB row + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 0 1 * *", "timezone": "Asia/Tokyo"}), + ] + ) + + # Call create_crons + await create_crons(graph_template) + + # Query the database to verify correct number of triggers + triggers = await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_complex_graph", + DatabaseTriggers.namespace == "test_namespace" + ).to_list() + + # Expected: 4 unique (expression, timezone) pairs + # 1. (0 9 * * *, America/New_York) + # 2. (0 9 * * *, Europe/London) + # 3. (*/15 * * * *, UTC) + # 4. (0 0 1 * *, Asia/Tokyo) + assert len(triggers) == 4 + + # Verify each unique pair exists + trigger_pairs = {(t.expression, t.timezone) for t in triggers} + assert trigger_pairs == { + ("0 9 * * *", "America/New_York"), + ("0 9 * * *", "Europe/London"), + ("*/15 * * * *", "UTC"), + ("0 0 1 * *", "Asia/Tokyo"), + } + + # Clean up + await DatabaseTriggers.find( + DatabaseTriggers.graph_name == "test_complex_graph", + DatabaseTriggers.namespace == "test_namespace" + ).delete_many() \ No newline at end of file From b425b3152d78c92e51f97a63c868ba0226af7c5c Mon Sep 17 00:00:00 2001 From: Sparsh Date: Mon, 6 Oct 2025 13:47:14 +0530 Subject: [PATCH 09/14] fix: add loop_scope="session" to integration tests to fix AsyncMongoClient event loop error Add loop_scope="session" to all test decorators in test_trigger_deduplication.py to ensure tests run in the same event loop as the session-scoped app_started fixture. This fixes "Cannot use AsyncMongoClient in different event loop" errors. Signed-off-by: Sparsh --- .../unit/with_database/test_trigger_deduplication.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/state-manager/tests/unit/with_database/test_trigger_deduplication.py b/state-manager/tests/unit/with_database/test_trigger_deduplication.py index 9a6afe51..fcf0cfba 100644 --- a/state-manager/tests/unit/with_database/test_trigger_deduplication.py +++ b/state-manager/tests/unit/with_database/test_trigger_deduplication.py @@ -13,7 +13,7 @@ from app.tasks.verify_graph import create_crons -@pytest.mark.asyncio +@pytest.mark.asyncio(loop_scope="session") async def test_create_crons_deduplicates_identical_triggers(app_started): """Test that create_crons deduplicates triggers with identical expression and timezone @@ -72,7 +72,7 @@ async def test_create_crons_deduplicates_identical_triggers(app_started): ).delete_many() -@pytest.mark.asyncio +@pytest.mark.asyncio(loop_scope="session") async def test_create_crons_keeps_triggers_with_different_timezones(app_started): """Test that create_crons keeps triggers with same expression but different timezones @@ -129,7 +129,7 @@ async def test_create_crons_keeps_triggers_with_different_timezones(app_started) ).delete_many() -@pytest.mark.asyncio +@pytest.mark.asyncio(loop_scope="session") async def test_create_crons_keeps_triggers_with_different_expressions(app_started): """Test that create_crons keeps triggers with different expressions @@ -185,7 +185,7 @@ async def test_create_crons_keeps_triggers_with_different_expressions(app_starte ).delete_many() -@pytest.mark.asyncio +@pytest.mark.asyncio(loop_scope="session") async def test_create_crons_complex_deduplication_scenario(app_started): """Test complex deduplication scenario with mix of duplicates and unique triggers From 530b13651e8750c6d316588ba1ce024fc7b4d2a8 Mon Sep 17 00:00:00 2001 From: Sparsh Date: Mon, 6 Oct 2025 14:50:43 +0530 Subject: [PATCH 10/14] test: extract helper function to eliminate duplication in trigger deduplication tests Extract run_deduplication_test helper to handle common setup, teardown, and assertion logic across all 4 integration tests, reducing code from 265 to 176 lines while maintaining identical test behavior. Signed-off-by: Sparsh --- .../test_trigger_deduplication.py | 261 ++++++------------ 1 file changed, 86 insertions(+), 175 deletions(-) diff --git a/state-manager/tests/unit/with_database/test_trigger_deduplication.py b/state-manager/tests/unit/with_database/test_trigger_deduplication.py index fcf0cfba..8343957a 100644 --- a/state-manager/tests/unit/with_database/test_trigger_deduplication.py +++ b/state-manager/tests/unit/with_database/test_trigger_deduplication.py @@ -4,6 +4,7 @@ result in only one DatabaseTriggers row being created. """ import pytest +from typing import List, Optional, Set, Tuple from app.models.db.graph_template_model import GraphTemplate from app.models.graph_template_validation_status import GraphTemplateValidationStatus @@ -13,28 +14,36 @@ from app.tasks.verify_graph import create_crons -@pytest.mark.asyncio(loop_scope="session") -async def test_create_crons_deduplicates_identical_triggers(app_started): - """Test that create_crons deduplicates triggers with identical expression and timezone +async def run_deduplication_test( + graph_name: str, + triggers: List[Trigger], + expected_count: int, + expected_pairs: Optional[Set[Tuple[str, str]]] = None +): + """Helper to test trigger deduplication with actual database. - This integration test verifies the actual database behavior, not mocks. - It creates a GraphTemplate with duplicate CRON triggers and verifies that only - one DatabaseTriggers row exists per (expression, timezone) pair. + Args: + graph_name: Unique name for the test graph + triggers: List of Trigger objects to create + expected_count: Expected number of unique triggers in database + expected_pairs: Optional set of (expression, timezone) tuples to verify """ - # Clean up any existing triggers for this test + namespace = "test_namespace" + + # Pre-test cleanup await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_dedup_graph", - DatabaseTriggers.namespace == "test_namespace" + DatabaseTriggers.graph_name == graph_name, + DatabaseTriggers.namespace == namespace ).delete_many() - # Create a graph template with duplicate triggers + # Create graph template with triggers graph_template = GraphTemplate( - name="test_dedup_graph", - namespace="test_namespace", + name=graph_name, + namespace=namespace, nodes=[ NodeTemplate( node_name="test_node", - namespace="test_namespace", + namespace=namespace, identifier="test_node", inputs={}, next_nodes=None, @@ -42,92 +51,74 @@ async def test_create_crons_deduplicates_identical_triggers(app_started): ) ], validation_status=GraphTemplateValidationStatus.PENDING, - triggers=[ - Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), - Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), - Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), - ] + triggers=triggers ) # Call create_crons await create_crons(graph_template) - # Query the database to verify only one trigger was created - triggers = await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_dedup_graph", - DatabaseTriggers.namespace == "test_namespace", - DatabaseTriggers.expression == "0 9 * * *", - DatabaseTriggers.timezone == "America/New_York" + # Query database + triggers_in_db = await DatabaseTriggers.find( + DatabaseTriggers.graph_name == graph_name, + DatabaseTriggers.namespace == namespace ).to_list() - # Assert only one trigger exists (deduplication worked) - assert len(triggers) == 1 - assert triggers[0].expression == "0 9 * * *" - assert triggers[0].timezone == "America/New_York" + # Assert count + assert len(triggers_in_db) == expected_count - # Clean up + # Assert pairs if provided + if expected_pairs is not None: + actual_pairs = {(t.expression, t.timezone) for t in triggers_in_db} + assert actual_pairs == expected_pairs + + # Post-test cleanup await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_dedup_graph", - DatabaseTriggers.namespace == "test_namespace" + DatabaseTriggers.graph_name == graph_name, + DatabaseTriggers.namespace == namespace ).delete_many() +@pytest.mark.asyncio(loop_scope="session") +async def test_create_crons_deduplicates_identical_triggers(app_started): + """Test that create_crons deduplicates triggers with identical expression and timezone + + This integration test verifies the actual database behavior, not mocks. + It creates a GraphTemplate with duplicate CRON triggers and verifies that only + one DatabaseTriggers row exists per (expression, timezone) pair. + """ + await run_deduplication_test( + graph_name="test_dedup_graph", + triggers=[ + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), + ], + expected_count=1, + expected_pairs={("0 9 * * *", "America/New_York")} + ) + + @pytest.mark.asyncio(loop_scope="session") async def test_create_crons_keeps_triggers_with_different_timezones(app_started): """Test that create_crons keeps triggers with same expression but different timezones This verifies that triggers are only deduplicated when BOTH expression AND timezone match. """ - # Clean up any existing triggers for this test - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_timezone_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() - - # Create a graph template with same expression, different timezones - graph_template = GraphTemplate( - name="test_timezone_graph", - namespace="test_namespace", - nodes=[ - NodeTemplate( - node_name="test_node", - namespace="test_namespace", - identifier="test_node", - inputs={}, - next_nodes=None, - unites=None - ) - ], - validation_status=GraphTemplateValidationStatus.PENDING, + await run_deduplication_test( + graph_name="test_timezone_graph", triggers=[ Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Europe/London"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Asia/Tokyo"}), - ] + ], + expected_count=3, + expected_pairs={ + ("0 9 * * *", "America/New_York"), + ("0 9 * * *", "Europe/London"), + ("0 9 * * *", "Asia/Tokyo") + } ) - # Call create_crons - await create_crons(graph_template) - - # Query the database to verify all three triggers were created (different timezones) - triggers = await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_timezone_graph", - DatabaseTriggers.namespace == "test_namespace", - DatabaseTriggers.expression == "0 9 * * *" - ).to_list() - - # Assert three triggers exist (one for each timezone) - assert len(triggers) == 3 - - timezones = {t.timezone for t in triggers} - assert timezones == {"America/New_York", "Europe/London", "Asia/Tokyo"} - - # Clean up - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_timezone_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() - @pytest.mark.asyncio(loop_scope="session") async def test_create_crons_keeps_triggers_with_different_expressions(app_started): @@ -135,55 +126,21 @@ async def test_create_crons_keeps_triggers_with_different_expressions(app_starte This verifies basic functionality - triggers with different expressions should all be created. """ - # Clean up any existing triggers for this test - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_expr_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() - - # Create a graph template with different expressions - graph_template = GraphTemplate( - name="test_expr_graph", - namespace="test_namespace", - nodes=[ - NodeTemplate( - node_name="test_node", - namespace="test_namespace", - identifier="test_node", - inputs={}, - next_nodes=None, - unites=None - ) - ], - validation_status=GraphTemplateValidationStatus.PENDING, + await run_deduplication_test( + graph_name="test_expr_graph", triggers=[ Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "UTC"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 12 * * *", "timezone": "UTC"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), - ] + ], + expected_count=3, + expected_pairs={ + ("0 9 * * *", "UTC"), + ("0 12 * * *", "UTC"), + ("*/15 * * * *", "UTC") + } ) - # Call create_crons - await create_crons(graph_template) - - # Query the database to verify all three triggers were created - triggers = await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_expr_graph", - DatabaseTriggers.namespace == "test_namespace" - ).to_list() - - # Assert three triggers exist (all different expressions) - assert len(triggers) == 3 - - expressions = {t.expression for t in triggers} - assert expressions == {"0 9 * * *", "0 12 * * *", "*/15 * * * *"} - - # Clean up - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_expr_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() - @pytest.mark.asyncio(loop_scope="session") async def test_create_crons_complex_deduplication_scenario(app_started): @@ -194,72 +151,26 @@ async def test_create_crons_complex_deduplication_scenario(app_started): - Some unique triggers - Triggers with same expression but different timezone """ - # Clean up any existing triggers for this test - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_complex_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() - - # Create a graph template with a complex mix of triggers - graph_template = GraphTemplate( - name="test_complex_graph", - namespace="test_namespace", - nodes=[ - NodeTemplate( - node_name="test_node", - namespace="test_namespace", - identifier="test_node", - inputs={}, - next_nodes=None, - unites=None - ) - ], - validation_status=GraphTemplateValidationStatus.PENDING, + await run_deduplication_test( + graph_name="test_complex_graph", triggers=[ # Three duplicates - should result in 1 DB row Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "America/New_York"}), - # Same expression, different timezone - should result in 1 DB row Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 9 * * *", "timezone": "Europe/London"}), - # Two duplicates of a different expression - should result in 1 DB row Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "*/15 * * * *", "timezone": "UTC"}), - # Unique trigger - should result in 1 DB row Trigger(value={"type": TriggerTypeEnum.CRON, "expression": "0 0 1 * *", "timezone": "Asia/Tokyo"}), - ] - ) - - # Call create_crons - await create_crons(graph_template) - - # Query the database to verify correct number of triggers - triggers = await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_complex_graph", - DatabaseTriggers.namespace == "test_namespace" - ).to_list() - - # Expected: 4 unique (expression, timezone) pairs - # 1. (0 9 * * *, America/New_York) - # 2. (0 9 * * *, Europe/London) - # 3. (*/15 * * * *, UTC) - # 4. (0 0 1 * *, Asia/Tokyo) - assert len(triggers) == 4 - - # Verify each unique pair exists - trigger_pairs = {(t.expression, t.timezone) for t in triggers} - assert trigger_pairs == { - ("0 9 * * *", "America/New_York"), - ("0 9 * * *", "Europe/London"), - ("*/15 * * * *", "UTC"), - ("0 0 1 * *", "Asia/Tokyo"), - } - - # Clean up - await DatabaseTriggers.find( - DatabaseTriggers.graph_name == "test_complex_graph", - DatabaseTriggers.namespace == "test_namespace" - ).delete_many() \ No newline at end of file + ], + expected_count=4, + expected_pairs={ + ("0 9 * * *", "America/New_York"), + ("0 9 * * *", "Europe/London"), + ("*/15 * * * *", "UTC"), + ("0 0 1 * *", "Asia/Tokyo"), + } + ) \ No newline at end of file From 243762490806d3f95d6eeb74977b4d776f640cf7 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 24 Oct 2025 10:12:55 +0530 Subject: [PATCH 11/14] feat: add timezone validation for CronTrigger model - Introduced a field validator for the 'timezone' attribute in the CronTrigger model to ensure it contains a valid IANA timezone. - Utilized the available_timezones from the zoneinfo module to validate the input, raising a ValueError for invalid timezones. This enhancement improves the robustness of the CronTrigger configuration by preventing incorrect timezone entries. --- python-sdk/exospherehost/models.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/python-sdk/exospherehost/models.py b/python-sdk/exospherehost/models.py index ecb7c17d..b5be5a29 100644 --- a/python-sdk/exospherehost/models.py +++ b/python-sdk/exospherehost/models.py @@ -1,7 +1,9 @@ from pydantic import BaseModel, Field, field_validator from typing import Any, Optional, List from enum import Enum +from zoneinfo import available_timezones +_AVAILABLE_TIMEZONES = available_timezones() class UnitesStrategyEnum(str, Enum): ALL_SUCCESS = "ALL_SUCCESS" @@ -161,4 +163,11 @@ def validate_default_values(cls, v: dict[str, str]) -> dict[str, str]: class CronTrigger(BaseModel): expression: str = Field(..., description="Cron expression for scheduling automatic graph execution. Uses standard 5-field format: minute hour day-of-month month day-of-week. Example: '0 9 * * 1-5' for weekdays at 9 AM.") - timezone: str = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC'). Defaults to 'UTC'.") \ No newline at end of file + timezone: str = Field(default="UTC", description="Timezone for the cron expression (e.g., 'America/New_York', 'Europe/London', 'UTC'). Defaults to 'UTC'.") + + @field_validator('timezone') + @classmethod + def validate_timezone(cls, v: str) -> str: + if v not in _AVAILABLE_TIMEZONES: + raise ValueError(f"Invalid timezone: {v}. Must be a valid IANA timezone (e.g., 'America/New_York', 'Europe/London', 'UTC')") + return v \ No newline at end of file From cacf060ea7fd376f53cb5dad8dba78977463e721 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 24 Oct 2025 11:50:15 +0530 Subject: [PATCH 12/14] fix: correct placement of CRON type in StateManager triggers - Moved the "type": "CRON" field to the correct location within the triggers body in the StateManager class. - Ensured proper structure for trigger configuration, enhancing clarity and functionality. --- python-sdk/exospherehost/statemanager.py | 2 +- state-manager/app/models/trigger_models.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python-sdk/exospherehost/statemanager.py b/python-sdk/exospherehost/statemanager.py index eed01414..e4d4b010 100644 --- a/python-sdk/exospherehost/statemanager.py +++ b/python-sdk/exospherehost/statemanager.py @@ -171,8 +171,8 @@ async def upsert_graph(self, graph_name: str, graph_nodes: list[GraphNodeModel], if triggers is not None: body["triggers"] = [ { - "type": "CRON", "value": { + "type": "CRON", "expression": trigger.expression, "timezone": trigger.timezone } diff --git a/state-manager/app/models/trigger_models.py b/state-manager/app/models/trigger_models.py index a31693b7..c44ef168 100644 --- a/state-manager/app/models/trigger_models.py +++ b/state-manager/app/models/trigger_models.py @@ -37,7 +37,7 @@ def validate_timezone(cls, v: str) -> str: return v # Union type for all trigger types - add new trigger types here -TriggerValue = Annotated[Union[CronTrigger], Field(discriminator="type")] +TriggerValue = Annotated[Union[CronTrigger], Field(discriminator="type")] # type: ignore class Trigger(BaseModel): """ From e7fc2f70dae6b52d9455ae0ed4dd24de63e3d873 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 24 Oct 2025 11:56:07 +0530 Subject: [PATCH 13/14] fix: ensure timezone-aware datetime in trigger tests - Updated test cases in test_trigger_cron.py to use timezone-aware datetime by replacing naive datetime.now() with datetime.now(timezone.utc). - Added timezone attribute to the trigger mock to ensure consistency in testing future triggers. --- state-manager/tests/unit/tasks/test_trigger_cron.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/state-manager/tests/unit/tasks/test_trigger_cron.py b/state-manager/tests/unit/tasks/test_trigger_cron.py index c49addf5..fe8d6f38 100644 --- a/state-manager/tests/unit/tasks/test_trigger_cron.py +++ b/state-manager/tests/unit/tasks/test_trigger_cron.py @@ -257,9 +257,10 @@ async def test_call_trigger_graph(): @pytest.mark.asyncio async def test_create_next_triggers_creates_future_trigger(): """Test create_next_triggers creates next trigger in the future""" - cron_time = datetime.now(timezone.utc) + cron_time = datetime.now(timezone.utc).replace(tzinfo=None) trigger = MagicMock(spec=DatabaseTriggers) trigger.expression = "0 9 * * *" + trigger.timezone = "UTC" trigger.trigger_time = cron_time - timedelta(days=1) trigger.graph_name = "test_graph" trigger.namespace = "test_ns" @@ -357,9 +358,10 @@ async def test_create_next_triggers_creates_multiple_triggers(): @pytest.mark.asyncio async def test_create_next_triggers_raises_on_other_exceptions(): """Test create_next_triggers raises on non-DuplicateKeyError exceptions""" - cron_time = datetime.now(timezone.utc) + cron_time = datetime.now(timezone.utc).replace(tzinfo=None) trigger = MagicMock(spec=DatabaseTriggers) trigger.expression = "0 9 * * *" + trigger.timezone = "UTC" trigger.trigger_time = cron_time - timedelta(days=1) trigger.graph_name = "test_graph" trigger.namespace = "test_ns" From 9510f296a92fd946a755dfd90aadec1033393a03 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Fri, 24 Oct 2025 11:59:06 +0530 Subject: [PATCH 14/14] chore: update pytest.ini to filter specific deprecation warnings - Added filterwarnings configuration to ignore deprecation warnings from Pydantic and lazy_model, improving test output clarity and reducing noise during test runs. --- state-manager/pytest.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/state-manager/pytest.ini b/state-manager/pytest.ini index 0e617e9f..e38d3c98 100644 --- a/state-manager/pytest.ini +++ b/state-manager/pytest.ini @@ -7,4 +7,7 @@ markers = unit: marks a test as a unit test with_database: marks a test as a test that requires a database asyncio_mode = auto +filterwarnings = + ignore::pydantic.PydanticDeprecatedSince211:lazy_model.* + ignore::DeprecationWarning:lazy_model.*