From e0dc4be8d32254ce9631a13ace7505ab0bbd3159 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Mon, 25 Aug 2025 06:49:02 +0800 Subject: [PATCH 01/26] feat: add workflow schedule trigger support --- api/core/workflow/nodes/enums.py | 1 + api/core/workflow/nodes/node_mapping.py | 5 + .../nodes/trigger_schedule/__init__.py | 3 + .../nodes/trigger_schedule/entities.py | 20 ++ .../trigger_schedule/trigger_schedule_node.py | 70 +++++ api/docker/entrypoint.sh | 9 +- api/extensions/ext_celery.py | 1 + ...1e06b2654c6c_add_workflow_schedule_plan.py | 52 ++++ api/models/workflow.py | 69 +++++ api/pyproject.toml | 1 + api/schedule/schedule_dispatch.py | 239 +++++++++++++++ api/services/workflow/schedule_manager.py | 275 ++++++++++++++++++ api/services/workflow/schedule_sync.py | 256 ++++++++++++++++ api/services/workflow_service.py | 12 + api/tasks/workflow_schedule_tasks.py | 65 +++++ dev/start-worker | 5 +- dev/start-workflow-scheduler | 75 +++++ docker/docker-compose.yaml | 19 ++ 18 files changed, 1173 insertions(+), 4 deletions(-) create mode 100644 api/core/workflow/nodes/trigger_schedule/__init__.py create mode 100644 api/core/workflow/nodes/trigger_schedule/entities.py create mode 100644 api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py create mode 100644 api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py create mode 100644 api/schedule/schedule_dispatch.py create mode 100644 api/services/workflow/schedule_manager.py create mode 100644 api/services/workflow/schedule_sync.py create mode 100644 api/tasks/workflow_schedule_tasks.py create mode 100755 dev/start-workflow-scheduler diff --git a/api/core/workflow/nodes/enums.py b/api/core/workflow/nodes/enums.py index 7cf9ab9107441c..21fbaf4bcb8989 100644 --- a/api/core/workflow/nodes/enums.py +++ b/api/core/workflow/nodes/enums.py @@ -25,6 +25,7 @@ class NodeType(StrEnum): DOCUMENT_EXTRACTOR = "document-extractor" LIST_OPERATOR = "list-operator" AGENT = "agent" + TRIGGER_SCHEDULE = "trigger-schedule" class ErrorStrategy(StrEnum): diff --git a/api/core/workflow/nodes/node_mapping.py b/api/core/workflow/nodes/node_mapping.py index 294b47670b1f53..0a237fb3a114e2 100644 --- a/api/core/workflow/nodes/node_mapping.py +++ b/api/core/workflow/nodes/node_mapping.py @@ -19,6 +19,7 @@ from core.workflow.nodes.start import StartNode from core.workflow.nodes.template_transform import TemplateTransformNode from core.workflow.nodes.tool import ToolNode +from core.workflow.nodes.trigger_schedule import TriggerScheduleNode from core.workflow.nodes.variable_aggregator import VariableAggregatorNode from core.workflow.nodes.variable_assigner.v1 import VariableAssignerNode as VariableAssignerNodeV1 from core.workflow.nodes.variable_assigner.v2 import VariableAssignerNode as VariableAssignerNodeV2 @@ -132,4 +133,8 @@ "2": AgentNode, "1": AgentNode, }, + NodeType.TRIGGER_SCHEDULE: { + LATEST_VERSION: TriggerScheduleNode, + "1": TriggerScheduleNode, + }, } diff --git a/api/core/workflow/nodes/trigger_schedule/__init__.py b/api/core/workflow/nodes/trigger_schedule/__init__.py new file mode 100644 index 00000000000000..6773bae5027235 --- /dev/null +++ b/api/core/workflow/nodes/trigger_schedule/__init__.py @@ -0,0 +1,3 @@ +from core.workflow.nodes.trigger_schedule.trigger_schedule_node import TriggerScheduleNode + +__all__ = ["TriggerScheduleNode"] diff --git a/api/core/workflow/nodes/trigger_schedule/entities.py b/api/core/workflow/nodes/trigger_schedule/entities.py new file mode 100644 index 00000000000000..9cd081568bc324 --- /dev/null +++ b/api/core/workflow/nodes/trigger_schedule/entities.py @@ -0,0 +1,20 @@ +from typing import Optional + +from pydantic import Field + +from core.workflow.nodes.base import BaseNodeData + + +class TriggerScheduleNodeData(BaseNodeData): + """ + Trigger Schedule Node Data + """ + + mode: str = Field(default="visual", description="Schedule mode: visual or cron") + frequency: Optional[str] = Field( + default=None, description="Frequency for visual mode: hourly, daily, weekly, monthly" + ) + cron_expression: Optional[str] = Field(default=None, description="Cron expression for cron mode") + visual_config: Optional[dict] = Field(default=None, description="Visual configuration details") + timezone: str = Field(default="UTC", description="Timezone for schedule execution") + enabled: bool = Field(default=True, description="Whether the schedule is enabled") diff --git a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py new file mode 100644 index 00000000000000..5611945dcbdb79 --- /dev/null +++ b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py @@ -0,0 +1,70 @@ +from collections.abc import Mapping +from datetime import UTC, datetime +from typing import Any, Optional + +from core.workflow.constants import SYSTEM_VARIABLE_NODE_ID +from core.workflow.entities.node_entities import NodeRunResult +from core.workflow.entities.workflow_node_execution import WorkflowNodeExecutionStatus +from core.workflow.nodes.base import BaseNode +from core.workflow.nodes.base.entities import BaseNodeData, RetryConfig +from core.workflow.nodes.enums import ErrorStrategy, NodeType +from core.workflow.nodes.trigger_schedule.entities import TriggerScheduleNodeData + + +class TriggerScheduleNode(BaseNode): + _node_type = NodeType.TRIGGER_SCHEDULE + + _node_data: TriggerScheduleNodeData + + def init_node_data(self, data: Mapping[str, Any]) -> None: + self._node_data = TriggerScheduleNodeData(**data) + + def _get_error_strategy(self) -> Optional[ErrorStrategy]: + return self._node_data.error_strategy + + def _get_retry_config(self) -> RetryConfig: + return self._node_data.retry_config + + def _get_title(self) -> str: + return self._node_data.title + + def _get_description(self) -> Optional[str]: + return self._node_data.desc + + def _get_default_value_dict(self) -> dict[str, Any]: + return self._node_data.default_value_dict + + def get_base_node_data(self) -> BaseNodeData: + return self._node_data + + @classmethod + def version(cls) -> str: + return "1" + + def _run(self) -> NodeRunResult: + node_inputs = dict(self.graph_runtime_state.variable_pool.user_inputs) + system_inputs = self.graph_runtime_state.variable_pool.system_variables.to_dict() + + # Set system variables as node outputs + for var in system_inputs: + node_inputs[SYSTEM_VARIABLE_NODE_ID + "." + var] = system_inputs[var] + + # Add schedule-specific outputs + triggered_at = datetime.now(UTC) + node_inputs["triggered_at"] = triggered_at.isoformat() + node_inputs["timezone"] = self._node_data.timezone + node_inputs["mode"] = self._node_data.mode + node_inputs["enabled"] = self._node_data.enabled + + # Add configuration details based on mode + if self._node_data.mode == "cron" and self._node_data.cron_expression: + node_inputs["cron_expression"] = self._node_data.cron_expression + elif self._node_data.mode == "visual": + node_inputs["frequency"] = self._node_data.frequency or "unknown" + if self._node_data.visual_config: + node_inputs["visual_config"] = self._node_data.visual_config + + return NodeRunResult( + status=WorkflowNodeExecutionStatus.SUCCEEDED, + outputs=node_inputs, + ) diff --git a/api/docker/entrypoint.sh b/api/docker/entrypoint.sh index c6b1afc3bd2242..939feae814e3d3 100755 --- a/api/docker/entrypoint.sh +++ b/api/docker/entrypoint.sh @@ -34,10 +34,10 @@ if [[ "${MODE}" == "worker" ]]; then if [[ -z "${CELERY_QUEUES}" ]]; then if [[ "${EDITION}" == "CLOUD" ]]; then # Cloud edition: separate queues for dataset and trigger tasks - DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow_professional,workflow_team,workflow_sandbox" + DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow_professional,workflow_team,workflow_sandbox,schedule" else # Community edition (SELF_HOSTED): dataset and workflow have separate queues - DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow" + DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow,schedule" fi else DEFAULT_QUEUES="${CELERY_QUEUES}" @@ -68,6 +68,11 @@ if [[ "${MODE}" == "worker" ]]; then elif [[ "${MODE}" == "beat" ]]; then exec celery -A app.celery beat --loglevel ${LOG_LEVEL:-INFO} +elif [[ "${MODE}" == "workflow-scheduler" ]]; then + echo "Starting Workflow Scheduler..." + exec celery -A app.celery beat \ + --scheduler schedule.schedule_dispatch:WorkflowScheduler \ + --loglevel ${LOG_LEVEL:-INFO} else if [[ "${DEBUG}" == "true" ]]; then exec flask run --host=${DIFY_BIND_ADDRESS:-0.0.0.0} --port=${DIFY_PORT:-5001} --debug diff --git a/api/extensions/ext_celery.py b/api/extensions/ext_celery.py index bcda75aee2e16c..555d3151df8e81 100644 --- a/api/extensions/ext_celery.py +++ b/api/extensions/ext_celery.py @@ -98,6 +98,7 @@ def __call__(self, *args: object, **kwargs: object) -> object: imports = [ "tasks.async_workflow_tasks", # trigger workers + "tasks.workflow_schedule_tasks", # schedule trigger tasks ] day = dify_config.CELERY_BEAT_SCHEDULER_TIME diff --git a/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py b/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py new file mode 100644 index 00000000000000..180efea42e6baa --- /dev/null +++ b/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py @@ -0,0 +1,52 @@ +"""add workflow_schedule_plan + +Revision ID: 1e06b2654c6c +Revises: 4558cfabe44e +Create Date: 2025-08-24 13:13:23.495063 + +""" +from alembic import op +import models as models +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '1e06b2654c6c' +down_revision = '4558cfabe44e' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('workflow_schedule_plans', + sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuid_generate_v4()'), nullable=False), + sa.Column('tenant_id', models.types.StringUUID(), nullable=False), + sa.Column('app_id', models.types.StringUUID(), nullable=False), + sa.Column('workflow_id', models.types.StringUUID(), nullable=False), + sa.Column('root_node_id', sa.String(length=255), nullable=False), + sa.Column('cron_expression', sa.String(length=255), nullable=False), + sa.Column('timezone', sa.String(length=64), nullable=False), + sa.Column('enabled', sa.Boolean(), nullable=False), + sa.Column('next_run_at', sa.DateTime(timezone=True), nullable=True), + sa.Column('created_by', models.types.StringUUID(), nullable=False), + sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.PrimaryKeyConstraint('id', name='workflow_schedule_plan_pkey') + ) + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.create_index('workflow_schedule_plan_app_workflow_idx', ['app_id', 'workflow_id'], unique=False) + batch_op.create_index('workflow_schedule_plan_created_at_idx', ['created_at'], unique=False) + batch_op.create_index('workflow_schedule_plan_tenant_enabled_next_idx', ['tenant_id', 'enabled', 'next_run_at'], unique=False) + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.drop_index('workflow_schedule_plan_tenant_enabled_next_idx') + batch_op.drop_index('workflow_schedule_plan_created_at_idx') + batch_op.drop_index('workflow_schedule_plan_app_workflow_idx') + + op.drop_table('workflow_schedule_plans') + # ### end Alembic commands ### diff --git a/api/models/workflow.py b/api/models/workflow.py index 38eda31c5eb7d9..f52d47b18333e5 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -1383,3 +1383,72 @@ def to_dict(self) -> dict: "triggered_at": self.triggered_at.isoformat() if self.triggered_at else None, "finished_at": self.finished_at.isoformat() if self.finished_at else None, } + + +class WorkflowSchedulePlan(Base): + """ + Workflow Schedule Configuration + + Store schedule configurations for time-based workflow triggers. + Uses cron expressions with timezone support for flexible scheduling. + + Attributes: + id: UUID primary key + tenant_id: Workspace ID for multi-tenancy + app_id: Application ID this schedule belongs to + workflow_id: Workflow to trigger + root_node_id: Starting node ID for workflow execution + cron_expression: Cron expression defining schedule pattern + timezone: Timezone for cron evaluation (e.g., 'Asia/Shanghai') + enabled: Whether schedule is active + next_run_at: Next scheduled execution time (timestamptz) + created_by: Creator account ID + created_at: Creation timestamp + updated_at: Last update timestamp + """ + + __tablename__ = "workflow_schedule_plans" + __table_args__ = ( + sa.PrimaryKeyConstraint("id", name="workflow_schedule_plan_pkey"), + sa.Index("workflow_schedule_plan_tenant_enabled_next_idx", "tenant_id", "enabled", "next_run_at"), + sa.Index("workflow_schedule_plan_app_workflow_idx", "app_id", "workflow_id"), + sa.Index("workflow_schedule_plan_created_at_idx", "created_at"), + ) + + id: Mapped[str] = mapped_column(StringUUID, server_default=sa.text("uuid_generate_v4()")) + tenant_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + app_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + workflow_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + + root_node_id: Mapped[str] = mapped_column(String(255), nullable=False) + + # Schedule configuration + cron_expression: Mapped[str] = mapped_column(String(255), nullable=False) + timezone: Mapped[str] = mapped_column(String(64), nullable=False) + + # Schedule control + enabled: Mapped[bool] = mapped_column(sa.Boolean, nullable=False, default=True) + next_run_at: Mapped[Optional[datetime]] = mapped_column(DateTime(timezone=True), nullable=True) + + created_by: Mapped[str] = mapped_column(StringUUID, nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=func.current_timestamp()) + updated_at: Mapped[datetime] = mapped_column( + DateTime, nullable=False, server_default=func.current_timestamp(), onupdate=func.current_timestamp() + ) + + def to_dict(self) -> dict: + """Convert to dictionary representation""" + return { + "id": self.id, + "tenant_id": self.tenant_id, + "app_id": self.app_id, + "workflow_id": self.workflow_id, + "root_node_id": self.root_node_id, + "cron_expression": self.cron_expression, + "timezone": self.timezone, + "enabled": self.enabled, + "next_run_at": self.next_run_at.isoformat() if self.next_run_at else None, + "created_by": self.created_by, + "created_at": self.created_at.isoformat(), + "updated_at": self.updated_at.isoformat(), + } diff --git a/api/pyproject.toml b/api/pyproject.toml index cf5ad8e7d2f9a6..f82997c03632e6 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -88,6 +88,7 @@ dependencies = [ "sseclient-py>=1.8.0", "httpx-sse>=0.4.0", "sendgrid~=6.12.3", + "croniter>=6.0.0", ] # Before adding new dependency, consider place it in # alphabet order (a-z) and suitable group. diff --git a/api/schedule/schedule_dispatch.py b/api/schedule/schedule_dispatch.py new file mode 100644 index 00000000000000..9a5d5e4d00dc5d --- /dev/null +++ b/api/schedule/schedule_dispatch.py @@ -0,0 +1,239 @@ +import logging +import os +import time +from datetime import UTC, datetime + +import sqlalchemy as sa +from celery.beat import Scheduler +from sqlalchemy import create_engine, func, select +from sqlalchemy.orm import Session + +from configs import dify_config +from models.workflow import WorkflowSchedulePlan +from services.workflow.schedule_manager import ScheduleService +from tasks.workflow_schedule_tasks import run_schedule_trigger + +logger = logging.getLogger(__name__) + +# Standalone engine because Beat runs in separate process without Flask context +engine = create_engine( + dify_config.SQLALCHEMY_DATABASE_URI, + pool_pre_ping=True, + pool_recycle=300, + # pool_size=2, # Small pool for Beat's limited concurrent needs + # max_overflow=3, # Allow a few extra connections if needed + # pool_timeout=10, # Fail fast if pool exhausted +) + + +class WorkflowScheduler(Scheduler): + """ + Celery Beat scheduler for workflow schedule triggers. + + Features: + - Dynamic schedule configuration from database + - High availability support with SKIP LOCKED + - Real-time schedule updates without restart + - Misfire handling with grace period + """ + + def __init__(self, *args, **kwargs): + # Must initialize before super() because setup_schedule() needs these + self._last_sync = None + self.sync_every = int(os.getenv("WORKFLOW_SCHEDULER_SYNC_EVERY", "60")) + self.misfire_grace_time = int(os.getenv("WORKFLOW_SCHEDULER_MISFIRE_GRACE", "300")) + self.min_tick_interval = float(os.getenv("WORKFLOW_SCHEDULER_MIN_TICK", "5.0")) + self.max_tick_interval = float(os.getenv("WORKFLOW_SCHEDULER_MAX_TICK", "60.0")) + self.batch_size = int(os.getenv("WORKFLOW_SCHEDULER_BATCH_SIZE", "100")) + + logger.info( + "WorkflowScheduler initialized with: sync_every=%ds, " + "misfire_grace=%ds, tick_interval=[%.1f, %.1f]s, batch_size=%d", + self.sync_every, + self.misfire_grace_time, + self.min_tick_interval, + self.max_tick_interval, + self.batch_size, + ) + + super().__init__(*args, **kwargs) + + @staticmethod + def ensure_utc(dt): + """Ensure datetime is UTC-aware.""" + if dt and dt.tzinfo is None: + return dt.replace(tzinfo=UTC) + return dt + + def setup_schedule(self): + """Initial schedule setup from database.""" + self.sync_schedules() + + def sync_schedules(self): + """ + Synchronize schedules from database. + + This method validates that schedules exist and are properly configured. + """ + with Session(engine) as session: + # Count enabled schedules for monitoring + enabled_count = ( + session.scalar( + select(func.count(WorkflowSchedulePlan.id)).where( + WorkflowSchedulePlan.enabled == True, + WorkflowSchedulePlan.next_run_at.isnot(None), + ) + ) + or 0 + ) + + logger.debug("Found %d enabled schedules", enabled_count) + + self._last_sync = time.monotonic() + + def tick(self): + """ + Main scheduler tick - called periodically by Celery Beat. + + This method: + 1. Syncs with database if needed + 2. Finds and dispatches due schedules + 3. Uses SELECT FOR UPDATE SKIP LOCKED for HA + + Returns: + Seconds until next tick + """ + if self._last_sync is None or (time.monotonic() - self._last_sync) > self.sync_every: + logger.debug("Syncing schedules with database") + self.sync_schedules() + + now = datetime.now(UTC) + + with Session(engine) as session: + # SKIP LOCKED ensures only one Beat instance processes each schedule in HA setup + due_schedules = session.scalars( + select(WorkflowSchedulePlan) + .where( + WorkflowSchedulePlan.enabled == True, + WorkflowSchedulePlan.next_run_at <= now, + WorkflowSchedulePlan.next_run_at.isnot(None), + ) + .with_for_update(skip_locked=True) + .limit(self.batch_size) # Limit to prevent memory issues + ).all() + + if due_schedules: + logger.info("Processing %d due schedules", len(due_schedules)) + + schedules_to_update = [] + schedules_to_disable = [] + + for schedule in due_schedules: + try: + skip_execution = False + if schedule.next_run_at: + next_run_at = self.ensure_utc(schedule.next_run_at) + time_since_due = (now - next_run_at).total_seconds() + + if time_since_due > self.misfire_grace_time: + logger.warning( + "Schedule %s misfired (due %.1fs ago) - skipping execution", + schedule.id, + time_since_due, + ) + skip_execution = True + + if not skip_execution: + result = run_schedule_trigger.delay(schedule.id) + logger.info("Dispatched schedule %s, task ID: %s", schedule.id, result.id) + + # Calculate next run immediately to prevent duplicate triggers + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + base_time=now, + ) + + if next_run_at is None: + schedules_to_disable.append(schedule.id) + logger.info("Schedule %s has no more runs, will disable", schedule.id) + else: + schedules_to_update.append((schedule.id, next_run_at)) + logger.debug("Schedule %s next run at %s", schedule.id, next_run_at) + + except Exception as e: + logger.error("Error processing schedule %s: %s", schedule.id, e, exc_info=True) + + # Bulk update to reduce database round trips from N to 1 + try: + if schedules_to_update: + from sqlalchemy import case + + stmt = ( + sa.update(WorkflowSchedulePlan) + .where(WorkflowSchedulePlan.id.in_([s[0] for s in schedules_to_update])) + .values(next_run_at=case(dict(schedules_to_update), value=WorkflowSchedulePlan.id)) + ) + session.execute(stmt) + + if schedules_to_disable: + stmt = ( + sa.update(WorkflowSchedulePlan) + .where(WorkflowSchedulePlan.id.in_(schedules_to_disable)) + .values(enabled=False, next_run_at=None) + ) + session.execute(stmt) + + session.commit() + + if schedules_to_update or schedules_to_disable: + logger.info( + "Bulk updated %d schedules, disabled %d", + len(schedules_to_update), + len(schedules_to_disable), + ) + + except Exception as e: + logger.error("Error bulk updating schedules: %s", e, exc_info=True) + session.rollback() + + next_schedule = session.scalar( + select(func.min(WorkflowSchedulePlan.next_run_at)).where( + WorkflowSchedulePlan.enabled == True, + WorkflowSchedulePlan.next_run_at > now, + ) + ) + + if next_schedule: + next_schedule = self.ensure_utc(next_schedule) + seconds_until_next = (next_schedule - now).total_seconds() + # Cap to avoid too frequent checks or missing schedules + return max(self.min_tick_interval, min(seconds_until_next, self.max_tick_interval)) + + return self.max_tick_interval + + @property + def info(self): + """Return scheduler info for debugging.""" + with Session(engine) as session: + schedule_count = ( + session.scalar( + select(func.count(WorkflowSchedulePlan.id)).where( + WorkflowSchedulePlan.enabled == True, + WorkflowSchedulePlan.next_run_at.isnot(None), + ) + ) + or 0 + ) + + return { + "scheduler": "WorkflowScheduler", + "last_sync": self._last_sync if self._last_sync else None, + "enabled_schedules": schedule_count, + "config": { + "sync_every": self.sync_every, + "misfire_grace_time": self.misfire_grace_time, + "min_tick_interval": self.min_tick_interval, + "max_tick_interval": self.max_tick_interval, + }, + } diff --git a/api/services/workflow/schedule_manager.py b/api/services/workflow/schedule_manager.py new file mode 100644 index 00000000000000..2bf7909d8486b8 --- /dev/null +++ b/api/services/workflow/schedule_manager.py @@ -0,0 +1,275 @@ +import logging +from datetime import UTC, datetime +from typing import Optional + +import pytz +from croniter import croniter +from sqlalchemy import select +from sqlalchemy.orm import Session + +from models.account import Account, TenantAccountJoin +from models.workflow import WorkflowSchedulePlan + +logger = logging.getLogger(__name__) + + +class ScheduleService: + @staticmethod + def calculate_next_run_at( + cron_expression: str, + timezone: str, + base_time: Optional[datetime] = None, + ) -> Optional[datetime]: + try: + tz = pytz.timezone(timezone) + if base_time is None: + base_time = datetime.now(UTC) + base_time_tz = base_time.astimezone(tz) + cron = croniter(cron_expression, base_time_tz) + next_run_tz = cron.get_next(datetime) + next_run_utc = next_run_tz.astimezone(UTC) + return next_run_utc + except Exception: + logger.exception("Error calculating next run time") + return None + + @staticmethod + def create_schedule( + session: Session, + tenant_id: str, + app_id: str, + workflow_id: str, + root_node_id: str, + cron_expression: str, + timezone: str, + created_by: str, + enabled: bool = True, + ) -> WorkflowSchedulePlan: + """ + Create a new workflow schedule. + + Args: + session: Database session + tenant_id: Tenant ID + app_id: Application ID + workflow_id: Workflow ID to trigger + root_node_id: Starting node ID + cron_expression: Cron expression + timezone: Timezone for cron evaluation + created_by: Creator account ID + enabled: Whether schedule is enabled + + Returns: + Created WorkflowSchedulePlan instance + """ + # Calculate initial next run time + next_run_at = ScheduleService.calculate_next_run_at( + cron_expression, + timezone, + ) + + # Create schedule record + schedule = WorkflowSchedulePlan( + tenant_id=tenant_id, + app_id=app_id, + workflow_id=workflow_id, + root_node_id=root_node_id, + cron_expression=cron_expression, + timezone=timezone, + enabled=enabled and next_run_at is not None, + next_run_at=next_run_at, + created_by=created_by, + ) + + session.add(schedule) + session.flush() + + return schedule + + @staticmethod + def update_schedule( + session: Session, + schedule_id: str, + updates: dict, + ) -> Optional[WorkflowSchedulePlan]: + """ + Update a schedule with the provided changes. + + Args: + session: Database session + schedule_id: Schedule ID to update + updates: Dictionary of fields to update + + Returns: + Updated schedule or None if not found + """ + schedule = session.get(WorkflowSchedulePlan, schedule_id) + if not schedule: + return None + + # Apply updates + for field, value in updates.items(): + if hasattr(schedule, field): + setattr(schedule, field, value) + + # Recalculate next_run_at if schedule parameters changed + if any(field in updates for field in ["cron_expression", "timezone", "enabled"]): + if schedule.enabled: + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + ) + schedule.next_run_at = next_run_at + + # Disable if no valid next run + if next_run_at is None: + schedule.enabled = False + + session.flush() + return schedule + + @staticmethod + def delete_schedule( + session: Session, + schedule_id: str, + ) -> bool: + """ + Delete a schedule. + + Args: + session: Database session + schedule_id: Schedule ID to delete + + Returns: + True if deleted, False if not found + """ + schedule = session.get(WorkflowSchedulePlan, schedule_id) + if not schedule: + return False + + session.delete(schedule) + session.flush() + return True + + @staticmethod + def get_schedule(session: Session, schedule_id: str) -> Optional[WorkflowSchedulePlan]: + """ + Get a schedule by ID. + + Args: + session: Database session + schedule_id: Schedule ID + + Returns: + WorkflowSchedulePlan or None + """ + return session.get(WorkflowSchedulePlan, schedule_id) + + @staticmethod + def list_schedules( + session: Session, + tenant_id: str, + app_id: Optional[str] = None, + workflow_id: Optional[str] = None, + enabled: Optional[bool] = None, + limit: int = 100, + offset: int = 0, + ) -> list[WorkflowSchedulePlan]: + """ + List schedules with optional filters. + + Args: + session: Database session + tenant_id: Tenant ID (required) + app_id: Optional app ID filter + workflow_id: Optional workflow ID filter + enabled: Optional enabled status filter + limit: Maximum results to return + offset: Number of results to skip + + Returns: + List of WorkflowSchedulePlan instances + """ + query = select(WorkflowSchedulePlan).where(WorkflowSchedulePlan.tenant_id == tenant_id) + + if app_id: + query = query.where(WorkflowSchedulePlan.app_id == app_id) + if workflow_id: + query = query.where(WorkflowSchedulePlan.workflow_id == workflow_id) + if enabled is not None: + query = query.where(WorkflowSchedulePlan.enabled == enabled) + + query = query.order_by(WorkflowSchedulePlan.created_at.desc()) + query = query.limit(limit).offset(offset) + + return list(session.scalars(query).all()) + + @staticmethod + def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: + """ + Get the owner account for a tenant. + Used to execute scheduled workflows on behalf of the tenant owner. + + Args: + session: Database session + tenant_id: Tenant ID + + Returns: + Owner Account or None + """ + # Query for owner role in tenant + result = session.execute( + select(TenantAccountJoin) + .where(TenantAccountJoin.tenant_id == tenant_id, TenantAccountJoin.role == "owner") + .limit(1) + ).scalar_one_or_none() + + if not result: + # Fallback to any admin if no owner + result = session.execute( + select(TenantAccountJoin) + .where(TenantAccountJoin.tenant_id == tenant_id, TenantAccountJoin.role == "admin") + .limit(1) + ).scalar_one_or_none() + + if result: + return session.get(Account, result.account_id) + + return None + + @staticmethod + def update_next_run_at( + session: Session, + schedule_id: str, + ) -> Optional[datetime]: + """ + Calculate and update the next run time for a schedule. + Used after a schedule has been triggered. + + Args: + session: Database session + schedule_id: Schedule ID + + Returns: + New next_run_at datetime or None + """ + schedule = session.get(WorkflowSchedulePlan, schedule_id) + if not schedule or not schedule.enabled: + return None + + # Calculate next run time + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + base_time=datetime.now(UTC), # Use current time as base + ) + + # Update schedule + schedule.next_run_at = next_run_at + + # If no next run, disable it + if next_run_at is None: + schedule.enabled = False + + session.flush() + return next_run_at diff --git a/api/services/workflow/schedule_sync.py b/api/services/workflow/schedule_sync.py new file mode 100644 index 00000000000000..245f757b79f69c --- /dev/null +++ b/api/services/workflow/schedule_sync.py @@ -0,0 +1,256 @@ +import json +import logging +from typing import Optional + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from models.workflow import WorkflowSchedulePlan +from services.workflow.schedule_manager import ScheduleService + +logger = logging.getLogger(__name__) + + +class ScheduleSyncService: + @staticmethod + def sync_schedule_from_graph( + session: Session, + tenant_id: str, + app_id: str, + workflow_id: str, + graph: str, + created_by: str, + ) -> Optional[WorkflowSchedulePlan]: + try: + graph_data = json.loads(graph) + except (json.JSONDecodeError, TypeError): + return None + + # Find schedule trigger node in graph + schedule_node = None + root_node_id = None + schedule_nodes_count = 0 + + for node in graph_data.get("nodes", []): + node_data = node.get("data", {}) + if node_data.get("type") == "trigger-schedule": + schedule_nodes_count += 1 + if not schedule_node: # Take the first schedule node + schedule_node = node_data + root_node_id = node.get("id", "start") + + if schedule_nodes_count > 1: + logger.warning("Found %d schedule nodes in workflow, only the first one will be used", schedule_nodes_count) + + # Get existing schedule plan for this app + # Note: We use app_id as the key since workflow_id changes with each publish + existing_plan = session.scalar( + select(WorkflowSchedulePlan).where( + WorkflowSchedulePlan.tenant_id == tenant_id, + WorkflowSchedulePlan.app_id == app_id, + ) + ) + + # If no schedule node exists, disable the schedule but keep the plan + if not schedule_node: + if existing_plan: + logger.info("No schedule node in workflow for app %s, disabling schedule plan", app_id) + updates = { + "workflow_id": workflow_id, + "enabled": False, # Disable but keep the plan + "next_run_at": None, # Clear next run time + } + return ScheduleService.update_schedule( + session=session, + schedule_id=existing_plan.id, + updates=updates, + ) + # No existing plan and no schedule node, nothing to do + return None + + # Extract schedule configuration + mode = schedule_node.get("mode", "visual") + timezone = schedule_node.get("timezone", "UTC") + enabled = schedule_node.get("enabled", True) + + # Convert to cron expression + cron_expression = None + if mode == "cron": + cron_expression = schedule_node.get("cron_expression") + elif mode == "visual": + cron_expression = ScheduleSyncService._visual_to_cron( + schedule_node.get("frequency"), schedule_node.get("visual_config", {}) + ) + + if not cron_expression: + # Invalid configuration, remove existing plan + logger.warning("Invalid schedule configuration for app %s, removing schedule plan", app_id) + if existing_plan: + session.delete(existing_plan) + session.flush() + return None + + # Update existing plan or create new one + if existing_plan: + # Update existing schedule with new workflow version + logger.info("Updating schedule plan for app %s with new workflow %s", app_id, workflow_id) + updates = { + "workflow_id": workflow_id, # Update to latest workflow version + "root_node_id": root_node_id, + "cron_expression": cron_expression, + "timezone": timezone, + "enabled": enabled, + } + updated_plan = ScheduleService.update_schedule( + session=session, + schedule_id=existing_plan.id, + updates=updates, + ) + return updated_plan + else: + # Create new schedule + logger.info("Creating new schedule plan for app %s, workflow %s", app_id, workflow_id) + new_plan = ScheduleService.create_schedule( + session=session, + tenant_id=tenant_id, + app_id=app_id, + workflow_id=workflow_id, + root_node_id=root_node_id, + cron_expression=cron_expression, + timezone=timezone, + created_by=created_by, + enabled=enabled, + ) + return new_plan + + @staticmethod + def _visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: + """ + Convert visual schedule configuration to cron expression. + + Args: + frequency: Schedule frequency (hourly, daily, weekly, monthly) + visual_config: Visual configuration with time, weekdays, etc. + + Returns: + Cron expression string or None if invalid + """ + if not frequency or not visual_config: + return None + + try: + if frequency == "hourly": + # Run at specific minute of every hour + on_minute = visual_config.get("on_minute", 0) + return f"{on_minute} * * * *" + + elif frequency == "daily": + # Run daily at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleSyncService._parse_time(time_str) + if hour is None or minute is None: + return None + return f"{minute} {hour} * * *" + + elif frequency == "weekly": + # Run weekly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleSyncService._parse_time(time_str) + if hour is None or minute is None: + return None + + weekdays = visual_config.get("weekdays", []) + if not weekdays: + return None + + # Convert weekday names to cron format (0-6, where 0=Sunday) + weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} + cron_weekdays = [] + for day in weekdays: + if day in weekday_map: + cron_weekdays.append(weekday_map[day]) + + if not cron_weekdays: + return None + + return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" + + elif frequency == "monthly": + # Run monthly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleSyncService._parse_time(time_str) + if hour is None or minute is None: + return None + + monthly_days = visual_config.get("monthly_days", []) + if not monthly_days: + return None + + # Convert monthly days to cron format + cron_days = [] + for day in monthly_days: + if day == "last": + # Last day of month (L is supported by some cron implementations) + # For standard cron, we use 28-31 as approximation + cron_days.extend(["28", "29", "30", "31"]) + elif isinstance(day, int) and 1 <= day <= 31: + cron_days.append(str(day)) + + if not cron_days: + return None + + # Remove duplicates and sort + cron_days = sorted(set(cron_days), key=int) + return f"{minute} {hour} {','.join(cron_days)} * *" + + else: + return None + + except Exception: + return None + + @staticmethod + def _parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: + """ + Parse time string in format "HH:MM AM/PM" to 24-hour format. + + Args: + time_str: Time string like "11:30 AM" or "2:45 PM" + + Returns: + Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid + """ + try: + # Split time and period + parts = time_str.strip().split() + if len(parts) != 2: + return None, None + + time_part, period = parts + period = period.upper() + + if period not in ["AM", "PM"]: + return None, None + + # Parse hour and minute + time_parts = time_part.split(":") + if len(time_parts) != 2: + return None, None + + hour = int(time_parts[0]) + minute = int(time_parts[1]) + + # Validate ranges + if hour < 1 or hour > 12 or minute < 0 or minute > 59: + return None, None + + # Convert to 24-hour format + if period == "PM" and hour != 12: + hour += 12 + elif period == "AM" and hour == 12: + hour = 0 + + return hour, minute + + except (ValueError, AttributeError): + return None, None diff --git a/api/services/workflow_service.py b/api/services/workflow_service.py index d2715a61fe1a43..6c02594471a6fb 100644 --- a/api/services/workflow_service.py +++ b/api/services/workflow_service.py @@ -44,6 +44,7 @@ ) from repositories.factory import DifyAPIRepositoryFactory from services.errors.app import IsDraftWorkflowError, WorkflowHashNotEqualError +from services.workflow.schedule_sync import ScheduleSyncService from services.workflow.workflow_converter import WorkflowConverter from .errors.workflow_service import DraftWorkflowDeletionError, WorkflowInUseError @@ -285,6 +286,17 @@ def publish_workflow( # commit db session changes session.add(workflow) + session.flush() + + # Sync schedule configuration from graph + ScheduleSyncService.sync_schedule_from_graph( + session=session, + tenant_id=app_model.tenant_id, + app_id=app_model.id, + workflow_id=workflow.id, + graph=workflow.graph, + created_by=account.id, + ) # trigger app workflow events app_published_workflow_was_updated.send(app_model, published_workflow=workflow) diff --git a/api/tasks/workflow_schedule_tasks.py b/api/tasks/workflow_schedule_tasks.py new file mode 100644 index 00000000000000..d901e0c9d3ec0a --- /dev/null +++ b/api/tasks/workflow_schedule_tasks.py @@ -0,0 +1,65 @@ +import logging +from datetime import UTC, datetime +from zoneinfo import ZoneInfo + +from celery import shared_task +from sqlalchemy.orm import sessionmaker + +from extensions.ext_database import db +from models.enums import WorkflowRunTriggeredFrom +from models.workflow import WorkflowSchedulePlan +from services.async_workflow_service import AsyncWorkflowService +from services.workflow.entities import TriggerData +from services.workflow.schedule_manager import ScheduleService + +logger = logging.getLogger(__name__) + + +@shared_task(queue="schedule") +def run_schedule_trigger(schedule_id: str): + """ + Execute a scheduled workflow trigger. + + Note: No retry logic needed as schedules will run again at next interval. + Failed executions are logged but don't block future runs. + """ + session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) + + with session_factory() as session: + schedule = session.get(WorkflowSchedulePlan, schedule_id) + if not schedule: + logger.warning("Schedule %s not found", schedule_id) + return + + if not schedule.enabled: + logger.debug("Schedule %s is disabled", schedule_id) + return + + tenant_owner = ScheduleService.get_tenant_owner(session, schedule.tenant_id) + if not tenant_owner: + logger.error("Tenant owner not found for tenant %s", schedule.tenant_id) + return + + try: + current_utc = datetime.now(UTC) + schedule_tz = ZoneInfo(schedule.timezone) if schedule.timezone else UTC + current_in_tz = current_utc.astimezone(schedule_tz) + inputs = {"current_time": current_in_tz.isoformat()} + + response = AsyncWorkflowService.trigger_workflow_async( + session=session, + user=tenant_owner, + trigger_data=TriggerData( + app_id=schedule.app_id, + workflow_id=schedule.workflow_id, + root_node_id=schedule.root_node_id, + trigger_type=WorkflowRunTriggeredFrom.SCHEDULE, + inputs=inputs, + tenant_id=schedule.tenant_id, + ), + ) + logger.info("Schedule %s triggered workflow: %s", schedule_id, response.workflow_trigger_log_id) + + except Exception as e: + # Don't retry - schedule will run again at next interval + logger.error("Failed to trigger workflow for schedule %s: %s", schedule_id, e, exc_info=True) diff --git a/dev/start-worker b/dev/start-worker index a05d3bcc48e4bb..5df9ed4a838470 100755 --- a/dev/start-worker +++ b/dev/start-worker @@ -24,6 +24,7 @@ show_help() { echo " workflow_professional - Professional tier workflows (cloud edition)" echo " workflow_team - Team tier workflows (cloud edition)" echo " workflow_sandbox - Sandbox tier workflows (cloud edition)" + echo " schedule - Scheduled workflow executions" echo " generation - Content generation tasks" echo " mail - Email notifications" echo " ops_trace - Operations tracing" @@ -79,10 +80,10 @@ if [[ -z "${QUEUES}" ]]; then # Configure queues based on edition if [[ "${EDITION}" == "CLOUD" ]]; then # Cloud edition: separate queues for dataset and trigger tasks - QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow_professional,workflow_team,workflow_sandbox" + QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow_professional,workflow_team,workflow_sandbox,schedule" else # Community edition (SELF_HOSTED): dataset and workflow have separate queues - QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow" + QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,workflow,schedule" fi echo "No queues specified, using edition-based defaults: ${QUEUES}" diff --git a/dev/start-workflow-scheduler b/dev/start-workflow-scheduler new file mode 100755 index 00000000000000..a2129450c0d722 --- /dev/null +++ b/dev/start-workflow-scheduler @@ -0,0 +1,75 @@ +#!/bin/bash + +set -x + +# Help function +show_help() { + echo "Usage: $0 [OPTIONS]" + echo "" + echo "Start the Workflow Scheduler service" + echo "This is a dedicated Celery Beat instance for workflow schedules" + echo "" + echo "Options:" + echo " --loglevel LEVEL Log level (default: INFO)" + echo " --pidfile FILE PID file path (optional)" + echo " -h, --help Show this help message" + echo "" + echo "Examples:" + echo " $0" + echo " $0 --loglevel DEBUG" + echo " $0 --pidfile /tmp/workflow-scheduler.pid" + echo "" + echo "Description:" + echo " The Workflow Scheduler is a required service when using schedule trigger nodes." + echo " It reads schedule configurations from the database and dispatches tasks" + echo " at the appropriate times." +} + +# Parse command line arguments +LOGLEVEL="INFO" +PIDFILE="" + +while [[ $# -gt 0 ]]; do + case $1 in + --loglevel) + LOGLEVEL="$2" + shift 2 + ;; + --pidfile) + PIDFILE="$2" + shift 2 + ;; + -h|--help) + show_help + exit 0 + ;; + *) + echo "Unknown option: $1" + show_help + exit 1 + ;; + esac +done + +SCRIPT_DIR="$(dirname "$(realpath "$0")")" +cd "$SCRIPT_DIR/.." + +echo "Starting Workflow Scheduler..." +echo " Log Level: ${LOGLEVEL}" +if [[ -n "${PIDFILE}" ]]; then + echo " PID File: ${PIDFILE}" +fi + +# Build celery beat command with custom scheduler +CMD="uv --directory api run celery -A app.celery beat" +CMD="${CMD} --scheduler schedule.schedule_dispatch:WorkflowScheduler" +CMD="${CMD} --loglevel ${LOGLEVEL}" + +if [[ -n "${PIDFILE}" ]]; then + CMD="${CMD} --pidfile ${PIDFILE}" +fi + +echo "Using scheduler: schedule.schedule_dispatch:WorkflowScheduler" + +# Run the workflow scheduler +exec ${CMD} \ No newline at end of file diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 0c352e4658fcda..cb017d0fc02b02 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -649,6 +649,25 @@ services: - ssrf_proxy_network - default + # Workflow scheduler service (Required for schedule trigger nodes) + # This is a dedicated Celery Beat instance for workflow schedules. + workflow_scheduler: + image: langgenius/dify-api:1.7.2 + restart: always + environment: + # Use the shared environment variables. + <<: *shared-api-worker-env + # Startup mode for workflow scheduler + MODE: workflow-scheduler + depends_on: + db: + condition: service_healthy + redis: + condition: service_started + networks: + - ssrf_proxy_network + - default + # Frontend web application. web: image: langgenius/dify-web:1.7.2 From 081c61c591f0446516df41cadbe7cb65dec5d813 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 24 Aug 2025 23:20:43 +0000 Subject: [PATCH 02/26] [autofix.ci] apply automated fixes --- api/uv.lock | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/api/uv.lock b/api/uv.lock index 52eedd9c6607d0..7fb32316ff4b77 100644 --- a/api/uv.lock +++ b/api/uv.lock @@ -1138,6 +1138,19 @@ version = "1.7" source = { registry = "https://pypi.org/simple" } sdist = { url = "https://files.pythonhosted.org/packages/6b/b0/e595ce2a2527e169c3bcd6c33d2473c1918e0b7f6826a043ca1245dd4e5b/crcmod-1.7.tar.gz", hash = "sha256:dc7051a0db5f2bd48665a990d3ec1cc305a466a77358ca4492826f41f283601e", size = 89670, upload-time = "2010-06-27T14:35:29.538Z" } +[[package]] +name = "croniter" +version = "6.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "python-dateutil" }, + { name = "pytz" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/ad/2f/44d1ae153a0e27be56be43465e5cb39b9650c781e001e7864389deb25090/croniter-6.0.0.tar.gz", hash = "sha256:37c504b313956114a983ece2c2b07790b1f1094fe9d81cc94739214748255577", size = 64481, upload-time = "2024-12-17T17:17:47.32Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/07/4b/290b4c3efd6417a8b0c284896de19b1d5855e6dbdb97d2a35e68fa42de85/croniter-6.0.0-py2.py3-none-any.whl", hash = "sha256:2f878c3856f17896979b2a4379ba1f09c83e374931ea15cc835c5dd2eee9b368", size = 25468, upload-time = "2024-12-17T17:17:45.359Z" }, +] + [[package]] name = "cryptography" version = "45.0.5" @@ -1248,6 +1261,7 @@ dependencies = [ { name = "cachetools" }, { name = "celery" }, { name = "chardet" }, + { name = "croniter" }, { name = "flask" }, { name = "flask-compress" }, { name = "flask-cors" }, @@ -1436,6 +1450,7 @@ requires-dist = [ { name = "cachetools", specifier = "~=5.3.0" }, { name = "celery", specifier = "~=5.5.2" }, { name = "chardet", specifier = "~=5.1.0" }, + { name = "croniter", specifier = ">=6.0.0" }, { name = "flask", specifier = "~=3.1.2" }, { name = "flask-compress", specifier = "~=1.17" }, { name = "flask-cors", specifier = "~=6.0.0" }, From 55272f1010ccc0797df6e566f0e86c48f312cf00 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 28 Aug 2025 01:42:30 +0800 Subject: [PATCH 03/26] refactor: use beat_schedule instead of custom schedule --- api/.env.example | 4 + api/configs/feature/__init__.py | 12 + api/docker/entrypoint.sh | 5 - api/extensions/ext_celery.py | 7 +- ...-4558cfabe44e_add_workflow_trigger_logs.py | 6 +- ...5871f634954d_add_workflow_webhook_table.py | 2 +- ...1e06b2654c6c_add_workflow_schedule_plan.py | 52 ---- ...270f60_add_workflow_schedule_plan_table.py | 109 ++++++++ api/models/workflow.py | 46 ++-- api/schedule/schedule_dispatch.py | 239 ------------------ api/schedule/workflow_schedule_task.py | 83 ++++++ api/services/workflow/schedule_manager.py | 54 +--- api/services/workflow/schedule_sync.py | 20 +- api/services/workflow_service.py | 2 - api/tasks/workflow_schedule_tasks.py | 10 +- .../unit_tests/extensions/test_celery_ssl.py | 3 + dev/start-workflow-scheduler | 75 ------ docker/.env.example | 3 + docker/docker-compose.yaml | 22 +- 19 files changed, 265 insertions(+), 489 deletions(-) delete mode 100644 api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py create mode 100644 api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py delete mode 100644 api/schedule/schedule_dispatch.py create mode 100644 api/schedule/workflow_schedule_task.py delete mode 100755 dev/start-workflow-scheduler diff --git a/api/.env.example b/api/.env.example index e947c5584b136a..7d7c8165cecd62 100644 --- a/api/.env.example +++ b/api/.env.example @@ -502,6 +502,10 @@ ENABLE_CLEAN_MESSAGES=false ENABLE_MAIL_CLEAN_DOCUMENT_NOTIFY_TASK=false ENABLE_DATASETS_QUEUE_MONITOR=false ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK=true +ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK=true +# Interval time in minutes for polling scheduled workflows(default: 1 min) +WORKFLOW_SCHEDULE_POLLER_INTERVAL=1 +WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE=100 # Position configuration POSITION_TOOL_PINS= diff --git a/api/configs/feature/__init__.py b/api/configs/feature/__init__.py index 7638cd18996e2f..51dbee823d36d1 100644 --- a/api/configs/feature/__init__.py +++ b/api/configs/feature/__init__.py @@ -871,6 +871,18 @@ class CeleryScheduleTasksConfig(BaseSettings): description="Enable check upgradable plugin task", default=True, ) + ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK: bool = Field( + description="Enable workflow schedule poller task", + default=True, + ) + WORKFLOW_SCHEDULE_POLLER_INTERVAL: int = Field( + description="Workflow schedule poller interval in minutes", + default=1, + ) + WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE: int = Field( + description="Maximum number of schedules to process in each poll batch", + default=100, + ) class PositionConfig(BaseSettings): diff --git a/api/docker/entrypoint.sh b/api/docker/entrypoint.sh index 939feae814e3d3..b1aec05fd7be2d 100755 --- a/api/docker/entrypoint.sh +++ b/api/docker/entrypoint.sh @@ -68,11 +68,6 @@ if [[ "${MODE}" == "worker" ]]; then elif [[ "${MODE}" == "beat" ]]; then exec celery -A app.celery beat --loglevel ${LOG_LEVEL:-INFO} -elif [[ "${MODE}" == "workflow-scheduler" ]]; then - echo "Starting Workflow Scheduler..." - exec celery -A app.celery beat \ - --scheduler schedule.schedule_dispatch:WorkflowScheduler \ - --loglevel ${LOG_LEVEL:-INFO} else if [[ "${DEBUG}" == "true" ]]; then exec flask run --host=${DIFY_BIND_ADDRESS:-0.0.0.0} --port=${DIFY_PORT:-5001} --debug diff --git a/api/extensions/ext_celery.py b/api/extensions/ext_celery.py index d97f3d8836cbea..50fac304f51de8 100644 --- a/api/extensions/ext_celery.py +++ b/api/extensions/ext_celery.py @@ -98,7 +98,6 @@ def __call__(self, *args: object, **kwargs: object) -> object: imports = [ "tasks.async_workflow_tasks", # trigger workers - "tasks.workflow_schedule_tasks", # schedule trigger tasks ] day = dify_config.CELERY_BEAT_SCHEDULER_TIME @@ -161,6 +160,12 @@ def __call__(self, *args: object, **kwargs: object) -> object: "task": "schedule.clean_workflow_runlogs_precise.clean_workflow_runlogs_precise", "schedule": crontab(minute="0", hour="2"), } + if dify_config.ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK: + imports.append("schedule.workflow_schedule_task") + beat_schedule["workflow_schedule_task"] = { + "task": "schedule.workflow_schedule_task.poll_workflow_schedules", + "schedule": timedelta(minutes=dify_config.WORKFLOW_SCHEDULE_POLLER_INTERVAL), + } celery_app.conf.update(beat_schedule=beat_schedule, imports=imports) return celery_app diff --git a/api/migrations/versions/2025_08_23_2038-4558cfabe44e_add_workflow_trigger_logs.py b/api/migrations/versions/2025_08_23_2038-4558cfabe44e_add_workflow_trigger_logs.py index 79c05d0aaa5017..f39105d4c5f715 100644 --- a/api/migrations/versions/2025_08_23_2038-4558cfabe44e_add_workflow_trigger_logs.py +++ b/api/migrations/versions/2025_08_23_2038-4558cfabe44e_add_workflow_trigger_logs.py @@ -1,7 +1,7 @@ -"""empty message +"""Add workflow trigger logs table Revision ID: 4558cfabe44e -Revises: fa8b0fa6f407 +Revises: 0e154742a5fa Create Date: 2025-08-23 20:38:20.059323 """ @@ -12,7 +12,7 @@ # revision identifiers, used by Alembic. revision = '4558cfabe44e' -down_revision = 'fa8b0fa6f407' +down_revision = '0e154742a5fa' branch_labels = None depends_on = None diff --git a/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py b/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py index fe37696e5ca2d0..a28e1223c35877 100644 --- a/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py +++ b/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py @@ -1,7 +1,7 @@ """Add workflow webhook table Revision ID: 5871f634954d -Revises: fa8b0fa6f407 +Revises: 4558cfabe44e Create Date: 2025-08-23 20:39:20.704501 """ diff --git a/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py b/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py deleted file mode 100644 index 180efea42e6baa..00000000000000 --- a/api/migrations/versions/2025_08_24_1313-1e06b2654c6c_add_workflow_schedule_plan.py +++ /dev/null @@ -1,52 +0,0 @@ -"""add workflow_schedule_plan - -Revision ID: 1e06b2654c6c -Revises: 4558cfabe44e -Create Date: 2025-08-24 13:13:23.495063 - -""" -from alembic import op -import models as models -import sqlalchemy as sa - -# revision identifiers, used by Alembic. -revision = '1e06b2654c6c' -down_revision = '4558cfabe44e' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('workflow_schedule_plans', - sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuid_generate_v4()'), nullable=False), - sa.Column('tenant_id', models.types.StringUUID(), nullable=False), - sa.Column('app_id', models.types.StringUUID(), nullable=False), - sa.Column('workflow_id', models.types.StringUUID(), nullable=False), - sa.Column('root_node_id', sa.String(length=255), nullable=False), - sa.Column('cron_expression', sa.String(length=255), nullable=False), - sa.Column('timezone', sa.String(length=64), nullable=False), - sa.Column('enabled', sa.Boolean(), nullable=False), - sa.Column('next_run_at', sa.DateTime(timezone=True), nullable=True), - sa.Column('created_by', models.types.StringUUID(), nullable=False), - sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.PrimaryKeyConstraint('id', name='workflow_schedule_plan_pkey') - ) - with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: - batch_op.create_index('workflow_schedule_plan_app_workflow_idx', ['app_id', 'workflow_id'], unique=False) - batch_op.create_index('workflow_schedule_plan_created_at_idx', ['created_at'], unique=False) - batch_op.create_index('workflow_schedule_plan_tenant_enabled_next_idx', ['tenant_id', 'enabled', 'next_run_at'], unique=False) - - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: - batch_op.drop_index('workflow_schedule_plan_tenant_enabled_next_idx') - batch_op.drop_index('workflow_schedule_plan_created_at_idx') - batch_op.drop_index('workflow_schedule_plan_app_workflow_idx') - - op.drop_table('workflow_schedule_plans') - # ### end Alembic commands ### diff --git a/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py b/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py new file mode 100644 index 00000000000000..92c80b0117ae32 --- /dev/null +++ b/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py @@ -0,0 +1,109 @@ +"""add workflow_schedule_plan table + +Revision ID: 2ad189270f60 +Revises: 5871f634954d +Create Date: 2025-08-28 01:14:59.101946 + +""" +from alembic import op +import models as models +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '2ad189270f60' +down_revision = '5871f634954d' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('provider_credentials', + sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuidv7()'), nullable=False), + sa.Column('tenant_id', models.types.StringUUID(), nullable=False), + sa.Column('provider_name', sa.String(length=255), nullable=False), + sa.Column('credential_name', sa.String(length=255), nullable=False), + sa.Column('encrypted_config', sa.Text(), nullable=False), + sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.PrimaryKeyConstraint('id', name='provider_credential_pkey') + ) + with op.batch_alter_table('provider_credentials', schema=None) as batch_op: + batch_op.create_index('provider_credential_tenant_provider_idx', ['tenant_id', 'provider_name'], unique=False) + + op.create_table('provider_model_credentials', + sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuidv7()'), nullable=False), + sa.Column('tenant_id', models.types.StringUUID(), nullable=False), + sa.Column('provider_name', sa.String(length=255), nullable=False), + sa.Column('model_name', sa.String(length=255), nullable=False), + sa.Column('model_type', sa.String(length=40), nullable=False), + sa.Column('credential_name', sa.String(length=255), nullable=False), + sa.Column('encrypted_config', sa.Text(), nullable=False), + sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.PrimaryKeyConstraint('id', name='provider_model_credential_pkey') + ) + with op.batch_alter_table('provider_model_credentials', schema=None) as batch_op: + batch_op.create_index('provider_model_credential_tenant_provider_model_idx', ['tenant_id', 'provider_name', 'model_name', 'model_type'], unique=False) + + op.create_table('workflow_schedule_plans', + sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuid_generate_v4()'), nullable=False), + sa.Column('app_id', models.types.StringUUID(), nullable=False), + sa.Column('node_id', sa.String(length=64), nullable=False), + sa.Column('tenant_id', models.types.StringUUID(), nullable=False), + sa.Column('cron_expression', sa.String(length=255), nullable=False), + sa.Column('timezone', sa.String(length=64), nullable=False), + sa.Column('enabled', sa.Boolean(), nullable=False), + sa.Column('triggered_by', sa.String(length=16), nullable=False), + sa.Column('next_run_at', sa.DateTime(), nullable=True), + sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.PrimaryKeyConstraint('id', name='workflow_schedule_plan_pkey'), + sa.UniqueConstraint('app_id', 'node_id', 'triggered_by', name='uniq_app_node_trigger') + ) + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.create_index('workflow_schedule_plan_enabled_next_idx', ['enabled', 'next_run_at'], unique=False) + + with op.batch_alter_table('load_balancing_model_configs', schema=None) as batch_op: + batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) + batch_op.add_column(sa.Column('credential_source_type', sa.String(length=40), nullable=True)) + + with op.batch_alter_table('provider_models', schema=None) as batch_op: + batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) + batch_op.drop_column('encrypted_config') + + with op.batch_alter_table('providers', schema=None) as batch_op: + batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) + batch_op.drop_column('encrypted_config') + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('providers', schema=None) as batch_op: + batch_op.add_column(sa.Column('encrypted_config', sa.TEXT(), autoincrement=False, nullable=True)) + batch_op.drop_column('credential_id') + + with op.batch_alter_table('provider_models', schema=None) as batch_op: + batch_op.add_column(sa.Column('encrypted_config', sa.TEXT(), autoincrement=False, nullable=True)) + batch_op.drop_column('credential_id') + + with op.batch_alter_table('load_balancing_model_configs', schema=None) as batch_op: + batch_op.drop_column('credential_source_type') + batch_op.drop_column('credential_id') + + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.drop_index('workflow_schedule_plan_enabled_next_idx') + + op.drop_table('workflow_schedule_plans') + with op.batch_alter_table('provider_model_credentials', schema=None) as batch_op: + batch_op.drop_index('provider_model_credential_tenant_provider_model_idx') + + op.drop_table('provider_model_credentials') + with op.batch_alter_table('provider_credentials', schema=None) as batch_op: + batch_op.drop_index('provider_credential_tenant_provider_idx') + + op.drop_table('provider_credentials') + # ### end Alembic commands ### diff --git a/api/models/workflow.py b/api/models/workflow.py index a0ffc36415887c..e088a9ca3b4ce4 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -1431,34 +1431,30 @@ class WorkflowSchedulePlan(Base): Uses cron expressions with timezone support for flexible scheduling. Attributes: - id: UUID primary key - tenant_id: Workspace ID for multi-tenancy - app_id: Application ID this schedule belongs to - workflow_id: Workflow to trigger - root_node_id: Starting node ID for workflow execution - cron_expression: Cron expression defining schedule pattern - timezone: Timezone for cron evaluation (e.g., 'Asia/Shanghai') - enabled: Whether schedule is active - next_run_at: Next scheduled execution time (timestamptz) - created_by: Creator account ID - created_at: Creation timestamp - updated_at: Last update timestamp + - id (uuid) Primary key + - app_id (uuid) App ID to bind to a specific app + - node_id (varchar) Starting node ID for workflow execution + - tenant_id (uuid) Workspace ID for multi-tenancy + - cron_expression (varchar) Cron expression defining schedule pattern + - timezone (varchar) Timezone for cron evaluation (e.g., 'Asia/Shanghai') + - enabled (bool) Whether schedule is active + - triggered_by (varchar) Environment: debugger or production + - next_run_at (timestamp) Next scheduled execution time + - created_at (timestamp) Creation timestamp + - updated_at (timestamp) Last update timestamp """ __tablename__ = "workflow_schedule_plans" __table_args__ = ( sa.PrimaryKeyConstraint("id", name="workflow_schedule_plan_pkey"), - sa.Index("workflow_schedule_plan_tenant_enabled_next_idx", "tenant_id", "enabled", "next_run_at"), - sa.Index("workflow_schedule_plan_app_workflow_idx", "app_id", "workflow_id"), - sa.Index("workflow_schedule_plan_created_at_idx", "created_at"), + sa.UniqueConstraint("app_id", "node_id", "triggered_by", name="uniq_app_node_trigger"), + sa.Index("workflow_schedule_plan_enabled_next_idx", "enabled", "next_run_at"), ) id: Mapped[str] = mapped_column(StringUUID, server_default=sa.text("uuid_generate_v4()")) - tenant_id: Mapped[str] = mapped_column(StringUUID, nullable=False) app_id: Mapped[str] = mapped_column(StringUUID, nullable=False) - workflow_id: Mapped[str] = mapped_column(StringUUID, nullable=False) - - root_node_id: Mapped[str] = mapped_column(String(255), nullable=False) + node_id: Mapped[str] = mapped_column(String(64), nullable=False) + tenant_id: Mapped[str] = mapped_column(StringUUID, nullable=False) # Schedule configuration cron_expression: Mapped[str] = mapped_column(String(255), nullable=False) @@ -1466,9 +1462,8 @@ class WorkflowSchedulePlan(Base): # Schedule control enabled: Mapped[bool] = mapped_column(sa.Boolean, nullable=False, default=True) - next_run_at: Mapped[Optional[datetime]] = mapped_column(DateTime(timezone=True), nullable=True) - - created_by: Mapped[str] = mapped_column(StringUUID, nullable=False) + triggered_by: Mapped[str] = mapped_column(String(16), nullable=False) + next_run_at: Mapped[Optional[datetime]] = mapped_column(DateTime, nullable=True) created_at: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=func.current_timestamp()) updated_at: Mapped[datetime] = mapped_column( DateTime, nullable=False, server_default=func.current_timestamp(), onupdate=func.current_timestamp() @@ -1478,15 +1473,14 @@ def to_dict(self) -> dict: """Convert to dictionary representation""" return { "id": self.id, - "tenant_id": self.tenant_id, "app_id": self.app_id, - "workflow_id": self.workflow_id, - "root_node_id": self.root_node_id, + "node_id": self.node_id, + "tenant_id": self.tenant_id, "cron_expression": self.cron_expression, "timezone": self.timezone, "enabled": self.enabled, "next_run_at": self.next_run_at.isoformat() if self.next_run_at else None, - "created_by": self.created_by, + "triggered_by": self.triggered_by, "created_at": self.created_at.isoformat(), "updated_at": self.updated_at.isoformat(), } diff --git a/api/schedule/schedule_dispatch.py b/api/schedule/schedule_dispatch.py deleted file mode 100644 index 9a5d5e4d00dc5d..00000000000000 --- a/api/schedule/schedule_dispatch.py +++ /dev/null @@ -1,239 +0,0 @@ -import logging -import os -import time -from datetime import UTC, datetime - -import sqlalchemy as sa -from celery.beat import Scheduler -from sqlalchemy import create_engine, func, select -from sqlalchemy.orm import Session - -from configs import dify_config -from models.workflow import WorkflowSchedulePlan -from services.workflow.schedule_manager import ScheduleService -from tasks.workflow_schedule_tasks import run_schedule_trigger - -logger = logging.getLogger(__name__) - -# Standalone engine because Beat runs in separate process without Flask context -engine = create_engine( - dify_config.SQLALCHEMY_DATABASE_URI, - pool_pre_ping=True, - pool_recycle=300, - # pool_size=2, # Small pool for Beat's limited concurrent needs - # max_overflow=3, # Allow a few extra connections if needed - # pool_timeout=10, # Fail fast if pool exhausted -) - - -class WorkflowScheduler(Scheduler): - """ - Celery Beat scheduler for workflow schedule triggers. - - Features: - - Dynamic schedule configuration from database - - High availability support with SKIP LOCKED - - Real-time schedule updates without restart - - Misfire handling with grace period - """ - - def __init__(self, *args, **kwargs): - # Must initialize before super() because setup_schedule() needs these - self._last_sync = None - self.sync_every = int(os.getenv("WORKFLOW_SCHEDULER_SYNC_EVERY", "60")) - self.misfire_grace_time = int(os.getenv("WORKFLOW_SCHEDULER_MISFIRE_GRACE", "300")) - self.min_tick_interval = float(os.getenv("WORKFLOW_SCHEDULER_MIN_TICK", "5.0")) - self.max_tick_interval = float(os.getenv("WORKFLOW_SCHEDULER_MAX_TICK", "60.0")) - self.batch_size = int(os.getenv("WORKFLOW_SCHEDULER_BATCH_SIZE", "100")) - - logger.info( - "WorkflowScheduler initialized with: sync_every=%ds, " - "misfire_grace=%ds, tick_interval=[%.1f, %.1f]s, batch_size=%d", - self.sync_every, - self.misfire_grace_time, - self.min_tick_interval, - self.max_tick_interval, - self.batch_size, - ) - - super().__init__(*args, **kwargs) - - @staticmethod - def ensure_utc(dt): - """Ensure datetime is UTC-aware.""" - if dt and dt.tzinfo is None: - return dt.replace(tzinfo=UTC) - return dt - - def setup_schedule(self): - """Initial schedule setup from database.""" - self.sync_schedules() - - def sync_schedules(self): - """ - Synchronize schedules from database. - - This method validates that schedules exist and are properly configured. - """ - with Session(engine) as session: - # Count enabled schedules for monitoring - enabled_count = ( - session.scalar( - select(func.count(WorkflowSchedulePlan.id)).where( - WorkflowSchedulePlan.enabled == True, - WorkflowSchedulePlan.next_run_at.isnot(None), - ) - ) - or 0 - ) - - logger.debug("Found %d enabled schedules", enabled_count) - - self._last_sync = time.monotonic() - - def tick(self): - """ - Main scheduler tick - called periodically by Celery Beat. - - This method: - 1. Syncs with database if needed - 2. Finds and dispatches due schedules - 3. Uses SELECT FOR UPDATE SKIP LOCKED for HA - - Returns: - Seconds until next tick - """ - if self._last_sync is None or (time.monotonic() - self._last_sync) > self.sync_every: - logger.debug("Syncing schedules with database") - self.sync_schedules() - - now = datetime.now(UTC) - - with Session(engine) as session: - # SKIP LOCKED ensures only one Beat instance processes each schedule in HA setup - due_schedules = session.scalars( - select(WorkflowSchedulePlan) - .where( - WorkflowSchedulePlan.enabled == True, - WorkflowSchedulePlan.next_run_at <= now, - WorkflowSchedulePlan.next_run_at.isnot(None), - ) - .with_for_update(skip_locked=True) - .limit(self.batch_size) # Limit to prevent memory issues - ).all() - - if due_schedules: - logger.info("Processing %d due schedules", len(due_schedules)) - - schedules_to_update = [] - schedules_to_disable = [] - - for schedule in due_schedules: - try: - skip_execution = False - if schedule.next_run_at: - next_run_at = self.ensure_utc(schedule.next_run_at) - time_since_due = (now - next_run_at).total_seconds() - - if time_since_due > self.misfire_grace_time: - logger.warning( - "Schedule %s misfired (due %.1fs ago) - skipping execution", - schedule.id, - time_since_due, - ) - skip_execution = True - - if not skip_execution: - result = run_schedule_trigger.delay(schedule.id) - logger.info("Dispatched schedule %s, task ID: %s", schedule.id, result.id) - - # Calculate next run immediately to prevent duplicate triggers - next_run_at = ScheduleService.calculate_next_run_at( - schedule.cron_expression, - schedule.timezone, - base_time=now, - ) - - if next_run_at is None: - schedules_to_disable.append(schedule.id) - logger.info("Schedule %s has no more runs, will disable", schedule.id) - else: - schedules_to_update.append((schedule.id, next_run_at)) - logger.debug("Schedule %s next run at %s", schedule.id, next_run_at) - - except Exception as e: - logger.error("Error processing schedule %s: %s", schedule.id, e, exc_info=True) - - # Bulk update to reduce database round trips from N to 1 - try: - if schedules_to_update: - from sqlalchemy import case - - stmt = ( - sa.update(WorkflowSchedulePlan) - .where(WorkflowSchedulePlan.id.in_([s[0] for s in schedules_to_update])) - .values(next_run_at=case(dict(schedules_to_update), value=WorkflowSchedulePlan.id)) - ) - session.execute(stmt) - - if schedules_to_disable: - stmt = ( - sa.update(WorkflowSchedulePlan) - .where(WorkflowSchedulePlan.id.in_(schedules_to_disable)) - .values(enabled=False, next_run_at=None) - ) - session.execute(stmt) - - session.commit() - - if schedules_to_update or schedules_to_disable: - logger.info( - "Bulk updated %d schedules, disabled %d", - len(schedules_to_update), - len(schedules_to_disable), - ) - - except Exception as e: - logger.error("Error bulk updating schedules: %s", e, exc_info=True) - session.rollback() - - next_schedule = session.scalar( - select(func.min(WorkflowSchedulePlan.next_run_at)).where( - WorkflowSchedulePlan.enabled == True, - WorkflowSchedulePlan.next_run_at > now, - ) - ) - - if next_schedule: - next_schedule = self.ensure_utc(next_schedule) - seconds_until_next = (next_schedule - now).total_seconds() - # Cap to avoid too frequent checks or missing schedules - return max(self.min_tick_interval, min(seconds_until_next, self.max_tick_interval)) - - return self.max_tick_interval - - @property - def info(self): - """Return scheduler info for debugging.""" - with Session(engine) as session: - schedule_count = ( - session.scalar( - select(func.count(WorkflowSchedulePlan.id)).where( - WorkflowSchedulePlan.enabled == True, - WorkflowSchedulePlan.next_run_at.isnot(None), - ) - ) - or 0 - ) - - return { - "scheduler": "WorkflowScheduler", - "last_sync": self._last_sync if self._last_sync else None, - "enabled_schedules": schedule_count, - "config": { - "sync_every": self.sync_every, - "misfire_grace_time": self.misfire_grace_time, - "min_tick_interval": self.min_tick_interval, - "max_tick_interval": self.max_tick_interval, - }, - } diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py new file mode 100644 index 00000000000000..5e3d072b1df73d --- /dev/null +++ b/api/schedule/workflow_schedule_task.py @@ -0,0 +1,83 @@ +import logging +from datetime import UTC, datetime + +from celery import shared_task +from sqlalchemy import func, select +from sqlalchemy.orm import sessionmaker + +from configs import dify_config +from extensions.ext_database import db +from models.workflow import WorkflowSchedulePlan +from services.workflow.schedule_manager import ScheduleService +from tasks.workflow_schedule_tasks import run_schedule_trigger + +logger = logging.getLogger(__name__) + + +@shared_task(queue="schedule") +def poll_workflow_schedules(): + """ + Poll for due workflow schedules and dispatch them for execution. + + This task runs every minute via beat_schedule to check for schedules + that need to be triggered. + """ + session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) + + with session_factory() as session: + now = datetime.now(UTC) + + # Find all due schedules with FOR UPDATE SKIP LOCKED for HA support + due_schedules = session.scalars( + select(WorkflowSchedulePlan) + .where( + WorkflowSchedulePlan.enabled == True, + WorkflowSchedulePlan.next_run_at <= now, + WorkflowSchedulePlan.next_run_at.isnot(None), + ) + .with_for_update(skip_locked=True) + .limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE) + ).all() + + if due_schedules: + logger.info("Processing %d due schedules", len(due_schedules)) + + schedules_to_update = [] + schedules_to_disable = [] + + for schedule in due_schedules: + try: + result = run_schedule_trigger.delay(schedule.id) + logger.info("Dispatched schedule %s, task ID: %s", schedule.id, result.id) + + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + base_time=now, + ) + + if next_run_at is None: + schedule.enabled = False + schedule.next_run_at = None + schedules_to_disable.append(schedule.id) + logger.info("Schedule %s has no more runs, disabling", schedule.id) + else: + # Update next run time + schedule.next_run_at = next_run_at + schedules_to_update.append(schedule.id) + logger.debug("Schedule %s next run at %s", schedule.id, next_run_at) + + except Exception as e: + logger.error("Error processing schedule %s: %s", schedule.id, e, exc_info=True) + + if schedules_to_update or schedules_to_disable: + try: + session.commit() + logger.info( + "Updated %d schedules, disabled %d", + len(schedules_to_update), + len(schedules_to_disable), + ) + except Exception as e: + logger.error("Error committing schedule updates: %s", e, exc_info=True) + session.rollback() \ No newline at end of file diff --git a/api/services/workflow/schedule_manager.py b/api/services/workflow/schedule_manager.py index 2bf7909d8486b8..045e390c98c2ff 100644 --- a/api/services/workflow/schedule_manager.py +++ b/api/services/workflow/schedule_manager.py @@ -38,11 +38,10 @@ def create_schedule( session: Session, tenant_id: str, app_id: str, - workflow_id: str, - root_node_id: str, + node_id: str, cron_expression: str, timezone: str, - created_by: str, + triggered_by: str, enabled: bool = True, ) -> WorkflowSchedulePlan: """ @@ -52,11 +51,10 @@ def create_schedule( session: Database session tenant_id: Tenant ID app_id: Application ID - workflow_id: Workflow ID to trigger - root_node_id: Starting node ID + node_id: Starting node ID cron_expression: Cron expression timezone: Timezone for cron evaluation - created_by: Creator account ID + triggered_by: Trigger context - 'debugger' or 'production' enabled: Whether schedule is enabled Returns: @@ -72,13 +70,12 @@ def create_schedule( schedule = WorkflowSchedulePlan( tenant_id=tenant_id, app_id=app_id, - workflow_id=workflow_id, - root_node_id=root_node_id, + node_id=node_id, cron_expression=cron_expression, timezone=timezone, enabled=enabled and next_run_at is not None, next_run_at=next_run_at, - created_by=created_by, + triggered_by=triggered_by, ) session.add(schedule) @@ -165,45 +162,6 @@ def get_schedule(session: Session, schedule_id: str) -> Optional[WorkflowSchedul """ return session.get(WorkflowSchedulePlan, schedule_id) - @staticmethod - def list_schedules( - session: Session, - tenant_id: str, - app_id: Optional[str] = None, - workflow_id: Optional[str] = None, - enabled: Optional[bool] = None, - limit: int = 100, - offset: int = 0, - ) -> list[WorkflowSchedulePlan]: - """ - List schedules with optional filters. - - Args: - session: Database session - tenant_id: Tenant ID (required) - app_id: Optional app ID filter - workflow_id: Optional workflow ID filter - enabled: Optional enabled status filter - limit: Maximum results to return - offset: Number of results to skip - - Returns: - List of WorkflowSchedulePlan instances - """ - query = select(WorkflowSchedulePlan).where(WorkflowSchedulePlan.tenant_id == tenant_id) - - if app_id: - query = query.where(WorkflowSchedulePlan.app_id == app_id) - if workflow_id: - query = query.where(WorkflowSchedulePlan.workflow_id == workflow_id) - if enabled is not None: - query = query.where(WorkflowSchedulePlan.enabled == enabled) - - query = query.order_by(WorkflowSchedulePlan.created_at.desc()) - query = query.limit(limit).offset(offset) - - return list(session.scalars(query).all()) - @staticmethod def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: """ diff --git a/api/services/workflow/schedule_sync.py b/api/services/workflow/schedule_sync.py index 245f757b79f69c..c42669d0c6c7d2 100644 --- a/api/services/workflow/schedule_sync.py +++ b/api/services/workflow/schedule_sync.py @@ -17,9 +17,7 @@ def sync_schedule_from_graph( session: Session, tenant_id: str, app_id: str, - workflow_id: str, graph: str, - created_by: str, ) -> Optional[WorkflowSchedulePlan]: try: graph_data = json.loads(graph) @@ -28,7 +26,7 @@ def sync_schedule_from_graph( # Find schedule trigger node in graph schedule_node = None - root_node_id = None + node_id = None schedule_nodes_count = 0 for node in graph_data.get("nodes", []): @@ -37,13 +35,12 @@ def sync_schedule_from_graph( schedule_nodes_count += 1 if not schedule_node: # Take the first schedule node schedule_node = node_data - root_node_id = node.get("id", "start") + node_id = node.get("id", "start") if schedule_nodes_count > 1: logger.warning("Found %d schedule nodes in workflow, only the first one will be used", schedule_nodes_count) # Get existing schedule plan for this app - # Note: We use app_id as the key since workflow_id changes with each publish existing_plan = session.scalar( select(WorkflowSchedulePlan).where( WorkflowSchedulePlan.tenant_id == tenant_id, @@ -56,7 +53,6 @@ def sync_schedule_from_graph( if existing_plan: logger.info("No schedule node in workflow for app %s, disabling schedule plan", app_id) updates = { - "workflow_id": workflow_id, "enabled": False, # Disable but keep the plan "next_run_at": None, # Clear next run time } @@ -93,10 +89,9 @@ def sync_schedule_from_graph( # Update existing plan or create new one if existing_plan: # Update existing schedule with new workflow version - logger.info("Updating schedule plan for app %s with new workflow %s", app_id, workflow_id) + logger.info("Updating schedule plan for app %s with new workflow %s", app_id) updates = { - "workflow_id": workflow_id, # Update to latest workflow version - "root_node_id": root_node_id, + "node_id": node_id, "cron_expression": cron_expression, "timezone": timezone, "enabled": enabled, @@ -109,17 +104,16 @@ def sync_schedule_from_graph( return updated_plan else: # Create new schedule - logger.info("Creating new schedule plan for app %s, workflow %s", app_id, workflow_id) + logger.info("Creating new schedule plan for app %s, workflow %s", app_id) new_plan = ScheduleService.create_schedule( session=session, tenant_id=tenant_id, app_id=app_id, - workflow_id=workflow_id, - root_node_id=root_node_id, + node_id=node_id, cron_expression=cron_expression, timezone=timezone, - created_by=created_by, enabled=enabled, + triggered_by="production", ) return new_plan diff --git a/api/services/workflow_service.py b/api/services/workflow_service.py index 6c02594471a6fb..7c036fcb28b7ed 100644 --- a/api/services/workflow_service.py +++ b/api/services/workflow_service.py @@ -293,9 +293,7 @@ def publish_workflow( session=session, tenant_id=app_model.tenant_id, app_id=app_model.id, - workflow_id=workflow.id, graph=workflow.graph, - created_by=account.id, ) # trigger app workflow events diff --git a/api/tasks/workflow_schedule_tasks.py b/api/tasks/workflow_schedule_tasks.py index d901e0c9d3ec0a..c973526ef0885a 100644 --- a/api/tasks/workflow_schedule_tasks.py +++ b/api/tasks/workflow_schedule_tasks.py @@ -9,14 +9,14 @@ from models.enums import WorkflowRunTriggeredFrom from models.workflow import WorkflowSchedulePlan from services.async_workflow_service import AsyncWorkflowService -from services.workflow.entities import TriggerData +from services.workflow.entities import AsyncTriggerResponse, TriggerData from services.workflow.schedule_manager import ScheduleService logger = logging.getLogger(__name__) @shared_task(queue="schedule") -def run_schedule_trigger(schedule_id: str): +def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: """ Execute a scheduled workflow trigger. @@ -51,15 +51,15 @@ def run_schedule_trigger(schedule_id: str): user=tenant_owner, trigger_data=TriggerData( app_id=schedule.app_id, - workflow_id=schedule.workflow_id, - root_node_id=schedule.root_node_id, + root_node_id=schedule.node_id, + # TODO: need `workflow_id` when triggered_by = `debugger` trigger_type=WorkflowRunTriggeredFrom.SCHEDULE, inputs=inputs, tenant_id=schedule.tenant_id, ), ) logger.info("Schedule %s triggered workflow: %s", schedule_id, response.workflow_trigger_log_id) + return response except Exception as e: - # Don't retry - schedule will run again at next interval logger.error("Failed to trigger workflow for schedule %s: %s", schedule_id, e, exc_info=True) diff --git a/api/tests/unit_tests/extensions/test_celery_ssl.py b/api/tests/unit_tests/extensions/test_celery_ssl.py index bc46fe8322a4a6..31719fc05ee877 100644 --- a/api/tests/unit_tests/extensions/test_celery_ssl.py +++ b/api/tests/unit_tests/extensions/test_celery_ssl.py @@ -131,6 +131,9 @@ def test_celery_init_applies_ssl_to_broker_and_backend(self): mock_config.ENABLE_MAIL_CLEAN_DOCUMENT_NOTIFY_TASK = False mock_config.ENABLE_DATASETS_QUEUE_MONITOR = False mock_config.ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK = False + mock_config.ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK = False + mock_config.WORKFLOW_SCHEDULE_POLLER_INTERVAL = 1 + mock_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE = 100 with patch("extensions.ext_celery.dify_config", mock_config): from dify_app import DifyApp diff --git a/dev/start-workflow-scheduler b/dev/start-workflow-scheduler deleted file mode 100755 index a2129450c0d722..00000000000000 --- a/dev/start-workflow-scheduler +++ /dev/null @@ -1,75 +0,0 @@ -#!/bin/bash - -set -x - -# Help function -show_help() { - echo "Usage: $0 [OPTIONS]" - echo "" - echo "Start the Workflow Scheduler service" - echo "This is a dedicated Celery Beat instance for workflow schedules" - echo "" - echo "Options:" - echo " --loglevel LEVEL Log level (default: INFO)" - echo " --pidfile FILE PID file path (optional)" - echo " -h, --help Show this help message" - echo "" - echo "Examples:" - echo " $0" - echo " $0 --loglevel DEBUG" - echo " $0 --pidfile /tmp/workflow-scheduler.pid" - echo "" - echo "Description:" - echo " The Workflow Scheduler is a required service when using schedule trigger nodes." - echo " It reads schedule configurations from the database and dispatches tasks" - echo " at the appropriate times." -} - -# Parse command line arguments -LOGLEVEL="INFO" -PIDFILE="" - -while [[ $# -gt 0 ]]; do - case $1 in - --loglevel) - LOGLEVEL="$2" - shift 2 - ;; - --pidfile) - PIDFILE="$2" - shift 2 - ;; - -h|--help) - show_help - exit 0 - ;; - *) - echo "Unknown option: $1" - show_help - exit 1 - ;; - esac -done - -SCRIPT_DIR="$(dirname "$(realpath "$0")")" -cd "$SCRIPT_DIR/.." - -echo "Starting Workflow Scheduler..." -echo " Log Level: ${LOGLEVEL}" -if [[ -n "${PIDFILE}" ]]; then - echo " PID File: ${PIDFILE}" -fi - -# Build celery beat command with custom scheduler -CMD="uv --directory api run celery -A app.celery beat" -CMD="${CMD} --scheduler schedule.schedule_dispatch:WorkflowScheduler" -CMD="${CMD} --loglevel ${LOGLEVEL}" - -if [[ -n "${PIDFILE}" ]]; then - CMD="${CMD} --pidfile ${PIDFILE}" -fi - -echo "Using scheduler: schedule.schedule_dispatch:WorkflowScheduler" - -# Run the workflow scheduler -exec ${CMD} \ No newline at end of file diff --git a/docker/.env.example b/docker/.env.example index c6ed2acb35e0c8..206e9bfcdc9eb1 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -1263,3 +1263,6 @@ ENABLE_CLEAN_MESSAGES=false ENABLE_MAIL_CLEAN_DOCUMENT_NOTIFY_TASK=false ENABLE_DATASETS_QUEUE_MONITOR=false ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK=true +ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK=true +WORKFLOW_SCHEDULE_POLLER_INTERVAL=1 +WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE=100 diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 5beadaf0a65d35..bd676a56988fb4 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -576,6 +576,9 @@ x-shared-env: &shared-api-worker-env ENABLE_MAIL_CLEAN_DOCUMENT_NOTIFY_TASK: ${ENABLE_MAIL_CLEAN_DOCUMENT_NOTIFY_TASK:-false} ENABLE_DATASETS_QUEUE_MONITOR: ${ENABLE_DATASETS_QUEUE_MONITOR:-false} ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK: ${ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK:-true} + ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK: ${ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK:-true} + WORKFLOW_SCHEDULE_POLLER_INTERVAL: ${WORKFLOW_SCHEDULE_POLLER_INTERVAL:-1} + WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE: ${WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE:-100} services: # API service @@ -652,25 +655,6 @@ services: - ssrf_proxy_network - default - # Workflow scheduler service (Required for schedule trigger nodes) - # This is a dedicated Celery Beat instance for workflow schedules. - workflow_scheduler: - image: langgenius/dify-api:1.7.2 - restart: always - environment: - # Use the shared environment variables. - <<: *shared-api-worker-env - # Startup mode for workflow scheduler - MODE: workflow-scheduler - depends_on: - db: - condition: service_healthy - redis: - condition: service_started - networks: - - ssrf_proxy_network - - default - # Frontend web application. web: image: langgenius/dify-web:1.7.2 From 9ff831850e6942badea439aa3dc0f7d39deafbf5 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 01:22:14 +0800 Subject: [PATCH 04/26] refactor: update workflow_schedule_plans table structrue & add daily limit checked before dispatching tasks --- .../console/app/workflow_trigger.py | 8 +- api/core/workflow/nodes/node_mapping.py | 2 +- ...5871f634954d_add_workflow_webhook_table.py | 2 +- ...270f60_add_workflow_schedule_plan_table.py | 109 -------- .../versions/2025_08_28_2052-c19938f630b6_.py | 47 ++++ api/models/workflow.py | 12 +- api/schedule/workflow_schedule_task.py | 236 ++++++++++++----- ...chedule_manager.py => schedule_service.py} | 44 +-- api/services/workflow/schedule_sync.py | 250 ------------------ api/services/workflow_service.py | 10 - api/tasks/workflow_schedule_tasks.py | 16 +- 11 files changed, 246 insertions(+), 490 deletions(-) delete mode 100644 api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py create mode 100644 api/migrations/versions/2025_08_28_2052-c19938f630b6_.py rename api/services/{workflow/schedule_manager.py => schedule_service.py} (81%) delete mode 100644 api/services/workflow/schedule_sync.py diff --git a/api/controllers/console/app/workflow_trigger.py b/api/controllers/console/app/workflow_trigger.py index 099dbea02f2e40..2a099935a8a3f9 100644 --- a/api/controllers/console/app/workflow_trigger.py +++ b/api/controllers/console/app/workflow_trigger.py @@ -19,10 +19,6 @@ logger = logging.getLogger(__name__) - - - - class WebhookTriggerApi(Resource): """Webhook Trigger API""" @@ -87,7 +83,7 @@ def post(self, app_model): base_url = dify_config.SERVICE_API_URL webhook_trigger.webhook_url = f"{base_url}/triggers/webhook/{webhook_trigger.webhook_id}" webhook_trigger.webhook_debug_url = f"{base_url}/triggers/webhook-debug/{webhook_trigger.webhook_id}" - + return webhook_trigger @setup_required @@ -231,7 +227,7 @@ def post(self, app_model): trigger.icon = url_prefix + trigger.provider_name + "/icon" else: trigger.icon = "" - + return trigger diff --git a/api/core/workflow/nodes/node_mapping.py b/api/core/workflow/nodes/node_mapping.py index f449dce4658f5c..aa1f84eb5d0ad8 100644 --- a/api/core/workflow/nodes/node_mapping.py +++ b/api/core/workflow/nodes/node_mapping.py @@ -19,8 +19,8 @@ from core.workflow.nodes.start import StartNode from core.workflow.nodes.template_transform import TemplateTransformNode from core.workflow.nodes.tool import ToolNode -from core.workflow.nodes.trigger_webhook import TriggerWebhookNode from core.workflow.nodes.trigger_schedule import TriggerScheduleNode +from core.workflow.nodes.trigger_webhook import TriggerWebhookNode from core.workflow.nodes.variable_aggregator import VariableAggregatorNode from core.workflow.nodes.variable_assigner.v1 import VariableAssignerNode as VariableAssignerNodeV1 from core.workflow.nodes.variable_assigner.v2 import VariableAssignerNode as VariableAssignerNodeV2 diff --git a/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py b/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py index 20099905e7c5d2..24fe84b6ac93fa 100644 --- a/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py +++ b/api/migrations/versions/2025_08_23_2039-5871f634954d_add_workflow_webhook_table.py @@ -1,7 +1,7 @@ """Add workflow webhook table Revision ID: 5871f634954d -Revises: 4558cfabe44e +Revises: fa8b0fa6f407 Create Date: 2025-08-23 20:39:20.704501 """ diff --git a/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py b/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py deleted file mode 100644 index 92c80b0117ae32..00000000000000 --- a/api/migrations/versions/2025_08_28_0114-2ad189270f60_add_workflow_schedule_plan_table.py +++ /dev/null @@ -1,109 +0,0 @@ -"""add workflow_schedule_plan table - -Revision ID: 2ad189270f60 -Revises: 5871f634954d -Create Date: 2025-08-28 01:14:59.101946 - -""" -from alembic import op -import models as models -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '2ad189270f60' -down_revision = '5871f634954d' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('provider_credentials', - sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuidv7()'), nullable=False), - sa.Column('tenant_id', models.types.StringUUID(), nullable=False), - sa.Column('provider_name', sa.String(length=255), nullable=False), - sa.Column('credential_name', sa.String(length=255), nullable=False), - sa.Column('encrypted_config', sa.Text(), nullable=False), - sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.PrimaryKeyConstraint('id', name='provider_credential_pkey') - ) - with op.batch_alter_table('provider_credentials', schema=None) as batch_op: - batch_op.create_index('provider_credential_tenant_provider_idx', ['tenant_id', 'provider_name'], unique=False) - - op.create_table('provider_model_credentials', - sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuidv7()'), nullable=False), - sa.Column('tenant_id', models.types.StringUUID(), nullable=False), - sa.Column('provider_name', sa.String(length=255), nullable=False), - sa.Column('model_name', sa.String(length=255), nullable=False), - sa.Column('model_type', sa.String(length=40), nullable=False), - sa.Column('credential_name', sa.String(length=255), nullable=False), - sa.Column('encrypted_config', sa.Text(), nullable=False), - sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.PrimaryKeyConstraint('id', name='provider_model_credential_pkey') - ) - with op.batch_alter_table('provider_model_credentials', schema=None) as batch_op: - batch_op.create_index('provider_model_credential_tenant_provider_model_idx', ['tenant_id', 'provider_name', 'model_name', 'model_type'], unique=False) - - op.create_table('workflow_schedule_plans', - sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuid_generate_v4()'), nullable=False), - sa.Column('app_id', models.types.StringUUID(), nullable=False), - sa.Column('node_id', sa.String(length=64), nullable=False), - sa.Column('tenant_id', models.types.StringUUID(), nullable=False), - sa.Column('cron_expression', sa.String(length=255), nullable=False), - sa.Column('timezone', sa.String(length=64), nullable=False), - sa.Column('enabled', sa.Boolean(), nullable=False), - sa.Column('triggered_by', sa.String(length=16), nullable=False), - sa.Column('next_run_at', sa.DateTime(), nullable=True), - sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.PrimaryKeyConstraint('id', name='workflow_schedule_plan_pkey'), - sa.UniqueConstraint('app_id', 'node_id', 'triggered_by', name='uniq_app_node_trigger') - ) - with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: - batch_op.create_index('workflow_schedule_plan_enabled_next_idx', ['enabled', 'next_run_at'], unique=False) - - with op.batch_alter_table('load_balancing_model_configs', schema=None) as batch_op: - batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) - batch_op.add_column(sa.Column('credential_source_type', sa.String(length=40), nullable=True)) - - with op.batch_alter_table('provider_models', schema=None) as batch_op: - batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) - batch_op.drop_column('encrypted_config') - - with op.batch_alter_table('providers', schema=None) as batch_op: - batch_op.add_column(sa.Column('credential_id', models.types.StringUUID(), nullable=True)) - batch_op.drop_column('encrypted_config') - - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table('providers', schema=None) as batch_op: - batch_op.add_column(sa.Column('encrypted_config', sa.TEXT(), autoincrement=False, nullable=True)) - batch_op.drop_column('credential_id') - - with op.batch_alter_table('provider_models', schema=None) as batch_op: - batch_op.add_column(sa.Column('encrypted_config', sa.TEXT(), autoincrement=False, nullable=True)) - batch_op.drop_column('credential_id') - - with op.batch_alter_table('load_balancing_model_configs', schema=None) as batch_op: - batch_op.drop_column('credential_source_type') - batch_op.drop_column('credential_id') - - with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: - batch_op.drop_index('workflow_schedule_plan_enabled_next_idx') - - op.drop_table('workflow_schedule_plans') - with op.batch_alter_table('provider_model_credentials', schema=None) as batch_op: - batch_op.drop_index('provider_model_credential_tenant_provider_model_idx') - - op.drop_table('provider_model_credentials') - with op.batch_alter_table('provider_credentials', schema=None) as batch_op: - batch_op.drop_index('provider_credential_tenant_provider_idx') - - op.drop_table('provider_credentials') - # ### end Alembic commands ### diff --git a/api/migrations/versions/2025_08_28_2052-c19938f630b6_.py b/api/migrations/versions/2025_08_28_2052-c19938f630b6_.py new file mode 100644 index 00000000000000..621dcc8a270c08 --- /dev/null +++ b/api/migrations/versions/2025_08_28_2052-c19938f630b6_.py @@ -0,0 +1,47 @@ +"""Add workflow schedule plan table + +Revision ID: c19938f630b6 +Revises: 9ee7d347f4c1 +Create Date: 2025-08-28 20:52:41.300028 + +""" +from alembic import op +import models as models +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c19938f630b6' +down_revision = '9ee7d347f4c1' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('workflow_schedule_plans', + sa.Column('id', models.types.StringUUID(), server_default=sa.text('uuidv7()'), nullable=False), + sa.Column('app_id', models.types.StringUUID(), nullable=False), + sa.Column('node_id', sa.String(length=64), nullable=False), + sa.Column('tenant_id', models.types.StringUUID(), nullable=False), + sa.Column('cron_expression', sa.String(length=255), nullable=False), + sa.Column('timezone', sa.String(length=64), nullable=False), + sa.Column('next_run_at', sa.DateTime(), nullable=True), + sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), + sa.PrimaryKeyConstraint('id', name='workflow_schedule_plan_pkey'), + sa.UniqueConstraint('app_id', 'node_id', name='uniq_app_node') + ) + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.create_index('workflow_schedule_plan_next_idx', ['next_run_at'], unique=False) + + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('workflow_schedule_plans', schema=None) as batch_op: + batch_op.drop_index('workflow_schedule_plan_next_idx') + + op.drop_table('workflow_schedule_plans') + # ### end Alembic commands ### diff --git a/api/models/workflow.py b/api/models/workflow.py index 7156acdb779ceb..58d6f2e067c325 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -1497,8 +1497,6 @@ class WorkflowSchedulePlan(Base): - tenant_id (uuid) Workspace ID for multi-tenancy - cron_expression (varchar) Cron expression defining schedule pattern - timezone (varchar) Timezone for cron evaluation (e.g., 'Asia/Shanghai') - - enabled (bool) Whether schedule is active - - triggered_by (varchar) Environment: debugger or production - next_run_at (timestamp) Next scheduled execution time - created_at (timestamp) Creation timestamp - updated_at (timestamp) Last update timestamp @@ -1507,11 +1505,11 @@ class WorkflowSchedulePlan(Base): __tablename__ = "workflow_schedule_plans" __table_args__ = ( sa.PrimaryKeyConstraint("id", name="workflow_schedule_plan_pkey"), - sa.UniqueConstraint("app_id", "node_id", "triggered_by", name="uniq_app_node_trigger"), - sa.Index("workflow_schedule_plan_enabled_next_idx", "enabled", "next_run_at"), + sa.UniqueConstraint("app_id", "node_id", name="uniq_app_node"), + sa.Index("workflow_schedule_plan_next_idx", "next_run_at"), ) - id: Mapped[str] = mapped_column(StringUUID, server_default=sa.text("uuid_generate_v4()")) + id: Mapped[str] = mapped_column(StringUUID, server_default=sa.text("uuidv7()")) app_id: Mapped[str] = mapped_column(StringUUID, nullable=False) node_id: Mapped[str] = mapped_column(String(64), nullable=False) tenant_id: Mapped[str] = mapped_column(StringUUID, nullable=False) @@ -1521,8 +1519,6 @@ class WorkflowSchedulePlan(Base): timezone: Mapped[str] = mapped_column(String(64), nullable=False) # Schedule control - enabled: Mapped[bool] = mapped_column(sa.Boolean, nullable=False, default=True) - triggered_by: Mapped[str] = mapped_column(String(16), nullable=False) next_run_at: Mapped[Optional[datetime]] = mapped_column(DateTime, nullable=True) created_at: Mapped[datetime] = mapped_column(DateTime, nullable=False, server_default=func.current_timestamp()) updated_at: Mapped[datetime] = mapped_column( @@ -1538,9 +1534,7 @@ def to_dict(self) -> dict: "tenant_id": self.tenant_id, "cron_expression": self.cron_expression, "timezone": self.timezone, - "enabled": self.enabled, "next_run_at": self.next_run_at.isoformat() if self.next_run_at else None, - "triggered_by": self.triggered_by, "created_at": self.created_at.isoformat(), "updated_at": self.updated_at.isoformat(), } diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py index 5e3d072b1df73d..844b76e0d4fe91 100644 --- a/api/schedule/workflow_schedule_task.py +++ b/api/schedule/workflow_schedule_task.py @@ -1,83 +1,199 @@ import logging +from collections import defaultdict from datetime import UTC, datetime from celery import shared_task -from sqlalchemy import func, select -from sqlalchemy.orm import sessionmaker +from sqlalchemy import and_, select +from sqlalchemy.orm import Session, sessionmaker from configs import dify_config from extensions.ext_database import db -from models.workflow import WorkflowSchedulePlan -from services.workflow.schedule_manager import ScheduleService +from models.workflow import AppTrigger, AppTriggerStatus, WorkflowSchedulePlan +from services.schedule_service import ScheduleService +from services.workflow.queue_dispatcher import QueueDispatcherManager from tasks.workflow_schedule_tasks import run_schedule_trigger logger = logging.getLogger(__name__) @shared_task(queue="schedule") -def poll_workflow_schedules(): - """ - Poll for due workflow schedules and dispatch them for execution. - - This task runs every minute via beat_schedule to check for schedules - that need to be triggered. - """ +def poll_workflow_schedules() -> None: + """Poll and process due workflow schedules.""" session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) - + with session_factory() as session: - now = datetime.now(UTC) - - # Find all due schedules with FOR UPDATE SKIP LOCKED for HA support - due_schedules = session.scalars( + due_schedules = _fetch_due_schedules(session) + + if due_schedules: + logger.info("Processing %d due schedules", len(due_schedules)) + processed = _process_schedules(session, due_schedules) + if processed: + logger.info("Successfully dispatched %d schedules", processed) + + +def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: + """Fetch all schedules that are due for execution.""" + now = datetime.now(UTC) + + return list( + session.scalars( select(WorkflowSchedulePlan) + .join( + AppTrigger, + and_( + AppTrigger.app_id == WorkflowSchedulePlan.app_id, + AppTrigger.node_id == WorkflowSchedulePlan.node_id, + AppTrigger.trigger_type == "trigger-schedule", + ), + ) .where( - WorkflowSchedulePlan.enabled == True, WorkflowSchedulePlan.next_run_at <= now, WorkflowSchedulePlan.next_run_at.isnot(None), + AppTrigger.status == AppTriggerStatus.ENABLED, ) .with_for_update(skip_locked=True) .limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE) ).all() - - if due_schedules: - logger.info("Processing %d due schedules", len(due_schedules)) - - schedules_to_update = [] - schedules_to_disable = [] - - for schedule in due_schedules: - try: - result = run_schedule_trigger.delay(schedule.id) - logger.info("Dispatched schedule %s, task ID: %s", schedule.id, result.id) - - next_run_at = ScheduleService.calculate_next_run_at( - schedule.cron_expression, - schedule.timezone, - base_time=now, - ) - - if next_run_at is None: - schedule.enabled = False - schedule.next_run_at = None - schedules_to_disable.append(schedule.id) - logger.info("Schedule %s has no more runs, disabling", schedule.id) - else: - # Update next run time - schedule.next_run_at = next_run_at - schedules_to_update.append(schedule.id) - logger.debug("Schedule %s next run at %s", schedule.id, next_run_at) - - except Exception as e: - logger.error("Error processing schedule %s: %s", schedule.id, e, exc_info=True) - - if schedules_to_update or schedules_to_disable: - try: - session.commit() - logger.info( - "Updated %d schedules, disabled %d", - len(schedules_to_update), - len(schedules_to_disable), - ) - except Exception as e: - logger.error("Error committing schedule updates: %s", e, exc_info=True) - session.rollback() \ No newline at end of file + ) + + +def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> int: + """Process schedules with rate limiting and two-phase commit.""" + if not schedules: + return 0 + + start_time = datetime.now(UTC) + + schedules_by_tenant = _group_by_tenant(schedules) + + dispatch_list, removal_list, rate_limited = _prepare_schedule_updates(schedules_by_tenant) + + if not _commit_schedule_updates(session, dispatch_list, removal_list): + return 0 + + dispatched_count = _dispatch_schedules(dispatch_list, rate_limited) + + _log_batch_summary( + total_count=len(schedules), + dispatched_count=dispatched_count, + removal_count=len(removal_list), + rate_limited_tenants=rate_limited, + elapsed_time=(datetime.now(UTC) - start_time).total_seconds(), + ) + + return dispatched_count + + +def _group_by_tenant(schedules: list[WorkflowSchedulePlan]) -> dict[str, list[WorkflowSchedulePlan]]: + """Group schedules by tenant ID for efficient processing.""" + schedules_by_tenant = defaultdict(list) + for schedule in schedules: + schedules_by_tenant[schedule.tenant_id].append(schedule) + return schedules_by_tenant + + +def _prepare_schedule_updates( + schedules_by_tenant: dict[str, list[WorkflowSchedulePlan]], +) -> tuple[list[WorkflowSchedulePlan], list[WorkflowSchedulePlan], set[str]]: + """ + Process schedules and check rate limits. + + Returns: + Tuple of (schedules_to_dispatch, schedules_to_remove, rate_limited_tenants) + """ + schedules_to_dispatch = [] + schedules_to_remove = [] + rate_limited_tenants = set() + + dispatcher_manager = QueueDispatcherManager() + + for tenant_id, tenant_schedules in schedules_by_tenant.items(): + if _is_tenant_rate_limited(tenant_id, dispatcher_manager): + rate_limited_tenants.add(tenant_id) + logger.warning("Tenant %s reached daily limit, skipping %d schedules", tenant_id, len(tenant_schedules)) + continue + + for schedule in tenant_schedules: + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + ) + + if next_run_at: + schedule.next_run_at = next_run_at + schedules_to_dispatch.append(schedule) + else: + schedules_to_remove.append(schedule) + + return schedules_to_dispatch, schedules_to_remove, rate_limited_tenants + + +def _is_tenant_rate_limited(tenant_id: str, dispatcher_manager: QueueDispatcherManager) -> bool: + """Check if tenant has reached daily rate limit.""" + dispatcher = dispatcher_manager.get_dispatcher(tenant_id) + return not dispatcher.check_daily_quota(tenant_id) + + +def _commit_schedule_updates( + session: Session, schedules_to_dispatch: list[WorkflowSchedulePlan], schedules_to_remove: list[WorkflowSchedulePlan] +) -> bool: + """ + Commit schedule updates to database. + + Returns: + True if commit successful, False otherwise + """ + for schedule in schedules_to_remove: + session.delete(schedule) + + try: + session.commit() + return True + except Exception as e: + total_count = len(schedules_to_dispatch) + len(schedules_to_remove) + logger.error("Failed to commit %d schedule updates: %s", total_count, e, exc_info=True) + session.rollback() + return False + + +def _dispatch_schedules(schedules_to_dispatch: list[WorkflowSchedulePlan], rate_limited_tenants: set[str]) -> int: + """ + Dispatch schedules to Celery for execution. + + Returns: + Number of successfully dispatched schedules + """ + dispatched_count = 0 + + for schedule in schedules_to_dispatch: + # Skip if tenant was rate limited + if schedule.tenant_id in rate_limited_tenants: + continue + + try: + run_schedule_trigger.delay(schedule.id) + dispatched_count += 1 + except Exception as e: + logger.error("Failed to dispatch schedule %s: %s", schedule.id, e, exc_info=True) + + return dispatched_count + + +def _log_batch_summary( + total_count: int, dispatched_count: int, removal_count: int, rate_limited_tenants: set[str], elapsed_time: float +) -> None: + """Log a summary of the batch processing.""" + summary_parts = [f"{dispatched_count} dispatched"] + + if rate_limited_tenants: + summary_parts.append(f"{len(rate_limited_tenants)} tenants rate-limited") + + if removal_count > 0: + summary_parts.append(f"{removal_count} expired") + + logger.info( + "Batch completed: %d processed (%s) in %.3fs", + total_count, + ", ".join(summary_parts), + elapsed_time, + ) diff --git a/api/services/workflow/schedule_manager.py b/api/services/schedule_service.py similarity index 81% rename from api/services/workflow/schedule_manager.py rename to api/services/schedule_service.py index 045e390c98c2ff..986cad34e1be8d 100644 --- a/api/services/workflow/schedule_manager.py +++ b/api/services/schedule_service.py @@ -41,8 +41,6 @@ def create_schedule( node_id: str, cron_expression: str, timezone: str, - triggered_by: str, - enabled: bool = True, ) -> WorkflowSchedulePlan: """ Create a new workflow schedule. @@ -54,8 +52,6 @@ def create_schedule( node_id: Starting node ID cron_expression: Cron expression timezone: Timezone for cron evaluation - triggered_by: Trigger context - 'debugger' or 'production' - enabled: Whether schedule is enabled Returns: Created WorkflowSchedulePlan instance @@ -73,9 +69,7 @@ def create_schedule( node_id=node_id, cron_expression=cron_expression, timezone=timezone, - enabled=enabled and next_run_at is not None, next_run_at=next_run_at, - triggered_by=triggered_by, ) session.add(schedule) @@ -110,17 +104,12 @@ def update_schedule( setattr(schedule, field, value) # Recalculate next_run_at if schedule parameters changed - if any(field in updates for field in ["cron_expression", "timezone", "enabled"]): - if schedule.enabled: - next_run_at = ScheduleService.calculate_next_run_at( - schedule.cron_expression, - schedule.timezone, - ) - schedule.next_run_at = next_run_at - - # Disable if no valid next run - if next_run_at is None: - schedule.enabled = False + if any(field in updates for field in ["cron_expression", "timezone"]): + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + ) + schedule.next_run_at = next_run_at session.flush() return schedule @@ -148,20 +137,6 @@ def delete_schedule( session.flush() return True - @staticmethod - def get_schedule(session: Session, schedule_id: str) -> Optional[WorkflowSchedulePlan]: - """ - Get a schedule by ID. - - Args: - session: Database session - schedule_id: Schedule ID - - Returns: - WorkflowSchedulePlan or None - """ - return session.get(WorkflowSchedulePlan, schedule_id) - @staticmethod def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: """ @@ -212,7 +187,7 @@ def update_next_run_at( New next_run_at datetime or None """ schedule = session.get(WorkflowSchedulePlan, schedule_id) - if not schedule or not schedule.enabled: + if not schedule: return None # Calculate next run time @@ -224,10 +199,5 @@ def update_next_run_at( # Update schedule schedule.next_run_at = next_run_at - - # If no next run, disable it - if next_run_at is None: - schedule.enabled = False - session.flush() return next_run_at diff --git a/api/services/workflow/schedule_sync.py b/api/services/workflow/schedule_sync.py deleted file mode 100644 index c42669d0c6c7d2..00000000000000 --- a/api/services/workflow/schedule_sync.py +++ /dev/null @@ -1,250 +0,0 @@ -import json -import logging -from typing import Optional - -from sqlalchemy import select -from sqlalchemy.orm import Session - -from models.workflow import WorkflowSchedulePlan -from services.workflow.schedule_manager import ScheduleService - -logger = logging.getLogger(__name__) - - -class ScheduleSyncService: - @staticmethod - def sync_schedule_from_graph( - session: Session, - tenant_id: str, - app_id: str, - graph: str, - ) -> Optional[WorkflowSchedulePlan]: - try: - graph_data = json.loads(graph) - except (json.JSONDecodeError, TypeError): - return None - - # Find schedule trigger node in graph - schedule_node = None - node_id = None - schedule_nodes_count = 0 - - for node in graph_data.get("nodes", []): - node_data = node.get("data", {}) - if node_data.get("type") == "trigger-schedule": - schedule_nodes_count += 1 - if not schedule_node: # Take the first schedule node - schedule_node = node_data - node_id = node.get("id", "start") - - if schedule_nodes_count > 1: - logger.warning("Found %d schedule nodes in workflow, only the first one will be used", schedule_nodes_count) - - # Get existing schedule plan for this app - existing_plan = session.scalar( - select(WorkflowSchedulePlan).where( - WorkflowSchedulePlan.tenant_id == tenant_id, - WorkflowSchedulePlan.app_id == app_id, - ) - ) - - # If no schedule node exists, disable the schedule but keep the plan - if not schedule_node: - if existing_plan: - logger.info("No schedule node in workflow for app %s, disabling schedule plan", app_id) - updates = { - "enabled": False, # Disable but keep the plan - "next_run_at": None, # Clear next run time - } - return ScheduleService.update_schedule( - session=session, - schedule_id=existing_plan.id, - updates=updates, - ) - # No existing plan and no schedule node, nothing to do - return None - - # Extract schedule configuration - mode = schedule_node.get("mode", "visual") - timezone = schedule_node.get("timezone", "UTC") - enabled = schedule_node.get("enabled", True) - - # Convert to cron expression - cron_expression = None - if mode == "cron": - cron_expression = schedule_node.get("cron_expression") - elif mode == "visual": - cron_expression = ScheduleSyncService._visual_to_cron( - schedule_node.get("frequency"), schedule_node.get("visual_config", {}) - ) - - if not cron_expression: - # Invalid configuration, remove existing plan - logger.warning("Invalid schedule configuration for app %s, removing schedule plan", app_id) - if existing_plan: - session.delete(existing_plan) - session.flush() - return None - - # Update existing plan or create new one - if existing_plan: - # Update existing schedule with new workflow version - logger.info("Updating schedule plan for app %s with new workflow %s", app_id) - updates = { - "node_id": node_id, - "cron_expression": cron_expression, - "timezone": timezone, - "enabled": enabled, - } - updated_plan = ScheduleService.update_schedule( - session=session, - schedule_id=existing_plan.id, - updates=updates, - ) - return updated_plan - else: - # Create new schedule - logger.info("Creating new schedule plan for app %s, workflow %s", app_id) - new_plan = ScheduleService.create_schedule( - session=session, - tenant_id=tenant_id, - app_id=app_id, - node_id=node_id, - cron_expression=cron_expression, - timezone=timezone, - enabled=enabled, - triggered_by="production", - ) - return new_plan - - @staticmethod - def _visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: - """ - Convert visual schedule configuration to cron expression. - - Args: - frequency: Schedule frequency (hourly, daily, weekly, monthly) - visual_config: Visual configuration with time, weekdays, etc. - - Returns: - Cron expression string or None if invalid - """ - if not frequency or not visual_config: - return None - - try: - if frequency == "hourly": - # Run at specific minute of every hour - on_minute = visual_config.get("on_minute", 0) - return f"{on_minute} * * * *" - - elif frequency == "daily": - # Run daily at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleSyncService._parse_time(time_str) - if hour is None or minute is None: - return None - return f"{minute} {hour} * * *" - - elif frequency == "weekly": - # Run weekly on specific days at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleSyncService._parse_time(time_str) - if hour is None or minute is None: - return None - - weekdays = visual_config.get("weekdays", []) - if not weekdays: - return None - - # Convert weekday names to cron format (0-6, where 0=Sunday) - weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} - cron_weekdays = [] - for day in weekdays: - if day in weekday_map: - cron_weekdays.append(weekday_map[day]) - - if not cron_weekdays: - return None - - return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" - - elif frequency == "monthly": - # Run monthly on specific days at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleSyncService._parse_time(time_str) - if hour is None or minute is None: - return None - - monthly_days = visual_config.get("monthly_days", []) - if not monthly_days: - return None - - # Convert monthly days to cron format - cron_days = [] - for day in monthly_days: - if day == "last": - # Last day of month (L is supported by some cron implementations) - # For standard cron, we use 28-31 as approximation - cron_days.extend(["28", "29", "30", "31"]) - elif isinstance(day, int) and 1 <= day <= 31: - cron_days.append(str(day)) - - if not cron_days: - return None - - # Remove duplicates and sort - cron_days = sorted(set(cron_days), key=int) - return f"{minute} {hour} {','.join(cron_days)} * *" - - else: - return None - - except Exception: - return None - - @staticmethod - def _parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: - """ - Parse time string in format "HH:MM AM/PM" to 24-hour format. - - Args: - time_str: Time string like "11:30 AM" or "2:45 PM" - - Returns: - Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid - """ - try: - # Split time and period - parts = time_str.strip().split() - if len(parts) != 2: - return None, None - - time_part, period = parts - period = period.upper() - - if period not in ["AM", "PM"]: - return None, None - - # Parse hour and minute - time_parts = time_part.split(":") - if len(time_parts) != 2: - return None, None - - hour = int(time_parts[0]) - minute = int(time_parts[1]) - - # Validate ranges - if hour < 1 or hour > 12 or minute < 0 or minute > 59: - return None, None - - # Convert to 24-hour format - if period == "PM" and hour != 12: - hour += 12 - elif period == "AM" and hour == 12: - hour = 0 - - return hour, minute - - except (ValueError, AttributeError): - return None, None diff --git a/api/services/workflow_service.py b/api/services/workflow_service.py index 7c036fcb28b7ed..d2715a61fe1a43 100644 --- a/api/services/workflow_service.py +++ b/api/services/workflow_service.py @@ -44,7 +44,6 @@ ) from repositories.factory import DifyAPIRepositoryFactory from services.errors.app import IsDraftWorkflowError, WorkflowHashNotEqualError -from services.workflow.schedule_sync import ScheduleSyncService from services.workflow.workflow_converter import WorkflowConverter from .errors.workflow_service import DraftWorkflowDeletionError, WorkflowInUseError @@ -286,15 +285,6 @@ def publish_workflow( # commit db session changes session.add(workflow) - session.flush() - - # Sync schedule configuration from graph - ScheduleSyncService.sync_schedule_from_graph( - session=session, - tenant_id=app_model.tenant_id, - app_id=app_model.id, - graph=workflow.graph, - ) # trigger app workflow events app_published_workflow_was_updated.send(app_model, published_workflow=workflow) diff --git a/api/tasks/workflow_schedule_tasks.py b/api/tasks/workflow_schedule_tasks.py index c973526ef0885a..412841b0f093d9 100644 --- a/api/tasks/workflow_schedule_tasks.py +++ b/api/tasks/workflow_schedule_tasks.py @@ -9,8 +9,8 @@ from models.enums import WorkflowRunTriggeredFrom from models.workflow import WorkflowSchedulePlan from services.async_workflow_service import AsyncWorkflowService +from services.schedule_service import ScheduleService from services.workflow.entities import AsyncTriggerResponse, TriggerData -from services.workflow.schedule_manager import ScheduleService logger = logging.getLogger(__name__) @@ -31,10 +31,6 @@ def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: logger.warning("Schedule %s not found", schedule_id) return - if not schedule.enabled: - logger.debug("Schedule %s is disabled", schedule_id) - return - tenant_owner = ScheduleService.get_tenant_owner(session, schedule.tenant_id) if not tenant_owner: logger.error("Tenant owner not found for tenant %s", schedule.tenant_id) @@ -52,7 +48,6 @@ def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: trigger_data=TriggerData( app_id=schedule.app_id, root_node_id=schedule.node_id, - # TODO: need `workflow_id` when triggered_by = `debugger` trigger_type=WorkflowRunTriggeredFrom.SCHEDULE, inputs=inputs, tenant_id=schedule.tenant_id, @@ -62,4 +57,11 @@ def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: return response except Exception as e: - logger.error("Failed to trigger workflow for schedule %s: %s", schedule_id, e, exc_info=True) + logger.error( + "Failed to trigger workflow for schedule %s. App: %s, Error: %s", + schedule_id, + schedule.app_id, + str(e), + exc_info=True, + ) + return None From 57502a424b92bc6ca68b576e8893c25f6a2fa7b2 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 01:25:16 +0800 Subject: [PATCH 05/26] feat: sync workflow schedule params when publishing workflow --- api/events/event_handlers/__init__.py | 1 + ...nc_workflow_schedule_when_app_published.py | 281 ++++++++++++++++++ api/fields/workflow_run_fields.py | 1 + api/models/__init__.py | 2 + 4 files changed, 285 insertions(+) create mode 100644 api/events/event_handlers/sync_workflow_schedule_when_app_published.py diff --git a/api/events/event_handlers/__init__.py b/api/events/event_handlers/__init__.py index 4f0a163d893879..76b08ccc79157d 100644 --- a/api/events/event_handlers/__init__.py +++ b/api/events/event_handlers/__init__.py @@ -4,6 +4,7 @@ from .create_installed_app_when_app_created import handle from .create_site_record_when_app_created import handle from .delete_tool_parameters_cache_when_sync_draft_workflow import handle +from .sync_workflow_schedule_when_app_published import handle from .update_app_dataset_join_when_app_model_config_updated import handle from .update_app_dataset_join_when_app_published_workflow_updated import handle from .update_app_triggers_when_app_published_workflow_updated import handle diff --git a/api/events/event_handlers/sync_workflow_schedule_when_app_published.py b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py new file mode 100644 index 00000000000000..75343b598e098b --- /dev/null +++ b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py @@ -0,0 +1,281 @@ +import json +import logging +from typing import Optional, cast + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from core.workflow.nodes import NodeType +from events.app_event import app_published_workflow_was_updated +from extensions.ext_database import db +from models import AppMode, Workflow, WorkflowSchedulePlan +from services.schedule_service import ScheduleService + +logger = logging.getLogger(__name__) + + +@app_published_workflow_was_updated.connect +def handle(sender, **kwargs): + """ + Handle app published workflow update event to sync workflow_schedule_plans table. + + When a workflow is published, this handler will: + 1. Extract schedule trigger nodes from the workflow graph + 2. Compare with existing workflow_schedule_plans records + 3. Create/update/delete schedule plans as needed + """ + app = sender + if app.mode != AppMode.WORKFLOW.value: + return + + published_workflow = kwargs.get("published_workflow") + published_workflow = cast(Workflow, published_workflow) + + # Sync schedule configuration + sync_schedule_from_workflow(tenant_id=app.tenant_id, app_id=app.id, workflow=published_workflow) + + +def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) -> Optional[WorkflowSchedulePlan]: + """ + Sync schedule plan from workflow graph configuration. + + Args: + tenant_id: Tenant ID + app_id: App ID + workflow: Published workflow instance + + Returns: + Updated or created WorkflowSchedulePlan, or None if no schedule node + """ + with Session(db.engine) as session: + # Extract schedule configuration from workflow + schedule_config = extract_schedule_config(workflow) + + # Get existing schedule plan for this app + existing_plan = session.scalar( + select(WorkflowSchedulePlan).where( + WorkflowSchedulePlan.tenant_id == tenant_id, + WorkflowSchedulePlan.app_id == app_id, + ) + ) + + # If no schedule node exists, remove any existing plan + if not schedule_config: + if existing_plan: + logger.info("No schedule node in workflow for app %s, removing schedule plan", app_id) + session.delete(existing_plan) + session.commit() + return None + + # Extract schedule parameters + node_id = schedule_config["node_id"] + cron_expression = schedule_config["cron_expression"] + timezone = schedule_config["timezone"] + + # Update existing plan or create new one + if existing_plan: + # Update existing schedule with new configuration + logger.info("Updating schedule plan for app %s", app_id) + updates = { + "node_id": node_id, + "cron_expression": cron_expression, + "timezone": timezone, + } + updated_plan = ScheduleService.update_schedule( + session=session, + schedule_id=existing_plan.id, + updates=updates, + ) + session.commit() + return updated_plan + else: + # Create new schedule + logger.info("Creating new schedule plan for app %s", app_id) + new_plan = ScheduleService.create_schedule( + session=session, + tenant_id=tenant_id, + app_id=app_id, + node_id=node_id, + cron_expression=cron_expression, + timezone=timezone, + ) + session.commit() + return new_plan + + +def extract_schedule_config(workflow: Workflow) -> Optional[dict]: + """ + Extract schedule configuration from workflow graph. + + Args: + workflow: Workflow instance + + Returns: + Schedule configuration dict or None if no schedule node found + """ + try: + graph_data = workflow.graph_dict + except (json.JSONDecodeError, TypeError, AttributeError): + return None + + if not graph_data: + return None + + # Find schedule trigger node in graph + for node in graph_data.get("nodes", []): + node_data = node.get("data", {}) + if node_data.get("type") == NodeType.TRIGGER_SCHEDULE.value: + # Extract configuration + mode = node_data.get("mode", "visual") + timezone = node_data.get("timezone", "UTC") + node_id = node.get("id", "start") + + # Convert to cron expression + cron_expression = None + if mode == "cron": + cron_expression = node_data.get("cron_expression") + elif mode == "visual": + cron_expression = _visual_to_cron(node_data.get("frequency"), node_data.get("visual_config", {})) + + if cron_expression: + return { + "node_id": node_id, + "cron_expression": cron_expression, + "timezone": timezone, + } + else: + logger.warning("Invalid schedule configuration in node %s", node_id) + return None + + return None + + +def _visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: + """ + Convert visual schedule configuration to cron expression. + + Args: + frequency: Schedule frequency (hourly, daily, weekly, monthly) + visual_config: Visual configuration with time, weekdays, etc. + + Returns: + Cron expression string or None if invalid + """ + if not frequency or not visual_config: + return None + + try: + if frequency == "hourly": + # Run at specific minute of every hour + on_minute = visual_config.get("on_minute", 0) + return f"{on_minute} * * * *" + + elif frequency == "daily": + # Run daily at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = _parse_time(time_str) + if hour is None or minute is None: + return None + return f"{minute} {hour} * * *" + + elif frequency == "weekly": + # Run weekly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = _parse_time(time_str) + if hour is None or minute is None: + return None + + weekdays = visual_config.get("weekdays", []) + if not weekdays: + return None + + # Convert weekday names to cron format (0-6, where 0=Sunday) + weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} + cron_weekdays = [] + for day in weekdays: + if day in weekday_map: + cron_weekdays.append(weekday_map[day]) + + if not cron_weekdays: + return None + + return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" + + elif frequency == "monthly": + # Run monthly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = _parse_time(time_str) + if hour is None or minute is None: + return None + + monthly_days = visual_config.get("monthly_days", []) + if not monthly_days: + return None + + # Convert monthly days to cron format + cron_days = [] + for day in monthly_days: + if day == "last": + # Last day of month - use 28-31 as approximation + cron_days.extend(["28", "29", "30", "31"]) + elif isinstance(day, int) and 1 <= day <= 31: + cron_days.append(str(day)) + + if not cron_days: + return None + + # Remove duplicates and sort + cron_days = sorted(set(cron_days), key=int) + return f"{minute} {hour} {','.join(cron_days)} * *" + + else: + return None + + except Exception: + return None + + +def _parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: + """ + Parse time string in format "HH:MM AM/PM" to 24-hour format. + + Args: + time_str: Time string like "11:30 AM" or "2:45 PM" + + Returns: + Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid + """ + try: + # Split time and period + parts = time_str.strip().split() + if len(parts) != 2: + return None, None + + time_part, period = parts + period = period.upper() + + if period not in ["AM", "PM"]: + return None, None + + # Parse hour and minute + time_parts = time_part.split(":") + if len(time_parts) != 2: + return None, None + + hour = int(time_parts[0]) + minute = int(time_parts[1]) + + # Validate ranges + if hour < 1 or hour > 12 or minute < 0 or minute > 59: + return None, None + + # Convert to 24-hour format + if period == "PM" and hour != 12: + hour += 12 + elif period == "AM" and hour == 12: + hour = 0 + + return hour, minute + + except (ValueError, AttributeError): + return None, None diff --git a/api/fields/workflow_run_fields.py b/api/fields/workflow_run_fields.py index 6462d8ce5a4043..2d6b092ae1878f 100644 --- a/api/fields/workflow_run_fields.py +++ b/api/fields/workflow_run_fields.py @@ -15,6 +15,7 @@ "created_at": TimestampField, "finished_at": TimestampField, "exceptions_count": fields.Integer, + "triggered_from": fields.String, } diff --git a/api/models/__init__.py b/api/models/__init__.py index 03cbf9bac6ec79..71468ff74807f4 100644 --- a/api/models/__init__.py +++ b/api/models/__init__.py @@ -91,6 +91,7 @@ WorkflowNodeExecutionModel, WorkflowNodeExecutionTriggeredFrom, WorkflowRun, + WorkflowSchedulePlan, WorkflowType, ) @@ -181,6 +182,7 @@ "WorkflowNodeExecutionTriggeredFrom", "WorkflowRun", "WorkflowRunTriggeredFrom", + "WorkflowSchedulePlan", "WorkflowToolProvider", "WorkflowType", "db", From aaa2c74f05cbaec9f159fdf4862edb52d16c63ac Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 16:51:46 +0800 Subject: [PATCH 06/26] chore: internal func move to ScheduleService --- ...nc_workflow_schedule_when_app_published.py | 190 +---------------- api/services/schedule_service.py | 192 +++++++++++++++++- 2 files changed, 192 insertions(+), 190 deletions(-) diff --git a/api/events/event_handlers/sync_workflow_schedule_when_app_published.py b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py index 75343b598e098b..91d6aa1d205235 100644 --- a/api/events/event_handlers/sync_workflow_schedule_when_app_published.py +++ b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py @@ -1,11 +1,9 @@ -import json import logging from typing import Optional, cast from sqlalchemy import select from sqlalchemy.orm import Session -from core.workflow.nodes import NodeType from events.app_event import app_published_workflow_was_updated from extensions.ext_database import db from models import AppMode, Workflow, WorkflowSchedulePlan @@ -31,7 +29,6 @@ def handle(sender, **kwargs): published_workflow = kwargs.get("published_workflow") published_workflow = cast(Workflow, published_workflow) - # Sync schedule configuration sync_schedule_from_workflow(tenant_id=app.tenant_id, app_id=app.id, workflow=published_workflow) @@ -48,10 +45,8 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) Updated or created WorkflowSchedulePlan, or None if no schedule node """ with Session(db.engine) as session: - # Extract schedule configuration from workflow - schedule_config = extract_schedule_config(workflow) + schedule_config = ScheduleService.extract_schedule_config(workflow) - # Get existing schedule plan for this app existing_plan = session.scalar( select(WorkflowSchedulePlan).where( WorkflowSchedulePlan.tenant_id == tenant_id, @@ -59,7 +54,6 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) ) ) - # If no schedule node exists, remove any existing plan if not schedule_config: if existing_plan: logger.info("No schedule node in workflow for app %s, removing schedule plan", app_id) @@ -67,14 +61,11 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) session.commit() return None - # Extract schedule parameters node_id = schedule_config["node_id"] cron_expression = schedule_config["cron_expression"] timezone = schedule_config["timezone"] - # Update existing plan or create new one if existing_plan: - # Update existing schedule with new configuration logger.info("Updating schedule plan for app %s", app_id) updates = { "node_id": node_id, @@ -89,7 +80,6 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) session.commit() return updated_plan else: - # Create new schedule logger.info("Creating new schedule plan for app %s", app_id) new_plan = ScheduleService.create_schedule( session=session, @@ -101,181 +91,3 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) ) session.commit() return new_plan - - -def extract_schedule_config(workflow: Workflow) -> Optional[dict]: - """ - Extract schedule configuration from workflow graph. - - Args: - workflow: Workflow instance - - Returns: - Schedule configuration dict or None if no schedule node found - """ - try: - graph_data = workflow.graph_dict - except (json.JSONDecodeError, TypeError, AttributeError): - return None - - if not graph_data: - return None - - # Find schedule trigger node in graph - for node in graph_data.get("nodes", []): - node_data = node.get("data", {}) - if node_data.get("type") == NodeType.TRIGGER_SCHEDULE.value: - # Extract configuration - mode = node_data.get("mode", "visual") - timezone = node_data.get("timezone", "UTC") - node_id = node.get("id", "start") - - # Convert to cron expression - cron_expression = None - if mode == "cron": - cron_expression = node_data.get("cron_expression") - elif mode == "visual": - cron_expression = _visual_to_cron(node_data.get("frequency"), node_data.get("visual_config", {})) - - if cron_expression: - return { - "node_id": node_id, - "cron_expression": cron_expression, - "timezone": timezone, - } - else: - logger.warning("Invalid schedule configuration in node %s", node_id) - return None - - return None - - -def _visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: - """ - Convert visual schedule configuration to cron expression. - - Args: - frequency: Schedule frequency (hourly, daily, weekly, monthly) - visual_config: Visual configuration with time, weekdays, etc. - - Returns: - Cron expression string or None if invalid - """ - if not frequency or not visual_config: - return None - - try: - if frequency == "hourly": - # Run at specific minute of every hour - on_minute = visual_config.get("on_minute", 0) - return f"{on_minute} * * * *" - - elif frequency == "daily": - # Run daily at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = _parse_time(time_str) - if hour is None or minute is None: - return None - return f"{minute} {hour} * * *" - - elif frequency == "weekly": - # Run weekly on specific days at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = _parse_time(time_str) - if hour is None or minute is None: - return None - - weekdays = visual_config.get("weekdays", []) - if not weekdays: - return None - - # Convert weekday names to cron format (0-6, where 0=Sunday) - weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} - cron_weekdays = [] - for day in weekdays: - if day in weekday_map: - cron_weekdays.append(weekday_map[day]) - - if not cron_weekdays: - return None - - return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" - - elif frequency == "monthly": - # Run monthly on specific days at specific time - time_str = visual_config.get("time", "12:00 PM") - hour, minute = _parse_time(time_str) - if hour is None or minute is None: - return None - - monthly_days = visual_config.get("monthly_days", []) - if not monthly_days: - return None - - # Convert monthly days to cron format - cron_days = [] - for day in monthly_days: - if day == "last": - # Last day of month - use 28-31 as approximation - cron_days.extend(["28", "29", "30", "31"]) - elif isinstance(day, int) and 1 <= day <= 31: - cron_days.append(str(day)) - - if not cron_days: - return None - - # Remove duplicates and sort - cron_days = sorted(set(cron_days), key=int) - return f"{minute} {hour} {','.join(cron_days)} * *" - - else: - return None - - except Exception: - return None - - -def _parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: - """ - Parse time string in format "HH:MM AM/PM" to 24-hour format. - - Args: - time_str: Time string like "11:30 AM" or "2:45 PM" - - Returns: - Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid - """ - try: - # Split time and period - parts = time_str.strip().split() - if len(parts) != 2: - return None, None - - time_part, period = parts - period = period.upper() - - if period not in ["AM", "PM"]: - return None, None - - # Parse hour and minute - time_parts = time_part.split(":") - if len(time_parts) != 2: - return None, None - - hour = int(time_parts[0]) - minute = int(time_parts[1]) - - # Validate ranges - if hour < 1 or hour > 12 or minute < 0 or minute > 59: - return None, None - - # Convert to 24-hour format - if period == "PM" and hour != 12: - hour += 12 - elif period == "AM" and hour == 12: - hour = 0 - - return hour, minute - - except (ValueError, AttributeError): - return None, None diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 986cad34e1be8d..3497207e6c4d07 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -1,3 +1,4 @@ +import json import logging from datetime import UTC, datetime from typing import Optional @@ -7,8 +8,9 @@ from sqlalchemy import select from sqlalchemy.orm import Session +from core.workflow.nodes import NodeType from models.account import Account, TenantAccountJoin -from models.workflow import WorkflowSchedulePlan +from models.workflow import Workflow, WorkflowSchedulePlan logger = logging.getLogger(__name__) @@ -201,3 +203,191 @@ def update_next_run_at( schedule.next_run_at = next_run_at session.flush() return next_run_at + + @staticmethod + def extract_schedule_config(workflow: Workflow) -> Optional[dict]: + """ + Extract schedule configuration from workflow graph. + + Args: + workflow: Workflow instance + + Returns: + Schedule configuration dict or None if no schedule node found + """ + try: + graph_data = workflow.graph_dict + except (json.JSONDecodeError, TypeError, AttributeError): + return None + + if not graph_data: + return None + + # Find schedule trigger node in graph + for node in graph_data.get("nodes", []): + node_data = node.get("data", {}) + if node_data.get("type") == NodeType.TRIGGER_SCHEDULE.value: + # Extract configuration + mode = node_data.get("mode", "visual") + timezone = node_data.get("timezone", "UTC") + node_id = node.get("id", "start") + + # Convert to cron expression + cron_expression = None + if mode == "cron": + cron_expression = node_data.get("cron_expression") + elif mode == "visual": + cron_expression = ScheduleService.visual_to_cron( + node_data.get("frequency"), node_data.get("visual_config", {}) + ) + + if cron_expression: + return { + "node_id": node_id, + "cron_expression": cron_expression, + "timezone": timezone, + } + else: + logger.warning("Invalid schedule configuration in node %s", node_id) + return None + + return None + + @staticmethod + def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: + """ + Convert visual schedule configuration to cron expression. + + Args: + frequency: Schedule frequency (hourly, daily, weekly, monthly) + visual_config: Visual configuration with time, weekdays, etc. + + Returns: + Cron expression string or None if invalid + """ + if not frequency or not visual_config: + return None + + try: + if frequency == "hourly": + # Run at specific minute of every hour + on_minute = visual_config.get("on_minute", 0) + return f"{on_minute} * * * *" + + elif frequency == "daily": + # Run daily at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleService.parse_time(time_str) + if hour is None or minute is None: + return None + return f"{minute} {hour} * * *" + + elif frequency == "weekly": + # Run weekly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleService.parse_time(time_str) + if hour is None or minute is None: + return None + + weekdays = visual_config.get("weekdays", []) + if not weekdays: + return None + + # Convert weekday names to cron format (0-6, where 0=Sunday) + weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} + cron_weekdays = [] + for day in weekdays: + if day in weekday_map: + cron_weekdays.append(weekday_map[day]) + + if not cron_weekdays: + return None + + return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" + + elif frequency == "monthly": + # Run monthly on specific days at specific time + time_str = visual_config.get("time", "12:00 PM") + hour, minute = ScheduleService.parse_time(time_str) + if hour is None or minute is None: + return None + + monthly_days = visual_config.get("monthly_days", []) + if not monthly_days: + return None + + # Convert monthly days to cron format + cron_days = [] + for day in monthly_days: + if day == "last": + # Use 'L' to represent the last day of month (supported by croniter) + cron_days.append("L") + elif isinstance(day, int) and 1 <= day <= 31: + cron_days.append(str(day)) + + if not cron_days: + return None + + # Sort numeric days, but keep 'L' at the end if present + numeric_days = [d for d in cron_days if d != "L"] + has_last = "L" in cron_days + + sorted_days = [] + if numeric_days: + sorted_days = sorted(set(numeric_days), key=int) + if has_last: + sorted_days.append("L") + + return f"{minute} {hour} {','.join(sorted_days)} * *" + + else: + return None + + except Exception: + return None + + @staticmethod + def parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: + """ + Parse time string in format "HH:MM AM/PM" to 24-hour format. + + Args: + time_str: Time string like "11:30 AM" or "2:45 PM" + + Returns: + Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid + """ + try: + # Split time and period + parts = time_str.strip().split() + if len(parts) != 2: + return None, None + + time_part, period = parts + period = period.upper() + + if period not in ["AM", "PM"]: + return None, None + + # Parse hour and minute + time_parts = time_part.split(":") + if len(time_parts) != 2: + return None, None + + hour = int(time_parts[0]) + minute = int(time_parts[1]) + + # Validate ranges + if hour < 1 or hour > 12 or minute < 0 or minute > 59: + return None, None + + # Convert to 24-hour format + if period == "PM" and hour != 12: + hour += 12 + elif period == "AM" and hour == 12: + hour = 0 + + return hour, minute + + except (ValueError, AttributeError): + return None, None From 3b9d3a33000f27eb5dd25e874174a0f7b5e44cbc Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 16:52:35 +0800 Subject: [PATCH 07/26] feat(test): add ScheduleService unit tests --- .../services/test_schedule_service.py | 629 ++++++++++++++++++ 1 file changed, 629 insertions(+) create mode 100644 api/tests/unit_tests/services/test_schedule_service.py diff --git a/api/tests/unit_tests/services/test_schedule_service.py b/api/tests/unit_tests/services/test_schedule_service.py new file mode 100644 index 00000000000000..5569b3700c6006 --- /dev/null +++ b/api/tests/unit_tests/services/test_schedule_service.py @@ -0,0 +1,629 @@ +import unittest +from datetime import UTC, datetime +from unittest.mock import MagicMock, Mock, patch + +from sqlalchemy.orm import Session + +from events.event_handlers.sync_workflow_schedule_when_app_published import ( + sync_schedule_from_workflow, +) +from models.account import Account, TenantAccountJoin +from models.workflow import Workflow, WorkflowSchedulePlan +from services.schedule_service import ScheduleService + + +class TestScheduleService(unittest.TestCase): + """Test cases for ScheduleService class.""" + + def test_calculate_next_run_at_valid_cron(self): + """Test calculating next run time with valid cron expression.""" + # Test daily cron at 10:30 AM + cron_expr = "30 10 * * *" + timezone = "UTC" + base_time = datetime(2025, 8, 29, 9, 0, 0, tzinfo=UTC) + + next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + + assert next_run is not None + assert next_run.hour == 10 + assert next_run.minute == 30 + assert next_run.day == 29 + + def test_calculate_next_run_at_with_timezone(self): + """Test calculating next run time with different timezone.""" + cron_expr = "0 9 * * *" # 9:00 AM + timezone = "America/New_York" + base_time = datetime(2025, 8, 29, 12, 0, 0, tzinfo=UTC) # 8:00 AM EDT + + next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + + assert next_run is not None + # 9:00 AM EDT = 13:00 UTC (during EDT) + expected_utc_hour = 13 + assert next_run.hour == expected_utc_hour + + def test_calculate_next_run_at_with_last_day_of_month(self): + """Test calculating next run time with 'L' (last day) syntax.""" + cron_expr = "0 10 L * *" # 10:00 AM on last day of month + timezone = "UTC" + base_time = datetime(2025, 2, 15, 9, 0, 0, tzinfo=UTC) + + next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + + assert next_run is not None + # February 2025 has 28 days + assert next_run.day == 28 + assert next_run.month == 2 + + def test_calculate_next_run_at_invalid_cron(self): + """Test calculating next run time with invalid cron expression.""" + cron_expr = "invalid cron" + timezone = "UTC" + + next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone) + + assert next_run is None + + def test_calculate_next_run_at_invalid_timezone(self): + """Test calculating next run time with invalid timezone.""" + cron_expr = "30 10 * * *" + timezone = "Invalid/Timezone" + + next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone) + + assert next_run is None + + @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + def test_create_schedule(self, mock_calculate_next_run): + """Test creating a new schedule.""" + mock_session = MagicMock(spec=Session) + mock_calculate_next_run.return_value = datetime(2025, 8, 30, 10, 30, 0, tzinfo=UTC) + + schedule = ScheduleService.create_schedule( + session=mock_session, + tenant_id="test-tenant", + app_id="test-app", + node_id="start", + cron_expression="30 10 * * *", + timezone="UTC", + ) + + assert schedule is not None + assert schedule.tenant_id == "test-tenant" + assert schedule.app_id == "test-app" + assert schedule.node_id == "start" + assert schedule.cron_expression == "30 10 * * *" + assert schedule.timezone == "UTC" + assert schedule.next_run_at is not None + mock_session.add.assert_called_once() + mock_session.flush.assert_called_once() + + @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + def test_update_schedule(self, mock_calculate_next_run): + """Test updating an existing schedule.""" + mock_session = MagicMock(spec=Session) + mock_schedule = Mock(spec=WorkflowSchedulePlan) + mock_schedule.cron_expression = "0 12 * * *" + mock_schedule.timezone = "UTC" + mock_session.get.return_value = mock_schedule + mock_calculate_next_run.return_value = datetime(2025, 8, 30, 12, 0, 0, tzinfo=UTC) + + updates = { + "cron_expression": "0 12 * * *", + "timezone": "America/New_York", + } + + result = ScheduleService.update_schedule( + session=mock_session, + schedule_id="test-schedule-id", + updates=updates, + ) + + assert result is not None + assert result.cron_expression == "0 12 * * *" + assert result.timezone == "America/New_York" + mock_calculate_next_run.assert_called_once() + mock_session.flush.assert_called_once() + + def test_update_schedule_not_found(self): + """Test updating a non-existent schedule.""" + mock_session = MagicMock(spec=Session) + mock_session.get.return_value = None + + result = ScheduleService.update_schedule( + session=mock_session, + schedule_id="non-existent-id", + updates={"cron_expression": "0 12 * * *"}, + ) + + assert result is None + mock_session.flush.assert_not_called() + + def test_delete_schedule(self): + """Test deleting a schedule.""" + mock_session = MagicMock(spec=Session) + mock_schedule = Mock(spec=WorkflowSchedulePlan) + mock_session.get.return_value = mock_schedule + + result = ScheduleService.delete_schedule( + session=mock_session, + schedule_id="test-schedule-id", + ) + + assert result + mock_session.delete.assert_called_once_with(mock_schedule) + mock_session.flush.assert_called_once() + + def test_delete_schedule_not_found(self): + """Test deleting a non-existent schedule.""" + mock_session = MagicMock(spec=Session) + mock_session.get.return_value = None + + result = ScheduleService.delete_schedule( + session=mock_session, + schedule_id="non-existent-id", + ) + + assert not result + mock_session.delete.assert_not_called() + + @patch("services.schedule_service.select") + def test_get_tenant_owner(self, mock_select): + """Test getting tenant owner account.""" + mock_session = MagicMock(spec=Session) + mock_account = Mock(spec=Account) + mock_account.id = "owner-account-id" + + # Mock owner query + mock_owner_result = Mock(spec=TenantAccountJoin) + mock_owner_result.account_id = "owner-account-id" + + mock_session.execute.return_value.scalar_one_or_none.return_value = mock_owner_result + mock_session.get.return_value = mock_account + + result = ScheduleService.get_tenant_owner( + session=mock_session, + tenant_id="test-tenant", + ) + + assert result is not None + assert result.id == "owner-account-id" + + @patch("services.schedule_service.select") + def test_get_tenant_owner_fallback_to_admin(self, mock_select): + """Test getting tenant owner falls back to admin if no owner.""" + mock_session = MagicMock(spec=Session) + mock_account = Mock(spec=Account) + mock_account.id = "admin-account-id" + + # Mock admin query (owner returns None) + mock_admin_result = Mock(spec=TenantAccountJoin) + mock_admin_result.account_id = "admin-account-id" + + mock_session.execute.return_value.scalar_one_or_none.side_effect = [None, mock_admin_result] + mock_session.get.return_value = mock_account + + result = ScheduleService.get_tenant_owner( + session=mock_session, + tenant_id="test-tenant", + ) + + assert result is not None + assert result.id == "admin-account-id" + + @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + def test_update_next_run_at(self, mock_calculate_next_run): + """Test updating next run time after schedule triggered.""" + mock_session = MagicMock(spec=Session) + mock_schedule = Mock(spec=WorkflowSchedulePlan) + mock_schedule.cron_expression = "30 10 * * *" + mock_schedule.timezone = "UTC" + mock_session.get.return_value = mock_schedule + + next_time = datetime(2025, 8, 31, 10, 30, 0, tzinfo=UTC) + mock_calculate_next_run.return_value = next_time + + result = ScheduleService.update_next_run_at( + session=mock_session, + schedule_id="test-schedule-id", + ) + + assert result == next_time + assert mock_schedule.next_run_at == next_time + mock_session.flush.assert_called_once() + + +class TestVisualToCron(unittest.TestCase): + """Test cases for visual configuration to cron conversion.""" + + def test_visual_to_cron_hourly(self): + """Test converting hourly visual config to cron.""" + visual_config = {"on_minute": 15} + result = ScheduleService.visual_to_cron("hourly", visual_config) + assert result == "15 * * * *" + + def test_visual_to_cron_daily(self): + """Test converting daily visual config to cron.""" + visual_config = {"time": "2:30 PM"} + result = ScheduleService.visual_to_cron("daily", visual_config) + assert result == "30 14 * * *" + + def test_visual_to_cron_weekly(self): + """Test converting weekly visual config to cron.""" + visual_config = { + "time": "10:00 AM", + "weekdays": ["mon", "wed", "fri"], + } + result = ScheduleService.visual_to_cron("weekly", visual_config) + assert result == "0 10 * * 1,3,5" + + def test_visual_to_cron_monthly_with_specific_days(self): + """Test converting monthly visual config with specific days.""" + visual_config = { + "time": "11:30 AM", + "monthly_days": [1, 15], + } + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "30 11 1,15 * *" + + def test_visual_to_cron_monthly_with_last_day(self): + """Test converting monthly visual config with last day using 'L' syntax.""" + visual_config = { + "time": "11:30 AM", + "monthly_days": [1, "last"], + } + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "30 11 1,L * *" + + def test_visual_to_cron_monthly_only_last_day(self): + """Test converting monthly visual config with only last day.""" + visual_config = { + "time": "9:00 PM", + "monthly_days": ["last"], + } + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "0 21 L * *" + + def test_visual_to_cron_monthly_with_end_days_and_last(self): + """Test converting monthly visual config with days 29, 30, 31 and 'last'.""" + visual_config = { + "time": "3:45 PM", + "monthly_days": [29, 30, 31, "last"], + } + result = ScheduleService.visual_to_cron("monthly", visual_config) + # Should have 29,30,31,L - the L handles all possible last days + assert result == "45 15 29,30,31,L * *" + + def test_visual_to_cron_invalid_frequency(self): + """Test converting with invalid frequency.""" + result = ScheduleService.visual_to_cron("invalid", {}) + assert result is None + + def test_visual_to_cron_weekly_no_weekdays(self): + """Test converting weekly with no weekdays specified.""" + visual_config = {"time": "10:00 AM"} + result = ScheduleService.visual_to_cron("weekly", visual_config) + assert result is None + + +class TestParseTime(unittest.TestCase): + """Test cases for time parsing function.""" + + def test_parse_time_am(self): + """Test parsing AM time.""" + hour, minute = ScheduleService.parse_time("9:30 AM") + assert hour == 9 + assert minute == 30 + + def test_parse_time_pm(self): + """Test parsing PM time.""" + hour, minute = ScheduleService.parse_time("2:45 PM") + assert hour == 14 + assert minute == 45 + + def test_parse_time_noon(self): + """Test parsing 12:00 PM (noon).""" + hour, minute = ScheduleService.parse_time("12:00 PM") + assert hour == 12 + assert minute == 0 + + def test_parse_time_midnight(self): + """Test parsing 12:00 AM (midnight).""" + hour, minute = ScheduleService.parse_time("12:00 AM") + assert hour == 0 + assert minute == 0 + + def test_parse_time_invalid_format(self): + """Test parsing invalid time format.""" + hour, minute = ScheduleService.parse_time("25:00") + assert hour is None + assert minute is None + + def test_parse_time_invalid_hour(self): + """Test parsing invalid hour.""" + hour, minute = ScheduleService.parse_time("13:00 PM") + assert hour is None + assert minute is None + + def test_parse_time_invalid_minute(self): + """Test parsing invalid minute.""" + hour, minute = ScheduleService.parse_time("10:60 AM") + assert hour is None + assert minute is None + + +class TestExtractScheduleConfig(unittest.TestCase): + """Test cases for extracting schedule configuration from workflow.""" + + def test_extract_schedule_config_with_cron_mode(self): + """Test extracting schedule config in cron mode.""" + workflow = Mock(spec=Workflow) + workflow.graph_dict = { + "nodes": [ + { + "id": "schedule-node", + "data": { + "type": "trigger-schedule", + "mode": "cron", + "cron_expression": "0 10 * * *", + "timezone": "America/New_York", + }, + } + ] + } + + config = ScheduleService.extract_schedule_config(workflow) + + assert config is not None + assert config["node_id"] == "schedule-node" + assert config["cron_expression"] == "0 10 * * *" + assert config["timezone"] == "America/New_York" + + def test_extract_schedule_config_with_visual_mode(self): + """Test extracting schedule config in visual mode.""" + workflow = Mock(spec=Workflow) + workflow.graph_dict = { + "nodes": [ + { + "id": "schedule-node", + "data": { + "type": "trigger-schedule", + "mode": "visual", + "frequency": "daily", + "visual_config": {"time": "10:30 AM"}, + "timezone": "UTC", + }, + } + ] + } + + config = ScheduleService.extract_schedule_config(workflow) + + assert config is not None + assert config["node_id"] == "schedule-node" + assert config["cron_expression"] == "30 10 * * *" + assert config["timezone"] == "UTC" + + def test_extract_schedule_config_no_schedule_node(self): + """Test extracting config when no schedule node exists.""" + workflow = Mock(spec=Workflow) + workflow.graph_dict = { + "nodes": [ + { + "id": "other-node", + "data": {"type": "llm"}, + } + ] + } + + config = ScheduleService.extract_schedule_config(workflow) + assert config is None + + def test_extract_schedule_config_invalid_graph(self): + """Test extracting config with invalid graph data.""" + workflow = Mock(spec=Workflow) + workflow.graph_dict = None + + config = ScheduleService.extract_schedule_config(workflow) + assert config is None + + +class TestScheduleWithTimezone(unittest.TestCase): + """Test cases for schedule with timezone handling.""" + + def test_visual_schedule_with_timezone_integration(self): + """Test complete flow: visual config → cron → execution in different timezones. + + This test verifies that when a user in Shanghai sets a schedule for 10:30 AM, + it runs at 10:30 AM Shanghai time, not 10:30 AM UTC. + """ + # User in Shanghai wants to run a task at 10:30 AM local time + visual_config = { + "time": "10:30 AM", # This is Shanghai time + "monthly_days": [1], + } + + # Convert to cron expression + cron_expr = ScheduleService.visual_to_cron("monthly", visual_config) + assert cron_expr is not None + + assert cron_expr == "30 10 1 * *" # Direct conversion + + # Now test execution with Shanghai timezone + shanghai_tz = "Asia/Shanghai" + # Base time: 2025-01-01 00:00:00 UTC (08:00:00 Shanghai) + base_time = datetime(2025, 1, 1, 0, 0, 0, tzinfo=UTC) + + next_run = ScheduleService.calculate_next_run_at(cron_expr, shanghai_tz, base_time) + + assert next_run is not None + + # Should run at 10:30 AM Shanghai time on Jan 1 + # 10:30 AM Shanghai = 02:30 AM UTC (Shanghai is UTC+8) + assert next_run.year == 2025 + assert next_run.month == 1 + assert next_run.day == 1 + assert next_run.hour == 2 # 02:30 UTC + assert next_run.minute == 30 + + def test_visual_schedule_different_timezones_same_local_time(self): + """Test that same visual config in different timezones runs at different UTC times. + + This verifies that a schedule set for "9:00 AM" runs at 9 AM local time + regardless of the timezone. + """ + visual_config = { + "time": "9:00 AM", + "weekdays": ["mon"], + } + + cron_expr = ScheduleService.visual_to_cron("weekly", visual_config) + assert cron_expr is not None + assert cron_expr == "0 9 * * 1" + + # Base time: Sunday 2025-01-05 12:00:00 UTC + base_time = datetime(2025, 1, 5, 12, 0, 0, tzinfo=UTC) + + # Test New York (UTC-5 in January) + ny_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", base_time) + assert ny_next is not None + # Monday 9 AM EST = Monday 14:00 UTC + assert ny_next.day == 6 + assert ny_next.hour == 14 # 9 AM EST = 2 PM UTC + + # Test Tokyo (UTC+9) + tokyo_next = ScheduleService.calculate_next_run_at(cron_expr, "Asia/Tokyo", base_time) + assert tokyo_next is not None + # Monday 9 AM JST = Monday 00:00 UTC + assert tokyo_next.day == 6 + assert tokyo_next.hour == 0 # 9 AM JST = 0 AM UTC + + def test_visual_schedule_daily_across_dst_change(self): + """Test that daily schedules adjust correctly during DST changes. + + A schedule set for "10:00 AM" should always run at 10 AM local time, + even when DST changes. + """ + visual_config = { + "time": "10:00 AM", + } + + cron_expr = ScheduleService.visual_to_cron("daily", visual_config) + assert cron_expr is not None + + assert cron_expr == "0 10 * * *" + + # Test before DST (EST - UTC-5) + winter_base = datetime(2025, 2, 1, 0, 0, 0, tzinfo=UTC) + winter_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", winter_base) + assert winter_next is not None + # 10 AM EST = 15:00 UTC + assert winter_next.hour == 15 + + # Test during DST (EDT - UTC-4) + summer_base = datetime(2025, 6, 1, 0, 0, 0, tzinfo=UTC) + summer_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", summer_base) + assert summer_next is not None + # 10 AM EDT = 14:00 UTC + assert summer_next.hour == 14 + + +class TestSyncScheduleFromWorkflow(unittest.TestCase): + """Test cases for syncing schedule from workflow.""" + + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.db") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.ScheduleService") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.select") + def test_sync_schedule_create_new(self, mock_select, mock_service, mock_db): + """Test creating new schedule when none exists.""" + mock_session = MagicMock() + mock_db.engine = MagicMock() + mock_session.__enter__ = MagicMock(return_value=mock_session) + mock_session.__exit__ = MagicMock(return_value=None) + Session = MagicMock(return_value=mock_session) + with patch("events.event_handlers.sync_workflow_schedule_when_app_published.Session", Session): + mock_session.scalar.return_value = None # No existing plan + + mock_service.extract_schedule_config.return_value = { + "node_id": "start", + "cron_expression": "30 10 * * *", + "timezone": "UTC", + } + + mock_new_plan = Mock(spec=WorkflowSchedulePlan) + mock_service.create_schedule.return_value = mock_new_plan + + workflow = Mock(spec=Workflow) + result = sync_schedule_from_workflow("tenant-id", "app-id", workflow) + + assert result == mock_new_plan + mock_service.create_schedule.assert_called_once() + mock_session.commit.assert_called_once() + + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.db") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.ScheduleService") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.select") + def test_sync_schedule_update_existing(self, mock_select, mock_service, mock_db): + """Test updating existing schedule.""" + mock_session = MagicMock() + mock_db.engine = MagicMock() + mock_session.__enter__ = MagicMock(return_value=mock_session) + mock_session.__exit__ = MagicMock(return_value=None) + Session = MagicMock(return_value=mock_session) + + with patch("events.event_handlers.sync_workflow_schedule_when_app_published.Session", Session): + mock_existing_plan = Mock(spec=WorkflowSchedulePlan) + mock_existing_plan.id = "existing-plan-id" + mock_session.scalar.return_value = mock_existing_plan + + mock_service.extract_schedule_config.return_value = { + "node_id": "start", + "cron_expression": "0 12 * * *", + "timezone": "America/New_York", + } + + mock_updated_plan = Mock(spec=WorkflowSchedulePlan) + mock_service.update_schedule.return_value = mock_updated_plan + + workflow = Mock(spec=Workflow) + result = sync_schedule_from_workflow("tenant-id", "app-id", workflow) + + assert result == mock_updated_plan + mock_service.update_schedule.assert_called_once_with( + session=mock_session, + schedule_id="existing-plan-id", + updates={ + "node_id": "start", + "cron_expression": "0 12 * * *", + "timezone": "America/New_York", + }, + ) + mock_session.commit.assert_called_once() + + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.db") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.ScheduleService") + @patch("events.event_handlers.sync_workflow_schedule_when_app_published.select") + def test_sync_schedule_remove_when_no_config(self, mock_select, mock_service, mock_db): + """Test removing schedule when no schedule config in workflow.""" + mock_session = MagicMock() + mock_db.engine = MagicMock() + mock_session.__enter__ = MagicMock(return_value=mock_session) + mock_session.__exit__ = MagicMock(return_value=None) + Session = MagicMock(return_value=mock_session) + + with patch("events.event_handlers.sync_workflow_schedule_when_app_published.Session", Session): + mock_existing_plan = Mock(spec=WorkflowSchedulePlan) + mock_session.scalar.return_value = mock_existing_plan + + mock_service.extract_schedule_config.return_value = None # No schedule config + + workflow = Mock(spec=Workflow) + result = sync_schedule_from_workflow("tenant-id", "app-id", workflow) + + assert result is None + mock_session.delete.assert_called_once_with(mock_existing_plan) + mock_session.commit.assert_called_once() + + +if __name__ == "__main__": + unittest.main() From 29fa80fd14dda153e64f3048e29fdd1795a452e1 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 18:11:11 +0800 Subject: [PATCH 08/26] chore: update schedule node default config & remove enable param --- .../nodes/trigger_schedule/entities.py | 1 - .../trigger_schedule/trigger_schedule_node.py | 42 +++++++++---------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/api/core/workflow/nodes/trigger_schedule/entities.py b/api/core/workflow/nodes/trigger_schedule/entities.py index 9cd081568bc324..01c66ab2a65e80 100644 --- a/api/core/workflow/nodes/trigger_schedule/entities.py +++ b/api/core/workflow/nodes/trigger_schedule/entities.py @@ -17,4 +17,3 @@ class TriggerScheduleNodeData(BaseNodeData): cron_expression: Optional[str] = Field(default=None, description="Cron expression for cron mode") visual_config: Optional[dict] = Field(default=None, description="Visual configuration details") timezone: str = Field(default="UTC", description="Timezone for schedule execution") - enabled: bool = Field(default=True, description="Whether the schedule is enabled") diff --git a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py index 5611945dcbdb79..e23d5369894e4d 100644 --- a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py +++ b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py @@ -40,31 +40,29 @@ def get_base_node_data(self) -> BaseNodeData: @classmethod def version(cls) -> str: return "1" + + @classmethod + def get_default_config(cls, filters: Optional[dict] = None) -> dict: + return { + "type": "trigger-schedule", + "config": { + "mode": "visual", + "frequency": "weekly", + "visual_config": { + "time": "11:30 AM", + "on_minute": 0, + "weekdays": ["sun"], + "monthly_days": [1] + }, + "timezone": "UTC" + } + } def _run(self) -> NodeRunResult: - node_inputs = dict(self.graph_runtime_state.variable_pool.user_inputs) - system_inputs = self.graph_runtime_state.variable_pool.system_variables.to_dict() - - # Set system variables as node outputs - for var in system_inputs: - node_inputs[SYSTEM_VARIABLE_NODE_ID + "." + var] = system_inputs[var] - - # Add schedule-specific outputs - triggered_at = datetime.now(UTC) - node_inputs["triggered_at"] = triggered_at.isoformat() - node_inputs["timezone"] = self._node_data.timezone - node_inputs["mode"] = self._node_data.mode - node_inputs["enabled"] = self._node_data.enabled - - # Add configuration details based on mode - if self._node_data.mode == "cron" and self._node_data.cron_expression: - node_inputs["cron_expression"] = self._node_data.cron_expression - elif self._node_data.mode == "visual": - node_inputs["frequency"] = self._node_data.frequency or "unknown" - if self._node_data.visual_config: - node_inputs["visual_config"] = self._node_data.visual_config + current_time = datetime.now(UTC) + node_outputs = {"current_time": current_time.isoformat()} return NodeRunResult( status=WorkflowNodeExecutionStatus.SUCCEEDED, - outputs=node_inputs, + outputs=node_outputs, ) From 5b49cdaaff620c6cd6d31301b2cad71d6ffd344d Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 18:26:27 +0800 Subject: [PATCH 09/26] chore: remove unused import and clean up comments in ScheduleService --- .../trigger_schedule/trigger_schedule_node.py | 1 - api/services/schedule_service.py | 119 +++--------------- 2 files changed, 20 insertions(+), 100 deletions(-) diff --git a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py index e23d5369894e4d..7852cf95c50f0c 100644 --- a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py +++ b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py @@ -2,7 +2,6 @@ from datetime import UTC, datetime from typing import Any, Optional -from core.workflow.constants import SYSTEM_VARIABLE_NODE_ID from core.workflow.entities.node_entities import NodeRunResult from core.workflow.entities.workflow_node_execution import WorkflowNodeExecutionStatus from core.workflow.nodes.base import BaseNode diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 3497207e6c4d07..88783687c853e3 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -44,27 +44,12 @@ def create_schedule( cron_expression: str, timezone: str, ) -> WorkflowSchedulePlan: - """ - Create a new workflow schedule. - - Args: - session: Database session - tenant_id: Tenant ID - app_id: Application ID - node_id: Starting node ID - cron_expression: Cron expression - timezone: Timezone for cron evaluation - - Returns: - Created WorkflowSchedulePlan instance - """ - # Calculate initial next run time + # Pre-calculate next_run_at to ensure the schedule is valid before persisting next_run_at = ScheduleService.calculate_next_run_at( cron_expression, timezone, ) - # Create schedule record schedule = WorkflowSchedulePlan( tenant_id=tenant_id, app_id=app_id, @@ -85,27 +70,15 @@ def update_schedule( schedule_id: str, updates: dict, ) -> Optional[WorkflowSchedulePlan]: - """ - Update a schedule with the provided changes. - - Args: - session: Database session - schedule_id: Schedule ID to update - updates: Dictionary of fields to update - - Returns: - Updated schedule or None if not found - """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: return None - # Apply updates for field, value in updates.items(): if hasattr(schedule, field): setattr(schedule, field, value) - # Recalculate next_run_at if schedule parameters changed + # Ensure next_run_at stays accurate when schedule timing changes if any(field in updates for field in ["cron_expression", "timezone"]): next_run_at = ScheduleService.calculate_next_run_at( schedule.cron_expression, @@ -121,16 +94,6 @@ def delete_schedule( session: Session, schedule_id: str, ) -> bool: - """ - Delete a schedule. - - Args: - session: Database session - schedule_id: Schedule ID to delete - - Returns: - True if deleted, False if not found - """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: return False @@ -142,17 +105,9 @@ def delete_schedule( @staticmethod def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: """ - Get the owner account for a tenant. - Used to execute scheduled workflows on behalf of the tenant owner. - - Args: - session: Database session - tenant_id: Tenant ID - - Returns: - Owner Account or None + Returns an account to execute scheduled workflows on behalf of the tenant. + Prioritizes owner over admin to ensure proper authorization hierarchy. """ - # Query for owner role in tenant result = session.execute( select(TenantAccountJoin) .where(TenantAccountJoin.tenant_id == tenant_id, TenantAccountJoin.role == "owner") @@ -160,7 +115,7 @@ def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: ).scalar_one_or_none() if not result: - # Fallback to any admin if no owner + # Owner may not exist in some tenant configurations, fallback to admin result = session.execute( select(TenantAccountJoin) .where(TenantAccountJoin.tenant_id == tenant_id, TenantAccountJoin.role == "admin") @@ -178,28 +133,20 @@ def update_next_run_at( schedule_id: str, ) -> Optional[datetime]: """ - Calculate and update the next run time for a schedule. - Used after a schedule has been triggered. - - Args: - session: Database session - schedule_id: Schedule ID - - Returns: - New next_run_at datetime or None + Advances the schedule to its next execution time after a successful trigger. + Uses current time as base to prevent missing executions during delays. """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: return None - # Calculate next run time + # Base on current time to handle execution delays gracefully next_run_at = ScheduleService.calculate_next_run_at( schedule.cron_expression, schedule.timezone, - base_time=datetime.now(UTC), # Use current time as base + base_time=datetime.now(UTC), ) - # Update schedule schedule.next_run_at = next_run_at session.flush() return next_run_at @@ -207,13 +154,8 @@ def update_next_run_at( @staticmethod def extract_schedule_config(workflow: Workflow) -> Optional[dict]: """ - Extract schedule configuration from workflow graph. - - Args: - workflow: Workflow instance - - Returns: - Schedule configuration dict or None if no schedule node found + Extracts schedule configuration from workflow graph for synchronization. + Normalizes both visual and cron modes into a unified cron expression format. """ try: graph_data = workflow.graph_dict @@ -223,16 +165,14 @@ def extract_schedule_config(workflow: Workflow) -> Optional[dict]: if not graph_data: return None - # Find schedule trigger node in graph for node in graph_data.get("nodes", []): node_data = node.get("data", {}) if node_data.get("type") == NodeType.TRIGGER_SCHEDULE.value: - # Extract configuration mode = node_data.get("mode", "visual") timezone = node_data.get("timezone", "UTC") node_id = node.get("id", "start") - # Convert to cron expression + # Normalize both modes to cron expression for consistent storage cron_expression = None if mode == "cron": cron_expression = node_data.get("cron_expression") @@ -256,26 +196,18 @@ def extract_schedule_config(workflow: Workflow) -> Optional[dict]: @staticmethod def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: """ - Convert visual schedule configuration to cron expression. - - Args: - frequency: Schedule frequency (hourly, daily, weekly, monthly) - visual_config: Visual configuration with time, weekdays, etc. - - Returns: - Cron expression string or None if invalid + Converts user-friendly visual schedule settings to cron expression. + Maintains consistency with frontend UI expectations while supporting croniter's extended syntax. """ if not frequency or not visual_config: return None try: if frequency == "hourly": - # Run at specific minute of every hour on_minute = visual_config.get("on_minute", 0) return f"{on_minute} * * * *" elif frequency == "daily": - # Run daily at specific time time_str = visual_config.get("time", "12:00 PM") hour, minute = ScheduleService.parse_time(time_str) if hour is None or minute is None: @@ -283,7 +215,6 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: return f"{minute} {hour} * * *" elif frequency == "weekly": - # Run weekly on specific days at specific time time_str = visual_config.get("time", "12:00 PM") hour, minute = ScheduleService.parse_time(time_str) if hour is None or minute is None: @@ -293,7 +224,7 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: if not weekdays: return None - # Convert weekday names to cron format (0-6, where 0=Sunday) + # Map to cron's 0-6 format where 0=Sunday for standard cron compatibility weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} cron_weekdays = [] for day in weekdays: @@ -306,7 +237,6 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" elif frequency == "monthly": - # Run monthly on specific days at specific time time_str = visual_config.get("time", "12:00 PM") hour, minute = ScheduleService.parse_time(time_str) if hour is None or minute is None: @@ -316,11 +246,10 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: if not monthly_days: return None - # Convert monthly days to cron format cron_days = [] for day in monthly_days: if day == "last": - # Use 'L' to represent the last day of month (supported by croniter) + # croniter supports 'L' for last day of month, avoiding hardcoded day ranges cron_days.append("L") elif isinstance(day, int) and 1 <= day <= 31: cron_days.append(str(day)) @@ -328,7 +257,7 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: if not cron_days: return None - # Sort numeric days, but keep 'L' at the end if present + # Keep 'L' at end for readability while sorting numeric days numeric_days = [d for d in cron_days if d != "L"] has_last = "L" in cron_days @@ -349,16 +278,10 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: @staticmethod def parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: """ - Parse time string in format "HH:MM AM/PM" to 24-hour format. - - Args: - time_str: Time string like "11:30 AM" or "2:45 PM" - - Returns: - Tuple of (hour, minute) in 24-hour format, or (None, None) if invalid + Parses 12-hour time format to 24-hour for cron compatibility. + Handles edge cases like 12:00 AM (midnight) and 12:00 PM (noon). """ try: - # Split time and period parts = time_str.strip().split() if len(parts) != 2: return None, None @@ -369,7 +292,6 @@ def parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: if period not in ["AM", "PM"]: return None, None - # Parse hour and minute time_parts = time_part.split(":") if len(time_parts) != 2: return None, None @@ -377,11 +299,10 @@ def parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: hour = int(time_parts[0]) minute = int(time_parts[1]) - # Validate ranges if hour < 1 or hour > 12 or minute < 0 or minute > 59: return None, None - # Convert to 24-hour format + # Handle 12-hour to 24-hour edge cases if period == "PM" and hour != 12: hour += 12 elif period == "AM" and hour == 12: From 7f4c40393447ca64a9c21b866cbe4715f64e6f0d Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 22:48:16 +0800 Subject: [PATCH 10/26] refactor: simplify schedule polling logic and reduce code complexity --- api/schedule/workflow_schedule_task.py | 226 ++++++++----------------- api/services/schedule_service.py | 1 - 2 files changed, 72 insertions(+), 155 deletions(-) diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py index 844b76e0d4fe91..b8022f2844914b 100644 --- a/api/schedule/workflow_schedule_task.py +++ b/api/schedule/workflow_schedule_task.py @@ -1,5 +1,4 @@ import logging -from collections import defaultdict from datetime import UTC, datetime from celery import shared_task @@ -18,182 +17,101 @@ @shared_task(queue="schedule") def poll_workflow_schedules() -> None: - """Poll and process due workflow schedules.""" + """ + Poll and process due workflow schedules. + + Simple 3-step flow: + 1. Get rate-limited tenants from Redis + 2. Fetch due schedules excluding rate-limited tenants + 3. Process and dispatch valid schedules + """ session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) with session_factory() as session: - due_schedules = _fetch_due_schedules(session) + rate_limited_tenants = _get_rate_limited_tenants(session) + due_schedules = _fetch_due_schedules(session, exclude_tenants=rate_limited_tenants) if due_schedules: - logger.info("Processing %d due schedules", len(due_schedules)) processed = _process_schedules(session, due_schedules) - if processed: - logger.info("Successfully dispatched %d schedules", processed) - - -def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: - """Fetch all schedules that are due for execution.""" - now = datetime.now(UTC) - - return list( - session.scalars( - select(WorkflowSchedulePlan) - .join( - AppTrigger, - and_( - AppTrigger.app_id == WorkflowSchedulePlan.app_id, - AppTrigger.node_id == WorkflowSchedulePlan.node_id, - AppTrigger.trigger_type == "trigger-schedule", - ), - ) - .where( - WorkflowSchedulePlan.next_run_at <= now, - WorkflowSchedulePlan.next_run_at.isnot(None), - AppTrigger.status == AppTriggerStatus.ENABLED, + logger.info( + "Processed %d/%d schedules (%d tenants rate-limited)", + processed, + len(due_schedules), + len(rate_limited_tenants), ) - .with_for_update(skip_locked=True) - .limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE) - ).all() - ) - - -def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> int: - """Process schedules with rate limiting and two-phase commit.""" - if not schedules: - return 0 - - start_time = datetime.now(UTC) - - schedules_by_tenant = _group_by_tenant(schedules) - - dispatch_list, removal_list, rate_limited = _prepare_schedule_updates(schedules_by_tenant) - - if not _commit_schedule_updates(session, dispatch_list, removal_list): - return 0 - dispatched_count = _dispatch_schedules(dispatch_list, rate_limited) - - _log_batch_summary( - total_count=len(schedules), - dispatched_count=dispatched_count, - removal_count=len(removal_list), - rate_limited_tenants=rate_limited, - elapsed_time=(datetime.now(UTC) - start_time).total_seconds(), - ) - - return dispatched_count - - -def _group_by_tenant(schedules: list[WorkflowSchedulePlan]) -> dict[str, list[WorkflowSchedulePlan]]: - """Group schedules by tenant ID for efficient processing.""" - schedules_by_tenant = defaultdict(list) - for schedule in schedules: - schedules_by_tenant[schedule.tenant_id].append(schedule) - return schedules_by_tenant +def _get_rate_limited_tenants(session: Session) -> set[str]: + """Get tenants that have reached their daily rate limit.""" + now = datetime.now(UTC) -def _prepare_schedule_updates( - schedules_by_tenant: dict[str, list[WorkflowSchedulePlan]], -) -> tuple[list[WorkflowSchedulePlan], list[WorkflowSchedulePlan], set[str]]: - """ - Process schedules and check rate limits. + tenant_ids = session.scalars( + select(WorkflowSchedulePlan.tenant_id) + .distinct() + .where( + WorkflowSchedulePlan.next_run_at <= now, + WorkflowSchedulePlan.next_run_at.isnot(None), + ) + ).all() - Returns: - Tuple of (schedules_to_dispatch, schedules_to_remove, rate_limited_tenants) - """ - schedules_to_dispatch = [] - schedules_to_remove = [] - rate_limited_tenants = set() + if not tenant_ids: + return set() dispatcher_manager = QueueDispatcherManager() + return { + tenant_id + for tenant_id in tenant_ids + if not dispatcher_manager.get_dispatcher(tenant_id).check_daily_quota(tenant_id) + } - for tenant_id, tenant_schedules in schedules_by_tenant.items(): - if _is_tenant_rate_limited(tenant_id, dispatcher_manager): - rate_limited_tenants.add(tenant_id) - logger.warning("Tenant %s reached daily limit, skipping %d schedules", tenant_id, len(tenant_schedules)) - continue - - for schedule in tenant_schedules: - next_run_at = ScheduleService.calculate_next_run_at( - schedule.cron_expression, - schedule.timezone, - ) - - if next_run_at: - schedule.next_run_at = next_run_at - schedules_to_dispatch.append(schedule) - else: - schedules_to_remove.append(schedule) - - return schedules_to_dispatch, schedules_to_remove, rate_limited_tenants - - -def _is_tenant_rate_limited(tenant_id: str, dispatcher_manager: QueueDispatcherManager) -> bool: - """Check if tenant has reached daily rate limit.""" - dispatcher = dispatcher_manager.get_dispatcher(tenant_id) - return not dispatcher.check_daily_quota(tenant_id) +def _fetch_due_schedules(session: Session, exclude_tenants: set[str]) -> list[WorkflowSchedulePlan]: + """Fetch all schedules that are due for execution, excluding rate-limited tenants.""" + now = datetime.now(UTC) -def _commit_schedule_updates( - session: Session, schedules_to_dispatch: list[WorkflowSchedulePlan], schedules_to_remove: list[WorkflowSchedulePlan] -) -> bool: - """ - Commit schedule updates to database. + query = ( + select(WorkflowSchedulePlan) + .join( + AppTrigger, + and_( + AppTrigger.app_id == WorkflowSchedulePlan.app_id, + AppTrigger.node_id == WorkflowSchedulePlan.node_id, + AppTrigger.trigger_type == "trigger-schedule", + ), + ) + .where( + WorkflowSchedulePlan.next_run_at <= now, + WorkflowSchedulePlan.next_run_at.isnot(None), + AppTrigger.status == AppTriggerStatus.ENABLED, + ) + ) - Returns: - True if commit successful, False otherwise - """ - for schedule in schedules_to_remove: - session.delete(schedule) + if exclude_tenants: + query = query.where(WorkflowSchedulePlan.tenant_id.notin_(exclude_tenants)) - try: - session.commit() - return True - except Exception as e: - total_count = len(schedules_to_dispatch) + len(schedules_to_remove) - logger.error("Failed to commit %d schedule updates: %s", total_count, e, exc_info=True) - session.rollback() - return False + return list( + session.scalars(query.with_for_update(skip_locked=True).limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE)) + ) -def _dispatch_schedules(schedules_to_dispatch: list[WorkflowSchedulePlan], rate_limited_tenants: set[str]) -> int: - """ - Dispatch schedules to Celery for execution. +def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> int: + """Process schedules: update next run time and dispatch to Celery.""" + if not schedules: + return 0 - Returns: - Number of successfully dispatched schedules - """ - dispatched_count = 0 + dispatched = 0 - for schedule in schedules_to_dispatch: - # Skip if tenant was rate limited - if schedule.tenant_id in rate_limited_tenants: - continue + for schedule in schedules: + next_run_at = ScheduleService.calculate_next_run_at( + schedule.cron_expression, + schedule.timezone, + ) - try: + if next_run_at: + schedule.next_run_at = next_run_at run_schedule_trigger.delay(schedule.id) - dispatched_count += 1 - except Exception as e: - logger.error("Failed to dispatch schedule %s: %s", schedule.id, e, exc_info=True) - - return dispatched_count - + dispatched += 1 -def _log_batch_summary( - total_count: int, dispatched_count: int, removal_count: int, rate_limited_tenants: set[str], elapsed_time: float -) -> None: - """Log a summary of the batch processing.""" - summary_parts = [f"{dispatched_count} dispatched"] + session.commit() - if rate_limited_tenants: - summary_parts.append(f"{len(rate_limited_tenants)} tenants rate-limited") - - if removal_count > 0: - summary_parts.append(f"{removal_count} expired") - - logger.info( - "Batch completed: %d processed (%s) in %.3fs", - total_count, - ", ".join(summary_parts), - elapsed_time, - ) + return dispatched diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 88783687c853e3..adce511de27095 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -144,7 +144,6 @@ def update_next_run_at( next_run_at = ScheduleService.calculate_next_run_at( schedule.cron_expression, schedule.timezone, - base_time=datetime.now(UTC), ) schedule.next_run_at = next_run_at From 2bab93b77fedd8c085372a50d356f35b05d19630 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 22:48:43 +0800 Subject: [PATCH 11/26] chore: reformat --- .../nodes/trigger_schedule/trigger_schedule_node.py | 13 ++++--------- api/fields/workflow_trigger_fields.py | 2 +- api/services/workflow/queue_dispatcher.py | 8 ++------ api/services/workflow/rate_limiter.py | 8 ++++---- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py index 7852cf95c50f0c..056bbc1f98dd8a 100644 --- a/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py +++ b/api/core/workflow/nodes/trigger_schedule/trigger_schedule_node.py @@ -39,7 +39,7 @@ def get_base_node_data(self) -> BaseNodeData: @classmethod def version(cls) -> str: return "1" - + @classmethod def get_default_config(cls, filters: Optional[dict] = None) -> dict: return { @@ -47,14 +47,9 @@ def get_default_config(cls, filters: Optional[dict] = None) -> dict: "config": { "mode": "visual", "frequency": "weekly", - "visual_config": { - "time": "11:30 AM", - "on_minute": 0, - "weekdays": ["sun"], - "monthly_days": [1] - }, - "timezone": "UTC" - } + "visual_config": {"time": "11:30 AM", "on_minute": 0, "weekdays": ["sun"], "monthly_days": [1]}, + "timezone": "UTC", + }, } def _run(self) -> NodeRunResult: diff --git a/api/fields/workflow_trigger_fields.py b/api/fields/workflow_trigger_fields.py index 665d4589ffa1f6..1e2fc3c3dbbf02 100644 --- a/api/fields/workflow_trigger_fields.py +++ b/api/fields/workflow_trigger_fields.py @@ -23,4 +23,4 @@ "node_id": fields.String, "triggered_by": fields.String, "created_at": fields.DateTime(dt_format="iso8601"), -} \ No newline at end of file +} diff --git a/api/services/workflow/queue_dispatcher.py b/api/services/workflow/queue_dispatcher.py index a3d71bbdd87df0..782f351d318581 100644 --- a/api/services/workflow/queue_dispatcher.py +++ b/api/services/workflow/queue_dispatcher.py @@ -55,9 +55,7 @@ def check_daily_quota(self, tenant_id: str) -> bool: True if quota available, False otherwise """ # Check without consuming - remaining = self.rate_limiter.get_remaining_quota( - tenant_id=tenant_id, max_daily_limit=self.get_daily_limit() - ) + remaining = self.rate_limiter.get_remaining_quota(tenant_id=tenant_id, max_daily_limit=self.get_daily_limit()) return remaining > 0 def consume_quota(self, tenant_id: str) -> bool: @@ -70,9 +68,7 @@ def consume_quota(self, tenant_id: str) -> bool: Returns: True if quota consumed successfully, False if limit reached """ - return self.rate_limiter.check_and_consume( - tenant_id=tenant_id, max_daily_limit=self.get_daily_limit() - ) + return self.rate_limiter.check_and_consume(tenant_id=tenant_id, max_daily_limit=self.get_daily_limit()) class ProfessionalQueueDispatcher(BaseQueueDispatcher): diff --git a/api/services/workflow/rate_limiter.py b/api/services/workflow/rate_limiter.py index 6f83ed8ad43e2f..dff284538aae47 100644 --- a/api/services/workflow/rate_limiter.py +++ b/api/services/workflow/rate_limiter.py @@ -74,10 +74,10 @@ def _get_ttl_seconds(self) -> int: Number of seconds until UTC midnight """ utc_now = datetime.utcnow() - + # Get next midnight in UTC next_midnight = datetime.combine(utc_now.date() + timedelta(days=1), time.min) - + return int((next_midnight - utc_now).total_seconds()) def check_and_consume(self, tenant_id: str, max_daily_limit: int) -> bool: @@ -174,9 +174,9 @@ def get_quota_reset_time(self, tenant_id: str, timezone_str: str) -> datetime: """ tz = pytz.timezone(timezone_str) utc_now = datetime.utcnow() - + # Get next midnight in UTC, then convert to tenant's timezone next_utc_midnight = datetime.combine(utc_now.date() + timedelta(days=1), time.min) next_utc_midnight = pytz.UTC.localize(next_utc_midnight) - + return next_utc_midnight.astimezone(tz) From f0ac0085a2118d632c4b81b62de384c7769d7dad Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Fri, 29 Aug 2025 22:49:42 +0800 Subject: [PATCH 12/26] feat: add start-beat --- dev/start-beat | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100755 dev/start-beat diff --git a/dev/start-beat b/dev/start-beat new file mode 100755 index 00000000000000..e417874b258443 --- /dev/null +++ b/dev/start-beat @@ -0,0 +1,60 @@ +#!/bin/bash + +set -x + +# Help function +show_help() { + echo "Usage: $0 [OPTIONS]" + echo "" + echo "Options:" + echo " --loglevel LEVEL Log level (default: INFO)" + echo " --scheduler SCHEDULER Scheduler class (default: celery.beat:PersistentScheduler)" + echo " -h, --help Show this help message" + echo "" + echo "Examples:" + echo " $0" + echo " $0 --loglevel DEBUG" + echo " $0 --scheduler django_celery_beat.schedulers:DatabaseScheduler" + echo "" + echo "Description:" + echo " Starts Celery Beat scheduler for periodic task execution." + echo " Beat sends scheduled tasks to worker queues at specified intervals." +} + +# Parse command line arguments +LOGLEVEL="INFO" +SCHEDULER="celery.beat:PersistentScheduler" + +while [[ $# -gt 0 ]]; do + case $1 in + --loglevel) + LOGLEVEL="$2" + shift 2 + ;; + --scheduler) + SCHEDULER="$2" + shift 2 + ;; + -h|--help) + show_help + exit 0 + ;; + *) + echo "Unknown option: $1" + show_help + exit 1 + ;; + esac +done + +SCRIPT_DIR="$(dirname "$(realpath "$0")")" +cd "$SCRIPT_DIR/.." + +echo "Starting Celery Beat with:" +echo " Log Level: ${LOGLEVEL}" +echo " Scheduler: ${SCHEDULER}" + +uv --directory api run \ + celery -A app.celery beat \ + --loglevel ${LOGLEVEL} \ + --scheduler ${SCHEDULER} \ No newline at end of file From 66591bc6fa93a3b51a6541e3028aeb88e8c4a443 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 00:45:42 +0800 Subject: [PATCH 13/26] refactor: use Pydantic.Basemodel handling schedule plan --- .../nodes/trigger_schedule/entities.py | 14 ++++++++++- ...nc_workflow_schedule_when_app_published.py | 23 +++++++------------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/api/core/workflow/nodes/trigger_schedule/entities.py b/api/core/workflow/nodes/trigger_schedule/entities.py index 01c66ab2a65e80..0881c55e1449fb 100644 --- a/api/core/workflow/nodes/trigger_schedule/entities.py +++ b/api/core/workflow/nodes/trigger_schedule/entities.py @@ -1,6 +1,6 @@ from typing import Optional -from pydantic import Field +from pydantic import BaseModel, Field from core.workflow.nodes.base import BaseNodeData @@ -17,3 +17,15 @@ class TriggerScheduleNodeData(BaseNodeData): cron_expression: Optional[str] = Field(default=None, description="Cron expression for cron mode") visual_config: Optional[dict] = Field(default=None, description="Visual configuration details") timezone: str = Field(default="UTC", description="Timezone for schedule execution") + + +class ScheduleConfig(BaseModel): + node_id: str + cron_expression: str + timezone: str = "UTC" + + +class SchedulePlanUpdate(BaseModel): + node_id: Optional[str] = None + cron_expression: Optional[str] = None + timezone: Optional[str] = None diff --git a/api/events/event_handlers/sync_workflow_schedule_when_app_published.py b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py index 91d6aa1d205235..928ce60bd28575 100644 --- a/api/events/event_handlers/sync_workflow_schedule_when_app_published.py +++ b/api/events/event_handlers/sync_workflow_schedule_when_app_published.py @@ -4,6 +4,7 @@ from sqlalchemy import select from sqlalchemy.orm import Session +from core.workflow.nodes.trigger_schedule.entities import SchedulePlanUpdate from events.app_event import app_published_workflow_was_updated from extensions.ext_database import db from models import AppMode, Workflow, WorkflowSchedulePlan @@ -57,21 +58,16 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) if not schedule_config: if existing_plan: logger.info("No schedule node in workflow for app %s, removing schedule plan", app_id) - session.delete(existing_plan) + ScheduleService.delete_schedule(session=session, schedule_id=existing_plan.id) session.commit() return None - node_id = schedule_config["node_id"] - cron_expression = schedule_config["cron_expression"] - timezone = schedule_config["timezone"] - if existing_plan: - logger.info("Updating schedule plan for app %s", app_id) - updates = { - "node_id": node_id, - "cron_expression": cron_expression, - "timezone": timezone, - } + updates = SchedulePlanUpdate( + node_id=schedule_config.node_id, + cron_expression=schedule_config.cron_expression, + timezone=schedule_config.timezone, + ) updated_plan = ScheduleService.update_schedule( session=session, schedule_id=existing_plan.id, @@ -80,14 +76,11 @@ def sync_schedule_from_workflow(tenant_id: str, app_id: str, workflow: Workflow) session.commit() return updated_plan else: - logger.info("Creating new schedule plan for app %s", app_id) new_plan = ScheduleService.create_schedule( session=session, tenant_id=tenant_id, app_id=app_id, - node_id=node_id, - cron_expression=cron_expression, - timezone=timezone, + config=schedule_config, ) session.commit() return new_plan From 8882df7fd99f9f5f12a6fc06ecf2fe6a4ba88df8 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 00:51:18 +0800 Subject: [PATCH 14/26] feat: add ScheduleNodeError handling & convert def that do not reference ScheduleService to utils --- .../workflow/nodes/trigger_schedule/exc.py | 19 ++++ api/libs/schedule_utils.py | 89 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 api/core/workflow/nodes/trigger_schedule/exc.py create mode 100644 api/libs/schedule_utils.py diff --git a/api/core/workflow/nodes/trigger_schedule/exc.py b/api/core/workflow/nodes/trigger_schedule/exc.py new file mode 100644 index 00000000000000..6fb000de11fa73 --- /dev/null +++ b/api/core/workflow/nodes/trigger_schedule/exc.py @@ -0,0 +1,19 @@ +from core.workflow.nodes.base.exc import BaseNodeError + + +class ScheduleNodeError(BaseNodeError): + """Base schedule node error.""" + + pass + + +class ScheduleNotFoundError(ScheduleNodeError): + """Schedule not found error.""" + + pass + + +class ScheduleConfigError(ScheduleNodeError): + """Schedule configuration error.""" + + pass diff --git a/api/libs/schedule_utils.py b/api/libs/schedule_utils.py new file mode 100644 index 00000000000000..faf1fd7f5608ff --- /dev/null +++ b/api/libs/schedule_utils.py @@ -0,0 +1,89 @@ +from datetime import UTC, datetime +from typing import Optional + +import pytz +from croniter import croniter + + +def calculate_next_run_at( + cron_expression: str, + timezone: str, + base_time: Optional[datetime] = None, +) -> datetime: + """ + Calculate the next run time for a cron expression in a specific timezone. + + Args: + cron_expression: Cron expression string (supports croniter extensions like 'L') + timezone: Timezone string (e.g., 'UTC', 'America/New_York') + base_time: Base time to calculate from (defaults to current UTC time) + + Returns: + Next run time in UTC + + Note: + Supports croniter's extended syntax including: + - 'L' for last day of month + - Standard 5-field cron expressions + """ + + tz = pytz.timezone(timezone) + + if base_time is None: + base_time = datetime.now(UTC) + + base_time_tz = base_time.astimezone(tz) + cron = croniter(cron_expression, base_time_tz) + next_run_tz = cron.get_next(datetime) + next_run_utc = next_run_tz.astimezone(UTC) + + return next_run_utc + + +def convert_12h_to_24h(time_str: str) -> tuple[Optional[int], Optional[int]]: + """ + Parse 12-hour time format to 24-hour format for cron compatibility. + + Args: + time_str: Time string in format "HH:MM AM/PM" (e.g., "12:30 PM") + + Returns: + Tuple of (hour, minute) in 24-hour format, or (None, None) if parsing fails + + Examples: + - "12:00 AM" -> (0, 0) # Midnight + - "12:00 PM" -> (12, 0) # Noon + - "1:30 PM" -> (13, 30) + - "11:59 PM" -> (23, 59) + """ + try: + parts = time_str.strip().split() + if len(parts) != 2: + return None, None + + time_part, period = parts + period = period.upper() + + if period not in ["AM", "PM"]: + return None, None + + time_parts = time_part.split(":") + if len(time_parts) != 2: + return None, None + + hour = int(time_parts[0]) + minute = int(time_parts[1]) + + if hour < 1 or hour > 12 or minute < 0 or minute > 59: + return None, None + + # Handle 12-hour to 24-hour edge cases + if period == "PM" and hour != 12: + hour += 12 + elif period == "AM" and hour == 12: + hour = 0 + + return hour, minute + + except (ValueError, AttributeError): + return None, None From 3a3178a867df8ccf846f2ea72fcc4a7790351d1c Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 00:52:44 +0800 Subject: [PATCH 15/26] refactor: simplify workflow schedule processing and remove pre-check rate limit logic --- api/schedule/workflow_schedule_task.py | 113 ++++++++++--------------- 1 file changed, 47 insertions(+), 66 deletions(-) diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py index b8022f2844914b..b8b078baefbf46 100644 --- a/api/schedule/workflow_schedule_task.py +++ b/api/schedule/workflow_schedule_task.py @@ -7,8 +7,8 @@ from configs import dify_config from extensions.ext_database import db +from libs.schedule_utils import calculate_next_run_at from models.workflow import AppTrigger, AppTriggerStatus, WorkflowSchedulePlan -from services.schedule_service import ScheduleService from services.workflow.queue_dispatcher import QueueDispatcherManager from tasks.workflow_schedule_tasks import run_schedule_trigger @@ -20,98 +20,79 @@ def poll_workflow_schedules() -> None: """ Poll and process due workflow schedules. - Simple 3-step flow: - 1. Get rate-limited tenants from Redis - 2. Fetch due schedules excluding rate-limited tenants - 3. Process and dispatch valid schedules + Simple 2-step flow: + 1. Fetch due schedules + 2. Process valid schedules """ session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) with session_factory() as session: - rate_limited_tenants = _get_rate_limited_tenants(session) - due_schedules = _fetch_due_schedules(session, exclude_tenants=rate_limited_tenants) + due_schedules = _fetch_due_schedules(session) if due_schedules: - processed = _process_schedules(session, due_schedules) + dispatched_count, rate_limited_count = _process_schedules(session, due_schedules) logger.info( - "Processed %d/%d schedules (%d tenants rate-limited)", - processed, + "Processed %d/%d schedules (%d skipped due to rate limit)", + dispatched_count, len(due_schedules), - len(rate_limited_tenants), + rate_limited_count, ) -def _get_rate_limited_tenants(session: Session) -> set[str]: - """Get tenants that have reached their daily rate limit.""" +def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: + """Fetch all schedules that are due for execution.""" now = datetime.now(UTC) - tenant_ids = session.scalars( - select(WorkflowSchedulePlan.tenant_id) - .distinct() - .where( - WorkflowSchedulePlan.next_run_at <= now, - WorkflowSchedulePlan.next_run_at.isnot(None), - ) - ).all() - - if not tenant_ids: - return set() - - dispatcher_manager = QueueDispatcherManager() - return { - tenant_id - for tenant_id in tenant_ids - if not dispatcher_manager.get_dispatcher(tenant_id).check_daily_quota(tenant_id) - } - - -def _fetch_due_schedules(session: Session, exclude_tenants: set[str]) -> list[WorkflowSchedulePlan]: - """Fetch all schedules that are due for execution, excluding rate-limited tenants.""" - now = datetime.now(UTC) - - query = ( - select(WorkflowSchedulePlan) - .join( - AppTrigger, - and_( - AppTrigger.app_id == WorkflowSchedulePlan.app_id, - AppTrigger.node_id == WorkflowSchedulePlan.node_id, - AppTrigger.trigger_type == "trigger-schedule", - ), - ) - .where( - WorkflowSchedulePlan.next_run_at <= now, - WorkflowSchedulePlan.next_run_at.isnot(None), - AppTrigger.status == AppTriggerStatus.ENABLED, + due_schedules = session.scalars( + ( + select(WorkflowSchedulePlan) + .join( + AppTrigger, + and_( + AppTrigger.app_id == WorkflowSchedulePlan.app_id, + AppTrigger.node_id == WorkflowSchedulePlan.node_id, + AppTrigger.trigger_type == "trigger-schedule", + ), + ) + .where( + WorkflowSchedulePlan.next_run_at <= now, + WorkflowSchedulePlan.next_run_at.isnot(None), + AppTrigger.status == AppTriggerStatus.ENABLED, + ) ) + .with_for_update(skip_locked=True) + .limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE) ) - if exclude_tenants: - query = query.where(WorkflowSchedulePlan.tenant_id.notin_(exclude_tenants)) - - return list( - session.scalars(query.with_for_update(skip_locked=True).limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE)) - ) + return list(due_schedules) -def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> int: - """Process schedules: update next run time and dispatch to Celery.""" +def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> tuple[int, int]: + """Process schedules: check quota, update next run time and dispatch to Celery.""" if not schedules: - return 0 + return 0, 0 - dispatched = 0 + dispatched_count = 0 + rate_limited_count = 0 + dispatcher_manager = QueueDispatcherManager() for schedule in schedules: - next_run_at = ScheduleService.calculate_next_run_at( + next_run_at = calculate_next_run_at( schedule.cron_expression, schedule.timezone, ) - if next_run_at: - schedule.next_run_at = next_run_at + schedule.next_run_at = next_run_at + + dispatcher = dispatcher_manager.get_dispatcher(schedule.tenant_id) + + if not dispatcher.check_daily_quota(schedule.tenant_id): + logger.info("Tenant %s rate limited, skipping schedule_plan %s", schedule.tenant_id, schedule.id) + rate_limited_count += 1 + else: run_schedule_trigger.delay(schedule.id) - dispatched += 1 + dispatched_count += 1 session.commit() - return dispatched + return dispatched_count, rate_limited_count From 3e0414e6e03bd6e71d00c5a2bece89f00c60b609 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 00:54:20 +0800 Subject: [PATCH 16/26] refactor: convert def that do not reference ScheduleService to utils & use Pydantic.Basemodel handling schedule plan --- api/services/schedule_service.py | 202 ++++++++++++++----------------- 1 file changed, 92 insertions(+), 110 deletions(-) diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index adce511de27095..89e4468f5d81d7 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -1,14 +1,15 @@ import json import logging -from datetime import UTC, datetime +from datetime import datetime from typing import Optional -import pytz -from croniter import croniter from sqlalchemy import select from sqlalchemy.orm import Session from core.workflow.nodes import NodeType +from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate +from core.workflow.nodes.trigger_schedule.exc import ScheduleConfigError, ScheduleNotFoundError +from libs.schedule_utils import calculate_next_run_at, convert_12h_to_24h from models.account import Account, TenantAccountJoin from models.workflow import Workflow, WorkflowSchedulePlan @@ -16,46 +17,36 @@ class ScheduleService: - @staticmethod - def calculate_next_run_at( - cron_expression: str, - timezone: str, - base_time: Optional[datetime] = None, - ) -> Optional[datetime]: - try: - tz = pytz.timezone(timezone) - if base_time is None: - base_time = datetime.now(UTC) - base_time_tz = base_time.astimezone(tz) - cron = croniter(cron_expression, base_time_tz) - next_run_tz = cron.get_next(datetime) - next_run_utc = next_run_tz.astimezone(UTC) - return next_run_utc - except Exception: - logger.exception("Error calculating next run time") - return None - @staticmethod def create_schedule( session: Session, tenant_id: str, app_id: str, - node_id: str, - cron_expression: str, - timezone: str, + config: ScheduleConfig, ) -> WorkflowSchedulePlan: - # Pre-calculate next_run_at to ensure the schedule is valid before persisting - next_run_at = ScheduleService.calculate_next_run_at( - cron_expression, - timezone, + """ + Create a new schedule with validated configuration. + + Args: + session: Database session + tenant_id: Tenant ID + app_id: Application ID + config: Validated schedule configuration + + Returns: + Created WorkflowSchedulePlan instance + """ + next_run_at = calculate_next_run_at( + config.cron_expression, + config.timezone, ) schedule = WorkflowSchedulePlan( tenant_id=tenant_id, app_id=app_id, - node_id=node_id, - cron_expression=cron_expression, - timezone=timezone, + node_id=config.node_id, + cron_expression=config.cron_expression, + timezone=config.timezone, next_run_at=next_run_at, ) @@ -68,19 +59,31 @@ def create_schedule( def update_schedule( session: Session, schedule_id: str, - updates: dict, + updates: SchedulePlanUpdate, ) -> Optional[WorkflowSchedulePlan]: + """ + Update an existing schedule with validated configuration. + + Args: + session: Database session + schedule_id: Schedule ID to update + updates: Validated update configuration + + Returns: + Updated WorkflowSchedulePlan instance or None if not found + """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: - return None + raise ScheduleNotFoundError(f"Schedule not found: {schedule_id}") - for field, value in updates.items(): + update_dict = updates.model_dump(exclude_none=True) + for field, value in update_dict.items(): if hasattr(schedule, field): setattr(schedule, field, value) # Ensure next_run_at stays accurate when schedule timing changes - if any(field in updates for field in ["cron_expression", "timezone"]): - next_run_at = ScheduleService.calculate_next_run_at( + if "cron_expression" in update_dict or "timezone" in update_dict: + next_run_at = calculate_next_run_at( schedule.cron_expression, schedule.timezone, ) @@ -93,14 +96,20 @@ def update_schedule( def delete_schedule( session: Session, schedule_id: str, - ) -> bool: + ) -> None: + """ + Delete a schedule plan. + + Args: + session: Database session + schedule_id: Schedule ID to delete + """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: - return False + raise ScheduleNotFoundError(f"Schedule not found: {schedule_id}") session.delete(schedule) session.flush() - return True @staticmethod def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: @@ -138,10 +147,10 @@ def update_next_run_at( """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: - return None + raise ScheduleNotFoundError(f"Schedule not found: {schedule_id}") # Base on current time to handle execution delays gracefully - next_run_at = ScheduleService.calculate_next_run_at( + next_run_at = calculate_next_run_at( schedule.cron_expression, schedule.timezone, ) @@ -151,44 +160,55 @@ def update_next_run_at( return next_run_at @staticmethod - def extract_schedule_config(workflow: Workflow) -> Optional[dict]: + def extract_schedule_config(workflow: Workflow) -> Optional[ScheduleConfig]: """ - Extracts schedule configuration from workflow graph for synchronization. - Normalizes both visual and cron modes into a unified cron expression format. + Extracts schedule configuration from workflow graph. + + Searches for the first schedule trigger node in the workflow and converts + its configuration (either visual or cron mode) into a unified ScheduleConfig. + + Args: + workflow: The workflow containing the graph definition + + Returns: + ScheduleConfig if a valid schedule node is found, None otherwise + + Note: + Currently only returns the first schedule node found. + Multiple schedule nodes in the same workflow are not supported. """ try: graph_data = workflow.graph_dict except (json.JSONDecodeError, TypeError, AttributeError): + logger.exception("Failed to parse workflow graph") return None if not graph_data: return None - for node in graph_data.get("nodes", []): + nodes = graph_data.get("nodes", []) + for node in nodes: node_data = node.get("data", {}) - if node_data.get("type") == NodeType.TRIGGER_SCHEDULE.value: - mode = node_data.get("mode", "visual") - timezone = node_data.get("timezone", "UTC") - node_id = node.get("id", "start") - - # Normalize both modes to cron expression for consistent storage - cron_expression = None - if mode == "cron": - cron_expression = node_data.get("cron_expression") - elif mode == "visual": - cron_expression = ScheduleService.visual_to_cron( - node_data.get("frequency"), node_data.get("visual_config", {}) - ) - - if cron_expression: - return { - "node_id": node_id, - "cron_expression": cron_expression, - "timezone": timezone, - } - else: - logger.warning("Invalid schedule configuration in node %s", node_id) - return None + + if node_data.get("type") != NodeType.TRIGGER_SCHEDULE.value: + continue + + mode = node_data.get("mode", "visual") + timezone = node_data.get("timezone", "UTC") + node_id = node.get("id", "start") + + cron_expression = None + if mode == "cron": + cron_expression = node_data.get("cron_expression") + elif mode == "visual": + cron_expression = ScheduleService.visual_to_cron( + node_data.get("frequency"), node_data.get("visual_config", {}) + ) + + if cron_expression: + return ScheduleConfig(node_id=node_id, cron_expression=cron_expression, timezone=timezone) + else: + raise ScheduleConfigError(f"Invalid schedule configuration: {node_data}") return None @@ -208,14 +228,14 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: elif frequency == "daily": time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleService.parse_time(time_str) + hour, minute = convert_12h_to_24h(time_str) if hour is None or minute is None: return None return f"{minute} {hour} * * *" elif frequency == "weekly": time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleService.parse_time(time_str) + hour, minute = convert_12h_to_24h(time_str) if hour is None or minute is None: return None @@ -237,7 +257,7 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: elif frequency == "monthly": time_str = visual_config.get("time", "12:00 PM") - hour, minute = ScheduleService.parse_time(time_str) + hour, minute = convert_12h_to_24h(time_str) if hour is None or minute is None: return None @@ -273,41 +293,3 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: except Exception: return None - - @staticmethod - def parse_time(time_str: str) -> tuple[Optional[int], Optional[int]]: - """ - Parses 12-hour time format to 24-hour for cron compatibility. - Handles edge cases like 12:00 AM (midnight) and 12:00 PM (noon). - """ - try: - parts = time_str.strip().split() - if len(parts) != 2: - return None, None - - time_part, period = parts - period = period.upper() - - if period not in ["AM", "PM"]: - return None, None - - time_parts = time_part.split(":") - if len(time_parts) != 2: - return None, None - - hour = int(time_parts[0]) - minute = int(time_parts[1]) - - if hour < 1 or hour > 12 or minute < 0 or minute > 59: - return None, None - - # Handle 12-hour to 24-hour edge cases - if period == "PM" and hour != 12: - hour += 12 - elif period == "AM" and hour == 12: - hour = 0 - - return hour, minute - - except (ValueError, AttributeError): - return None, None From 04994d676a78d2973fd22903fcd61b1d491623c3 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 00:55:33 +0800 Subject: [PATCH 17/26] chore: update schedule_service unit_tests --- .../services/test_schedule_service.py | 169 ++++++++++-------- 1 file changed, 97 insertions(+), 72 deletions(-) diff --git a/api/tests/unit_tests/services/test_schedule_service.py b/api/tests/unit_tests/services/test_schedule_service.py index 5569b3700c6006..27ddc747a87e1f 100644 --- a/api/tests/unit_tests/services/test_schedule_service.py +++ b/api/tests/unit_tests/services/test_schedule_service.py @@ -2,11 +2,14 @@ from datetime import UTC, datetime from unittest.mock import MagicMock, Mock, patch +import pytest from sqlalchemy.orm import Session +from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate from events.event_handlers.sync_workflow_schedule_when_app_published import ( sync_schedule_from_workflow, ) +from libs.schedule_utils import calculate_next_run_at, convert_12h_to_24h from models.account import Account, TenantAccountJoin from models.workflow import Workflow, WorkflowSchedulePlan from services.schedule_service import ScheduleService @@ -22,7 +25,7 @@ def test_calculate_next_run_at_valid_cron(self): timezone = "UTC" base_time = datetime(2025, 8, 29, 9, 0, 0, tzinfo=UTC) - next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + next_run = calculate_next_run_at(cron_expr, timezone, base_time) assert next_run is not None assert next_run.hour == 10 @@ -35,7 +38,7 @@ def test_calculate_next_run_at_with_timezone(self): timezone = "America/New_York" base_time = datetime(2025, 8, 29, 12, 0, 0, tzinfo=UTC) # 8:00 AM EDT - next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + next_run = calculate_next_run_at(cron_expr, timezone, base_time) assert next_run is not None # 9:00 AM EDT = 13:00 UTC (during EDT) @@ -48,7 +51,7 @@ def test_calculate_next_run_at_with_last_day_of_month(self): timezone = "UTC" base_time = datetime(2025, 2, 15, 9, 0, 0, tzinfo=UTC) - next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone, base_time) + next_run = calculate_next_run_at(cron_expr, timezone, base_time) assert next_run is not None # February 2025 has 28 days @@ -57,35 +60,41 @@ def test_calculate_next_run_at_with_last_day_of_month(self): def test_calculate_next_run_at_invalid_cron(self): """Test calculating next run time with invalid cron expression.""" + from croniter import CroniterBadCronError + cron_expr = "invalid cron" timezone = "UTC" - next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone) - - assert next_run is None + with pytest.raises(CroniterBadCronError): + calculate_next_run_at(cron_expr, timezone) def test_calculate_next_run_at_invalid_timezone(self): """Test calculating next run time with invalid timezone.""" + from pytz import UnknownTimeZoneError + cron_expr = "30 10 * * *" timezone = "Invalid/Timezone" - next_run = ScheduleService.calculate_next_run_at(cron_expr, timezone) - - assert next_run is None + with pytest.raises(UnknownTimeZoneError): + calculate_next_run_at(cron_expr, timezone) - @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + @patch("libs.schedule_utils.calculate_next_run_at") def test_create_schedule(self, mock_calculate_next_run): """Test creating a new schedule.""" mock_session = MagicMock(spec=Session) mock_calculate_next_run.return_value = datetime(2025, 8, 30, 10, 30, 0, tzinfo=UTC) + config = ScheduleConfig( + node_id="start", + cron_expression="30 10 * * *", + timezone="UTC", + ) + schedule = ScheduleService.create_schedule( session=mock_session, tenant_id="test-tenant", app_id="test-app", - node_id="start", - cron_expression="30 10 * * *", - timezone="UTC", + config=config, ) assert schedule is not None @@ -98,20 +107,20 @@ def test_create_schedule(self, mock_calculate_next_run): mock_session.add.assert_called_once() mock_session.flush.assert_called_once() - @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + @patch("services.schedule_service.calculate_next_run_at") def test_update_schedule(self, mock_calculate_next_run): """Test updating an existing schedule.""" mock_session = MagicMock(spec=Session) mock_schedule = Mock(spec=WorkflowSchedulePlan) mock_schedule.cron_expression = "0 12 * * *" - mock_schedule.timezone = "UTC" + mock_schedule.timezone = "America/New_York" mock_session.get.return_value = mock_schedule mock_calculate_next_run.return_value = datetime(2025, 8, 30, 12, 0, 0, tzinfo=UTC) - updates = { - "cron_expression": "0 12 * * *", - "timezone": "America/New_York", - } + updates = SchedulePlanUpdate( + cron_expression="0 12 * * *", + timezone="America/New_York", + ) result = ScheduleService.update_schedule( session=mock_session, @@ -126,17 +135,24 @@ def test_update_schedule(self, mock_calculate_next_run): mock_session.flush.assert_called_once() def test_update_schedule_not_found(self): - """Test updating a non-existent schedule.""" + """Test updating a non-existent schedule raises exception.""" + from core.workflow.nodes.trigger_schedule.exc import ScheduleNotFoundError + mock_session = MagicMock(spec=Session) mock_session.get.return_value = None - result = ScheduleService.update_schedule( - session=mock_session, - schedule_id="non-existent-id", - updates={"cron_expression": "0 12 * * *"}, + updates = SchedulePlanUpdate( + cron_expression="0 12 * * *", ) - assert result is None + with pytest.raises(ScheduleNotFoundError) as context: + ScheduleService.update_schedule( + session=mock_session, + schedule_id="non-existent-id", + updates=updates, + ) + + assert "Schedule not found: non-existent-id" in str(context.value) mock_session.flush.assert_not_called() def test_delete_schedule(self): @@ -145,26 +161,30 @@ def test_delete_schedule(self): mock_schedule = Mock(spec=WorkflowSchedulePlan) mock_session.get.return_value = mock_schedule - result = ScheduleService.delete_schedule( + # Should not raise exception and complete successfully + ScheduleService.delete_schedule( session=mock_session, schedule_id="test-schedule-id", ) - assert result mock_session.delete.assert_called_once_with(mock_schedule) mock_session.flush.assert_called_once() def test_delete_schedule_not_found(self): - """Test deleting a non-existent schedule.""" + """Test deleting a non-existent schedule raises exception.""" + from core.workflow.nodes.trigger_schedule.exc import ScheduleNotFoundError + mock_session = MagicMock(spec=Session) mock_session.get.return_value = None - result = ScheduleService.delete_schedule( - session=mock_session, - schedule_id="non-existent-id", - ) + # Should raise ScheduleNotFoundError + with pytest.raises(ScheduleNotFoundError) as context: + ScheduleService.delete_schedule( + session=mock_session, + schedule_id="non-existent-id", + ) - assert not result + assert "Schedule not found: non-existent-id" in str(context.value) mock_session.delete.assert_not_called() @patch("services.schedule_service.select") @@ -211,7 +231,7 @@ def test_get_tenant_owner_fallback_to_admin(self, mock_select): assert result is not None assert result.id == "admin-account-id" - @patch("services.schedule_service.ScheduleService.calculate_next_run_at") + @patch("services.schedule_service.calculate_next_run_at") def test_update_next_run_at(self, mock_calculate_next_run): """Test updating next run time after schedule triggered.""" mock_session = MagicMock(spec=Session) @@ -311,43 +331,43 @@ class TestParseTime(unittest.TestCase): def test_parse_time_am(self): """Test parsing AM time.""" - hour, minute = ScheduleService.parse_time("9:30 AM") + hour, minute = convert_12h_to_24h("9:30 AM") assert hour == 9 assert minute == 30 def test_parse_time_pm(self): """Test parsing PM time.""" - hour, minute = ScheduleService.parse_time("2:45 PM") + hour, minute = convert_12h_to_24h("2:45 PM") assert hour == 14 assert minute == 45 def test_parse_time_noon(self): """Test parsing 12:00 PM (noon).""" - hour, minute = ScheduleService.parse_time("12:00 PM") + hour, minute = convert_12h_to_24h("12:00 PM") assert hour == 12 assert minute == 0 def test_parse_time_midnight(self): """Test parsing 12:00 AM (midnight).""" - hour, minute = ScheduleService.parse_time("12:00 AM") + hour, minute = convert_12h_to_24h("12:00 AM") assert hour == 0 assert minute == 0 def test_parse_time_invalid_format(self): """Test parsing invalid time format.""" - hour, minute = ScheduleService.parse_time("25:00") + hour, minute = convert_12h_to_24h("25:00") assert hour is None assert minute is None def test_parse_time_invalid_hour(self): """Test parsing invalid hour.""" - hour, minute = ScheduleService.parse_time("13:00 PM") + hour, minute = convert_12h_to_24h("13:00 PM") assert hour is None assert minute is None def test_parse_time_invalid_minute(self): """Test parsing invalid minute.""" - hour, minute = ScheduleService.parse_time("10:60 AM") + hour, minute = convert_12h_to_24h("10:60 AM") assert hour is None assert minute is None @@ -375,9 +395,9 @@ def test_extract_schedule_config_with_cron_mode(self): config = ScheduleService.extract_schedule_config(workflow) assert config is not None - assert config["node_id"] == "schedule-node" - assert config["cron_expression"] == "0 10 * * *" - assert config["timezone"] == "America/New_York" + assert config.node_id == "schedule-node" + assert config.cron_expression == "0 10 * * *" + assert config.timezone == "America/New_York" def test_extract_schedule_config_with_visual_mode(self): """Test extracting schedule config in visual mode.""" @@ -400,9 +420,9 @@ def test_extract_schedule_config_with_visual_mode(self): config = ScheduleService.extract_schedule_config(workflow) assert config is not None - assert config["node_id"] == "schedule-node" - assert config["cron_expression"] == "30 10 * * *" - assert config["timezone"] == "UTC" + assert config.node_id == "schedule-node" + assert config.cron_expression == "30 10 * * *" + assert config.timezone == "UTC" def test_extract_schedule_config_no_schedule_node(self): """Test extracting config when no schedule node exists.""" @@ -454,7 +474,7 @@ def test_visual_schedule_with_timezone_integration(self): # Base time: 2025-01-01 00:00:00 UTC (08:00:00 Shanghai) base_time = datetime(2025, 1, 1, 0, 0, 0, tzinfo=UTC) - next_run = ScheduleService.calculate_next_run_at(cron_expr, shanghai_tz, base_time) + next_run = calculate_next_run_at(cron_expr, shanghai_tz, base_time) assert next_run is not None @@ -485,14 +505,14 @@ def test_visual_schedule_different_timezones_same_local_time(self): base_time = datetime(2025, 1, 5, 12, 0, 0, tzinfo=UTC) # Test New York (UTC-5 in January) - ny_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", base_time) + ny_next = calculate_next_run_at(cron_expr, "America/New_York", base_time) assert ny_next is not None # Monday 9 AM EST = Monday 14:00 UTC assert ny_next.day == 6 assert ny_next.hour == 14 # 9 AM EST = 2 PM UTC # Test Tokyo (UTC+9) - tokyo_next = ScheduleService.calculate_next_run_at(cron_expr, "Asia/Tokyo", base_time) + tokyo_next = calculate_next_run_at(cron_expr, "Asia/Tokyo", base_time) assert tokyo_next is not None # Monday 9 AM JST = Monday 00:00 UTC assert tokyo_next.day == 6 @@ -515,14 +535,14 @@ def test_visual_schedule_daily_across_dst_change(self): # Test before DST (EST - UTC-5) winter_base = datetime(2025, 2, 1, 0, 0, 0, tzinfo=UTC) - winter_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", winter_base) + winter_next = calculate_next_run_at(cron_expr, "America/New_York", winter_base) assert winter_next is not None # 10 AM EST = 15:00 UTC assert winter_next.hour == 15 # Test during DST (EDT - UTC-4) summer_base = datetime(2025, 6, 1, 0, 0, 0, tzinfo=UTC) - summer_next = ScheduleService.calculate_next_run_at(cron_expr, "America/New_York", summer_base) + summer_next = calculate_next_run_at(cron_expr, "America/New_York", summer_base) assert summer_next is not None # 10 AM EDT = 14:00 UTC assert summer_next.hour == 14 @@ -544,11 +564,12 @@ def test_sync_schedule_create_new(self, mock_select, mock_service, mock_db): with patch("events.event_handlers.sync_workflow_schedule_when_app_published.Session", Session): mock_session.scalar.return_value = None # No existing plan - mock_service.extract_schedule_config.return_value = { - "node_id": "start", - "cron_expression": "30 10 * * *", - "timezone": "UTC", - } + # Mock extract_schedule_config to return a ScheduleConfig object + mock_config = Mock(spec=ScheduleConfig) + mock_config.node_id = "start" + mock_config.cron_expression = "30 10 * * *" + mock_config.timezone = "UTC" + mock_service.extract_schedule_config.return_value = mock_config mock_new_plan = Mock(spec=WorkflowSchedulePlan) mock_service.create_schedule.return_value = mock_new_plan @@ -576,11 +597,12 @@ def test_sync_schedule_update_existing(self, mock_select, mock_service, mock_db) mock_existing_plan.id = "existing-plan-id" mock_session.scalar.return_value = mock_existing_plan - mock_service.extract_schedule_config.return_value = { - "node_id": "start", - "cron_expression": "0 12 * * *", - "timezone": "America/New_York", - } + # Mock extract_schedule_config to return a ScheduleConfig object + mock_config = Mock(spec=ScheduleConfig) + mock_config.node_id = "start" + mock_config.cron_expression = "0 12 * * *" + mock_config.timezone = "America/New_York" + mock_service.extract_schedule_config.return_value = mock_config mock_updated_plan = Mock(spec=WorkflowSchedulePlan) mock_service.update_schedule.return_value = mock_updated_plan @@ -589,15 +611,16 @@ def test_sync_schedule_update_existing(self, mock_select, mock_service, mock_db) result = sync_schedule_from_workflow("tenant-id", "app-id", workflow) assert result == mock_updated_plan - mock_service.update_schedule.assert_called_once_with( - session=mock_session, - schedule_id="existing-plan-id", - updates={ - "node_id": "start", - "cron_expression": "0 12 * * *", - "timezone": "America/New_York", - }, - ) + mock_service.update_schedule.assert_called_once() + # Verify the arguments passed to update_schedule + call_args = mock_service.update_schedule.call_args + assert call_args.kwargs["session"] == mock_session + assert call_args.kwargs["schedule_id"] == "existing-plan-id" + updates_obj = call_args.kwargs["updates"] + assert isinstance(updates_obj, SchedulePlanUpdate) + assert updates_obj.node_id == "start" + assert updates_obj.cron_expression == "0 12 * * *" + assert updates_obj.timezone == "America/New_York" mock_session.commit.assert_called_once() @patch("events.event_handlers.sync_workflow_schedule_when_app_published.db") @@ -613,6 +636,7 @@ def test_sync_schedule_remove_when_no_config(self, mock_select, mock_service, mo with patch("events.event_handlers.sync_workflow_schedule_when_app_published.Session", Session): mock_existing_plan = Mock(spec=WorkflowSchedulePlan) + mock_existing_plan.id = "existing-plan-id" mock_session.scalar.return_value = mock_existing_plan mock_service.extract_schedule_config.return_value = None # No schedule config @@ -621,7 +645,8 @@ def test_sync_schedule_remove_when_no_config(self, mock_select, mock_service, mo result = sync_schedule_from_workflow("tenant-id", "app-id", workflow) assert result is None - mock_session.delete.assert_called_once_with(mock_existing_plan) + # Now using ScheduleService.delete_schedule instead of session.delete + mock_service.delete_schedule.assert_called_once_with(session=mock_session, schedule_id="existing-plan-id") mock_session.commit.assert_called_once() From 38ffd95ab03038ff60a880248803ace502e217cf Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Wed, 3 Sep 2025 01:18:15 +0800 Subject: [PATCH 18/26] refactor: update convert_12h_to_24h to raise ValueErrors for invalid inputs --- api/fields/workflow_run_fields.py | 1 - api/libs/schedule_utils.py | 54 +++++++++++-------- api/services/schedule_service.py | 6 --- .../services/test_schedule_service.py | 25 +++++---- 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/api/fields/workflow_run_fields.py b/api/fields/workflow_run_fields.py index 6bbb8b5a0360b6..8d20bfaad4db1c 100644 --- a/api/fields/workflow_run_fields.py +++ b/api/fields/workflow_run_fields.py @@ -16,7 +16,6 @@ "created_at": TimestampField, "finished_at": TimestampField, "exceptions_count": fields.Integer, - "triggered_from": fields.String, } diff --git a/api/libs/schedule_utils.py b/api/libs/schedule_utils.py index faf1fd7f5608ff..635550dd7fc28b 100644 --- a/api/libs/schedule_utils.py +++ b/api/libs/schedule_utils.py @@ -40,7 +40,7 @@ def calculate_next_run_at( return next_run_utc -def convert_12h_to_24h(time_str: str) -> tuple[Optional[int], Optional[int]]: +def convert_12h_to_24h(time_str: str) -> tuple[int, int]: """ Parse 12-hour time format to 24-hour format for cron compatibility. @@ -48,7 +48,10 @@ def convert_12h_to_24h(time_str: str) -> tuple[Optional[int], Optional[int]]: time_str: Time string in format "HH:MM AM/PM" (e.g., "12:30 PM") Returns: - Tuple of (hour, minute) in 24-hour format, or (None, None) if parsing fails + Tuple of (hour, minute) in 24-hour format + + Raises: + ValueError: If time string format is invalid or values are out of range Examples: - "12:00 AM" -> (0, 0) # Midnight @@ -56,34 +59,39 @@ def convert_12h_to_24h(time_str: str) -> tuple[Optional[int], Optional[int]]: - "1:30 PM" -> (13, 30) - "11:59 PM" -> (23, 59) """ - try: - parts = time_str.strip().split() - if len(parts) != 2: - return None, None + if not time_str or not time_str.strip(): + raise ValueError("Time string cannot be empty") - time_part, period = parts - period = period.upper() + parts = time_str.strip().split() + if len(parts) != 2: + raise ValueError(f"Invalid time format: '{time_str}'. Expected 'HH:MM AM/PM'") - if period not in ["AM", "PM"]: - return None, None + time_part, period = parts + period = period.upper() - time_parts = time_part.split(":") - if len(time_parts) != 2: - return None, None + if period not in ["AM", "PM"]: + raise ValueError(f"Invalid period: '{period}'. Must be 'AM' or 'PM'") + time_parts = time_part.split(":") + if len(time_parts) != 2: + raise ValueError(f"Invalid time format: '{time_part}'. Expected 'HH:MM'") + + try: hour = int(time_parts[0]) minute = int(time_parts[1]) + except ValueError as e: + raise ValueError(f"Invalid time values: {e}") - if hour < 1 or hour > 12 or minute < 0 or minute > 59: - return None, None + if hour < 1 or hour > 12: + raise ValueError(f"Invalid hour: {hour}. Must be between 1 and 12") - # Handle 12-hour to 24-hour edge cases - if period == "PM" and hour != 12: - hour += 12 - elif period == "AM" and hour == 12: - hour = 0 + if minute < 0 or minute > 59: + raise ValueError(f"Invalid minute: {minute}. Must be between 0 and 59") - return hour, minute + # Handle 12-hour to 24-hour edge cases + if period == "PM" and hour != 12: + hour += 12 + elif period == "AM" and hour == 12: + hour = 0 - except (ValueError, AttributeError): - return None, None + return hour, minute diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 89e4468f5d81d7..bdc74d9272180d 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -229,15 +229,11 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: elif frequency == "daily": time_str = visual_config.get("time", "12:00 PM") hour, minute = convert_12h_to_24h(time_str) - if hour is None or minute is None: - return None return f"{minute} {hour} * * *" elif frequency == "weekly": time_str = visual_config.get("time", "12:00 PM") hour, minute = convert_12h_to_24h(time_str) - if hour is None or minute is None: - return None weekdays = visual_config.get("weekdays", []) if not weekdays: @@ -258,8 +254,6 @@ def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: elif frequency == "monthly": time_str = visual_config.get("time", "12:00 PM") hour, minute = convert_12h_to_24h(time_str) - if hour is None or minute is None: - return None monthly_days = visual_config.get("monthly_days", []) if not monthly_days: diff --git a/api/tests/unit_tests/services/test_schedule_service.py b/api/tests/unit_tests/services/test_schedule_service.py index 27ddc747a87e1f..cc6b4428ef0fe3 100644 --- a/api/tests/unit_tests/services/test_schedule_service.py +++ b/api/tests/unit_tests/services/test_schedule_service.py @@ -355,21 +355,28 @@ def test_parse_time_midnight(self): def test_parse_time_invalid_format(self): """Test parsing invalid time format.""" - hour, minute = convert_12h_to_24h("25:00") - assert hour is None - assert minute is None + with pytest.raises(ValueError, match="Invalid time format"): + convert_12h_to_24h("25:00") def test_parse_time_invalid_hour(self): """Test parsing invalid hour.""" - hour, minute = convert_12h_to_24h("13:00 PM") - assert hour is None - assert minute is None + with pytest.raises(ValueError, match="Invalid hour: 13"): + convert_12h_to_24h("13:00 PM") def test_parse_time_invalid_minute(self): """Test parsing invalid minute.""" - hour, minute = convert_12h_to_24h("10:60 AM") - assert hour is None - assert minute is None + with pytest.raises(ValueError, match="Invalid minute: 60"): + convert_12h_to_24h("10:60 AM") + + def test_parse_time_empty_string(self): + """Test parsing empty string.""" + with pytest.raises(ValueError, match="Time string cannot be empty"): + convert_12h_to_24h("") + + def test_parse_time_invalid_period(self): + """Test parsing invalid period.""" + with pytest.raises(ValueError, match="Invalid period"): + convert_12h_to_24h("10:30 XM") class TestExtractScheduleConfig(unittest.TestCase): From a7fbc9ff22578e4e6625bbf4a2f087fffdb864af Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 01:44:51 +0800 Subject: [PATCH 19/26] feat: add VisualConfig & ScheduleExecutionError class --- .../nodes/trigger_schedule/entities.py | 22 ++++++++++++++++++- .../workflow/nodes/trigger_schedule/exc.py | 12 ++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/api/core/workflow/nodes/trigger_schedule/entities.py b/api/core/workflow/nodes/trigger_schedule/entities.py index 0881c55e1449fb..0fea4dcda359ab 100644 --- a/api/core/workflow/nodes/trigger_schedule/entities.py +++ b/api/core/workflow/nodes/trigger_schedule/entities.py @@ -1,4 +1,4 @@ -from typing import Optional +from typing import Literal, Optional, Union from pydantic import BaseModel, Field @@ -29,3 +29,23 @@ class SchedulePlanUpdate(BaseModel): node_id: Optional[str] = None cron_expression: Optional[str] = None timezone: Optional[str] = None + + +class VisualConfig(BaseModel): + """Visual configuration for schedule trigger""" + + # For hourly frequency + on_minute: Optional[int] = Field(default=0, ge=0, le=59, description="Minute of the hour (0-59)") + + # For daily, weekly, monthly frequencies + time: Optional[str] = Field(default="12:00 PM", description="Time in 12-hour format (e.g., '2:30 PM')") + + # For weekly frequency + weekdays: Optional[list[Literal["sun", "mon", "tue", "wed", "thu", "fri", "sat"]]] = Field( + default=None, description="List of weekdays to run on" + ) + + # For monthly frequency + monthly_days: Optional[list[Union[int, Literal["last"]]]] = Field( + default=None, description="Days of month to run on (1-31 or 'last')" + ) diff --git a/api/core/workflow/nodes/trigger_schedule/exc.py b/api/core/workflow/nodes/trigger_schedule/exc.py index 6fb000de11fa73..2f99880ff138a6 100644 --- a/api/core/workflow/nodes/trigger_schedule/exc.py +++ b/api/core/workflow/nodes/trigger_schedule/exc.py @@ -17,3 +17,15 @@ class ScheduleConfigError(ScheduleNodeError): """Schedule configuration error.""" pass + + +class ScheduleExecutionError(ScheduleNodeError): + """Schedule execution error.""" + + pass + + +class TenantOwnerNotFoundError(ScheduleExecutionError): + """Tenant owner not found error for schedule execution.""" + + pass From 80b28e71d620de3bf1d89d111c84ad86d4638b06 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 01:46:14 +0800 Subject: [PATCH 20/26] chore: replace datetime.now() with naive_utc_now() --- api/schedule/workflow_schedule_task.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py index b8b078baefbf46..e03bac76e5d915 100644 --- a/api/schedule/workflow_schedule_task.py +++ b/api/schedule/workflow_schedule_task.py @@ -1,5 +1,4 @@ import logging -from datetime import UTC, datetime from celery import shared_task from sqlalchemy import and_, select @@ -7,6 +6,7 @@ from configs import dify_config from extensions.ext_database import db +from libs.datetime_utils import naive_utc_now from libs.schedule_utils import calculate_next_run_at from models.workflow import AppTrigger, AppTriggerStatus, WorkflowSchedulePlan from services.workflow.queue_dispatcher import QueueDispatcherManager @@ -41,7 +41,7 @@ def poll_workflow_schedules() -> None: def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: """Fetch all schedules that are due for execution.""" - now = datetime.now(UTC) + now = naive_utc_now() due_schedules = session.scalars( ( From 42fa1c43ad47e964c25fb7feb7b15e8b93825c0a Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 01:47:02 +0800 Subject: [PATCH 21/26] chore: enhance schedule update logic and error handling --- api/services/schedule_service.py | 169 ++++++++++++++----------------- 1 file changed, 75 insertions(+), 94 deletions(-) diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index bdc74d9272180d..478eeedf04fef2 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -7,7 +7,7 @@ from sqlalchemy.orm import Session from core.workflow.nodes import NodeType -from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate +from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate, VisualConfig from core.workflow.nodes.trigger_schedule.exc import ScheduleConfigError, ScheduleNotFoundError from libs.schedule_utils import calculate_next_run_at, convert_12h_to_24h from models.account import Account, TenantAccountJoin @@ -60,7 +60,7 @@ def update_schedule( session: Session, schedule_id: str, updates: SchedulePlanUpdate, - ) -> Optional[WorkflowSchedulePlan]: + ) -> WorkflowSchedulePlan: """ Update an existing schedule with validated configuration. @@ -69,25 +69,35 @@ def update_schedule( schedule_id: Schedule ID to update updates: Validated update configuration + Raises: + ScheduleNotFoundError: If schedule not found + Returns: - Updated WorkflowSchedulePlan instance or None if not found + Updated WorkflowSchedulePlan instance """ schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: raise ScheduleNotFoundError(f"Schedule not found: {schedule_id}") - update_dict = updates.model_dump(exclude_none=True) - for field, value in update_dict.items(): - if hasattr(schedule, field): - setattr(schedule, field, value) + # If time-related fields are updated, synchronously update the next_run_at. + time_fields_updated = False + + if updates.node_id is not None: + schedule.node_id = updates.node_id + + if updates.cron_expression is not None: + schedule.cron_expression = updates.cron_expression + time_fields_updated = True - # Ensure next_run_at stays accurate when schedule timing changes - if "cron_expression" in update_dict or "timezone" in update_dict: - next_run_at = calculate_next_run_at( + if updates.timezone is not None: + schedule.timezone = updates.timezone + time_fields_updated = True + + if time_fields_updated: + schedule.next_run_at = calculate_next_run_at( schedule.cron_expression, schedule.timezone, ) - schedule.next_run_at = next_run_at session.flush() return schedule @@ -140,7 +150,7 @@ def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: def update_next_run_at( session: Session, schedule_id: str, - ) -> Optional[datetime]: + ) -> datetime: """ Advances the schedule to its next execution time after a successful trigger. Uses current time as base to prevent missing executions during delays. @@ -171,7 +181,10 @@ def extract_schedule_config(workflow: Workflow) -> Optional[ScheduleConfig]: workflow: The workflow containing the graph definition Returns: - ScheduleConfig if a valid schedule node is found, None otherwise + ScheduleConfig if a valid schedule node is found, None if no schedule node exists + + Raises: + ScheduleConfigError: If graph parsing fails or schedule configuration is invalid Note: Currently only returns the first schedule node found. @@ -179,12 +192,11 @@ def extract_schedule_config(workflow: Workflow) -> Optional[ScheduleConfig]: """ try: graph_data = workflow.graph_dict - except (json.JSONDecodeError, TypeError, AttributeError): - logger.exception("Failed to parse workflow graph") - return None + except (json.JSONDecodeError, TypeError, AttributeError) as e: + raise ScheduleConfigError(f"Failed to parse workflow graph: {e}") if not graph_data: - return None + raise ScheduleConfigError("Workflow graph is empty") nodes = graph_data.get("nodes", []) for node in nodes: @@ -200,90 +212,59 @@ def extract_schedule_config(workflow: Workflow) -> Optional[ScheduleConfig]: cron_expression = None if mode == "cron": cron_expression = node_data.get("cron_expression") + if not cron_expression: + raise ScheduleConfigError("Cron expression is required for cron mode") elif mode == "visual": - cron_expression = ScheduleService.visual_to_cron( - node_data.get("frequency"), node_data.get("visual_config", {}) - ) - - if cron_expression: - return ScheduleConfig(node_id=node_id, cron_expression=cron_expression, timezone=timezone) + frequency = node_data.get("frequency") + visual_config_dict = node_data.get("visual_config", {}) + visual_config = VisualConfig(**visual_config_dict) + cron_expression = ScheduleService.visual_to_cron(frequency, visual_config) else: - raise ScheduleConfigError(f"Invalid schedule configuration: {node_data}") + raise ScheduleConfigError(f"Invalid schedule mode: {mode}") + + return ScheduleConfig(node_id=node_id, cron_expression=cron_expression, timezone=timezone) return None @staticmethod - def visual_to_cron(frequency: str, visual_config: dict) -> Optional[str]: + def visual_to_cron(frequency: str, visual_config: VisualConfig) -> str: """ Converts user-friendly visual schedule settings to cron expression. Maintains consistency with frontend UI expectations while supporting croniter's extended syntax. """ - if not frequency or not visual_config: - return None - - try: - if frequency == "hourly": - on_minute = visual_config.get("on_minute", 0) - return f"{on_minute} * * * *" - - elif frequency == "daily": - time_str = visual_config.get("time", "12:00 PM") - hour, minute = convert_12h_to_24h(time_str) - return f"{minute} {hour} * * *" - - elif frequency == "weekly": - time_str = visual_config.get("time", "12:00 PM") - hour, minute = convert_12h_to_24h(time_str) - - weekdays = visual_config.get("weekdays", []) - if not weekdays: - return None - - # Map to cron's 0-6 format where 0=Sunday for standard cron compatibility - weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} - cron_weekdays = [] - for day in weekdays: - if day in weekday_map: - cron_weekdays.append(weekday_map[day]) - - if not cron_weekdays: - return None - - return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" - - elif frequency == "monthly": - time_str = visual_config.get("time", "12:00 PM") - hour, minute = convert_12h_to_24h(time_str) - - monthly_days = visual_config.get("monthly_days", []) - if not monthly_days: - return None - - cron_days = [] - for day in monthly_days: - if day == "last": - # croniter supports 'L' for last day of month, avoiding hardcoded day ranges - cron_days.append("L") - elif isinstance(day, int) and 1 <= day <= 31: - cron_days.append(str(day)) - - if not cron_days: - return None - - # Keep 'L' at end for readability while sorting numeric days - numeric_days = [d for d in cron_days if d != "L"] - has_last = "L" in cron_days - - sorted_days = [] - if numeric_days: - sorted_days = sorted(set(numeric_days), key=int) - if has_last: - sorted_days.append("L") - - return f"{minute} {hour} {','.join(sorted_days)} * *" - - else: - return None - - except Exception: - return None + if frequency == "hourly": + return f"{visual_config.on_minute} * * * *" + + elif frequency == "daily": + hour, minute = convert_12h_to_24h(visual_config.time) + return f"{minute} {hour} * * *" + + elif frequency == "weekly": + hour, minute = convert_12h_to_24h(visual_config.time) + weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} + cron_weekdays = [weekday_map[day] for day in visual_config.weekdays] + return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" + + elif frequency == "monthly": + hour, minute = convert_12h_to_24h(visual_config.time) + + cron_days = [] + for day in visual_config.monthly_days: + if day == "last": + cron_days.append("L") + else: + cron_days.append(str(day)) + + numeric_days = [d for d in cron_days if d != "L"] + has_last = "L" in cron_days + + sorted_days = [] + if numeric_days: + sorted_days = sorted(set(numeric_days), key=int) + if has_last: + sorted_days.append("L") + + return f"{minute} {hour} {','.join(sorted_days)} * *" + + else: + raise ScheduleConfigError(f"Unsupported frequency: {frequency}") From bce26745b7693abca0bff5c258de53eea9f3de4e Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 01:48:01 +0800 Subject: [PATCH 22/26] chore: improve error handling and logging in run_schedule_trigger func --- api/tasks/workflow_schedule_tasks.py | 34 +++++++++++++++------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/api/tasks/workflow_schedule_tasks.py b/api/tasks/workflow_schedule_tasks.py index 412841b0f093d9..e2eeee86cc3250 100644 --- a/api/tasks/workflow_schedule_tasks.py +++ b/api/tasks/workflow_schedule_tasks.py @@ -5,36 +5,44 @@ from celery import shared_task from sqlalchemy.orm import sessionmaker +from core.workflow.nodes.trigger_schedule.exc import ( + ScheduleExecutionError, + ScheduleNotFoundError, + TenantOwnerNotFoundError, +) from extensions.ext_database import db from models.enums import WorkflowRunTriggeredFrom from models.workflow import WorkflowSchedulePlan from services.async_workflow_service import AsyncWorkflowService from services.schedule_service import ScheduleService -from services.workflow.entities import AsyncTriggerResponse, TriggerData +from services.workflow.entities import TriggerData logger = logging.getLogger(__name__) @shared_task(queue="schedule") -def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: +def run_schedule_trigger(schedule_id: str) -> None: """ Execute a scheduled workflow trigger. Note: No retry logic needed as schedules will run again at next interval. - Failed executions are logged but don't block future runs. + The execution result is tracked via WorkflowTriggerLog. + + Raises: + ScheduleNotFoundError: If schedule doesn't exist + TenantOwnerNotFoundError: If no owner/admin for tenant + ScheduleExecutionError: If workflow trigger fails """ session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) with session_factory() as session: schedule = session.get(WorkflowSchedulePlan, schedule_id) if not schedule: - logger.warning("Schedule %s not found", schedule_id) - return + raise ScheduleNotFoundError(f"Schedule {schedule_id} not found") tenant_owner = ScheduleService.get_tenant_owner(session, schedule.tenant_id) if not tenant_owner: - logger.error("Tenant owner not found for tenant %s", schedule.tenant_id) - return + raise TenantOwnerNotFoundError(f"No owner or admin found for tenant {schedule.tenant_id}") try: current_utc = datetime.now(UTC) @@ -54,14 +62,8 @@ def run_schedule_trigger(schedule_id: str) -> AsyncTriggerResponse | None: ), ) logger.info("Schedule %s triggered workflow: %s", schedule_id, response.workflow_trigger_log_id) - return response except Exception as e: - logger.error( - "Failed to trigger workflow for schedule %s. App: %s, Error: %s", - schedule_id, - schedule.app_id, - str(e), - exc_info=True, - ) - return None + raise ScheduleExecutionError( + f"Failed to trigger workflow for schedule {schedule_id}, app {schedule.app_id}" + ) from e From 30bb55d1564fc32aa0ee9a34efe70d23fdcaeb3f Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 18:45:16 +0800 Subject: [PATCH 23/26] chore: enhance visual_to_cron method --- api/services/schedule_service.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 478eeedf04fef2..5f71ed58e6536a 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -233,38 +233,46 @@ def visual_to_cron(frequency: str, visual_config: VisualConfig) -> str: Maintains consistency with frontend UI expectations while supporting croniter's extended syntax. """ if frequency == "hourly": + if visual_config.on_minute is None: + raise ScheduleConfigError("on_minute is required for hourly schedules") return f"{visual_config.on_minute} * * * *" elif frequency == "daily": + if not visual_config.time: + raise ScheduleConfigError("time is required for daily schedules") hour, minute = convert_12h_to_24h(visual_config.time) return f"{minute} {hour} * * *" elif frequency == "weekly": + if not visual_config.time: + raise ScheduleConfigError("time is required for weekly schedules") + if not visual_config.weekdays: + raise ScheduleConfigError("Weekdays are required for weekly schedules") hour, minute = convert_12h_to_24h(visual_config.time) weekday_map = {"sun": "0", "mon": "1", "tue": "2", "wed": "3", "thu": "4", "fri": "5", "sat": "6"} cron_weekdays = [weekday_map[day] for day in visual_config.weekdays] return f"{minute} {hour} * * {','.join(sorted(cron_weekdays))}" elif frequency == "monthly": + if not visual_config.time: + raise ScheduleConfigError("time is required for monthly schedules") + if not visual_config.monthly_days: + raise ScheduleConfigError("Monthly days are required for monthly schedules") hour, minute = convert_12h_to_24h(visual_config.time) - cron_days = [] + numeric_days = [] + has_last = False for day in visual_config.monthly_days: if day == "last": - cron_days.append("L") + has_last = True else: - cron_days.append(str(day)) - - numeric_days = [d for d in cron_days if d != "L"] - has_last = "L" in cron_days - - sorted_days = [] - if numeric_days: - sorted_days = sorted(set(numeric_days), key=int) + numeric_days.append(day) + + result_days = [str(d) for d in sorted(set(numeric_days))] if has_last: - sorted_days.append("L") + result_days.append("L") - return f"{minute} {hour} {','.join(sorted_days)} * *" + return f"{minute} {hour} {','.join(result_days)} * *" else: raise ScheduleConfigError(f"Unsupported frequency: {frequency}") From 22f3f2aaa0e4fd1f8b7f7d1e11233e159f2b6cc1 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Thu, 4 Sep 2025 18:45:50 +0800 Subject: [PATCH 24/26] feat: enhance visual_to_cron method with VisualConfig integration and additional test cases --- .../services/test_schedule_service.py | 199 ++++++++++++++---- 1 file changed, 159 insertions(+), 40 deletions(-) diff --git a/api/tests/unit_tests/services/test_schedule_service.py b/api/tests/unit_tests/services/test_schedule_service.py index cc6b4428ef0fe3..59849bc3f4abfd 100644 --- a/api/tests/unit_tests/services/test_schedule_service.py +++ b/api/tests/unit_tests/services/test_schedule_service.py @@ -5,7 +5,8 @@ import pytest from sqlalchemy.orm import Session -from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate +from core.workflow.nodes.trigger_schedule.entities import ScheduleConfig, SchedulePlanUpdate, VisualConfig +from core.workflow.nodes.trigger_schedule.exc import ScheduleConfigError from events.event_handlers.sync_workflow_schedule_when_app_published import ( sync_schedule_from_workflow, ) @@ -258,72 +259,190 @@ class TestVisualToCron(unittest.TestCase): def test_visual_to_cron_hourly(self): """Test converting hourly visual config to cron.""" - visual_config = {"on_minute": 15} + visual_config = VisualConfig(on_minute=15) result = ScheduleService.visual_to_cron("hourly", visual_config) assert result == "15 * * * *" def test_visual_to_cron_daily(self): """Test converting daily visual config to cron.""" - visual_config = {"time": "2:30 PM"} + visual_config = VisualConfig(time="2:30 PM") result = ScheduleService.visual_to_cron("daily", visual_config) assert result == "30 14 * * *" def test_visual_to_cron_weekly(self): """Test converting weekly visual config to cron.""" - visual_config = { - "time": "10:00 AM", - "weekdays": ["mon", "wed", "fri"], - } + visual_config = VisualConfig( + time="10:00 AM", + weekdays=["mon", "wed", "fri"], + ) result = ScheduleService.visual_to_cron("weekly", visual_config) assert result == "0 10 * * 1,3,5" def test_visual_to_cron_monthly_with_specific_days(self): """Test converting monthly visual config with specific days.""" - visual_config = { - "time": "11:30 AM", - "monthly_days": [1, 15], - } + visual_config = VisualConfig( + time="11:30 AM", + monthly_days=[1, 15], + ) result = ScheduleService.visual_to_cron("monthly", visual_config) assert result == "30 11 1,15 * *" def test_visual_to_cron_monthly_with_last_day(self): """Test converting monthly visual config with last day using 'L' syntax.""" - visual_config = { - "time": "11:30 AM", - "monthly_days": [1, "last"], - } + visual_config = VisualConfig( + time="11:30 AM", + monthly_days=[1, "last"], + ) result = ScheduleService.visual_to_cron("monthly", visual_config) assert result == "30 11 1,L * *" def test_visual_to_cron_monthly_only_last_day(self): """Test converting monthly visual config with only last day.""" - visual_config = { - "time": "9:00 PM", - "monthly_days": ["last"], - } + visual_config = VisualConfig( + time="9:00 PM", + monthly_days=["last"], + ) result = ScheduleService.visual_to_cron("monthly", visual_config) assert result == "0 21 L * *" def test_visual_to_cron_monthly_with_end_days_and_last(self): """Test converting monthly visual config with days 29, 30, 31 and 'last'.""" - visual_config = { - "time": "3:45 PM", - "monthly_days": [29, 30, 31, "last"], - } + visual_config = VisualConfig( + time="3:45 PM", + monthly_days=[29, 30, 31, "last"], + ) result = ScheduleService.visual_to_cron("monthly", visual_config) # Should have 29,30,31,L - the L handles all possible last days assert result == "45 15 29,30,31,L * *" def test_visual_to_cron_invalid_frequency(self): """Test converting with invalid frequency.""" - result = ScheduleService.visual_to_cron("invalid", {}) - assert result is None + with pytest.raises(ScheduleConfigError, match="Unsupported frequency: invalid"): + ScheduleService.visual_to_cron("invalid", VisualConfig()) def test_visual_to_cron_weekly_no_weekdays(self): """Test converting weekly with no weekdays specified.""" - visual_config = {"time": "10:00 AM"} + visual_config = VisualConfig(time="10:00 AM") + with pytest.raises(ScheduleConfigError, match="Weekdays are required for weekly schedules"): + ScheduleService.visual_to_cron("weekly", visual_config) + + def test_visual_to_cron_hourly_no_minute(self): + """Test converting hourly with no on_minute specified.""" + visual_config = VisualConfig() # on_minute defaults to 0 + result = ScheduleService.visual_to_cron("hourly", visual_config) + assert result == "0 * * * *" # Should use default value 0 + + def test_visual_to_cron_daily_no_time(self): + """Test converting daily with no time specified.""" + visual_config = VisualConfig(time=None) + with pytest.raises(ScheduleConfigError, match="time is required for daily schedules"): + ScheduleService.visual_to_cron("daily", visual_config) + + def test_visual_to_cron_weekly_no_time(self): + """Test converting weekly with no time specified.""" + visual_config = VisualConfig(weekdays=["mon"]) + visual_config.time = None # Override default + with pytest.raises(ScheduleConfigError, match="time is required for weekly schedules"): + ScheduleService.visual_to_cron("weekly", visual_config) + + def test_visual_to_cron_monthly_no_time(self): + """Test converting monthly with no time specified.""" + visual_config = VisualConfig(monthly_days=[1]) + visual_config.time = None # Override default + with pytest.raises(ScheduleConfigError, match="time is required for monthly schedules"): + ScheduleService.visual_to_cron("monthly", visual_config) + + def test_visual_to_cron_monthly_duplicate_days(self): + """Test monthly with duplicate days should be deduplicated.""" + visual_config = VisualConfig( + time="10:00 AM", + monthly_days=[1, 15, 1, 15, 31], # Duplicates + ) + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "0 10 1,15,31 * *" # Should be deduplicated + + def test_visual_to_cron_monthly_unsorted_days(self): + """Test monthly with unsorted days should be sorted.""" + visual_config = VisualConfig( + time="2:30 PM", + monthly_days=[20, 5, 15, 1, 10], # Unsorted + ) + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "30 14 1,5,10,15,20 * *" # Should be sorted + + def test_visual_to_cron_weekly_all_weekdays(self): + """Test weekly with all weekdays.""" + visual_config = VisualConfig( + time="8:00 AM", + weekdays=["sun", "mon", "tue", "wed", "thu", "fri", "sat"], + ) result = ScheduleService.visual_to_cron("weekly", visual_config) - assert result is None + assert result == "0 8 * * 0,1,2,3,4,5,6" + + def test_visual_to_cron_hourly_boundary_values(self): + """Test hourly with boundary minute values.""" + # Minimum value + visual_config = VisualConfig(on_minute=0) + result = ScheduleService.visual_to_cron("hourly", visual_config) + assert result == "0 * * * *" + + # Maximum value + visual_config = VisualConfig(on_minute=59) + result = ScheduleService.visual_to_cron("hourly", visual_config) + assert result == "59 * * * *" + + def test_visual_to_cron_daily_midnight_noon(self): + """Test daily at special times (midnight and noon).""" + # Midnight + visual_config = VisualConfig(time="12:00 AM") + result = ScheduleService.visual_to_cron("daily", visual_config) + assert result == "0 0 * * *" + + # Noon + visual_config = VisualConfig(time="12:00 PM") + result = ScheduleService.visual_to_cron("daily", visual_config) + assert result == "0 12 * * *" + + def test_visual_to_cron_monthly_mixed_with_last_and_duplicates(self): + """Test monthly with mixed days, 'last', and duplicates.""" + visual_config = VisualConfig( + time="11:45 PM", + monthly_days=[15, 1, "last", 15, 30, 1, "last"], # Mixed with duplicates + ) + result = ScheduleService.visual_to_cron("monthly", visual_config) + assert result == "45 23 1,15,30,L * *" # Deduplicated and sorted with L at end + + def test_visual_to_cron_weekly_single_day(self): + """Test weekly with single weekday.""" + visual_config = VisualConfig( + time="6:30 PM", + weekdays=["sun"], + ) + result = ScheduleService.visual_to_cron("weekly", visual_config) + assert result == "30 18 * * 0" + + def test_visual_to_cron_monthly_all_possible_days(self): + """Test monthly with all 31 days plus 'last'.""" + all_days = list(range(1, 32)) + ["last"] + visual_config = VisualConfig( + time="12:01 AM", + monthly_days=all_days, + ) + result = ScheduleService.visual_to_cron("monthly", visual_config) + expected_days = ','.join([str(i) for i in range(1, 32)]) + ',L' + assert result == f"1 0 {expected_days} * *" + + def test_visual_to_cron_monthly_no_days(self): + """Test monthly without any days specified should raise error.""" + visual_config = VisualConfig(time="10:00 AM", monthly_days=[]) + with pytest.raises(ScheduleConfigError, match="Monthly days are required for monthly schedules"): + ScheduleService.visual_to_cron("monthly", visual_config) + + def test_visual_to_cron_weekly_empty_weekdays_list(self): + """Test weekly with empty weekdays list should raise error.""" + visual_config = VisualConfig(time="10:00 AM", weekdays=[]) + with pytest.raises(ScheduleConfigError, match="Weekdays are required for weekly schedules"): + ScheduleService.visual_to_cron("weekly", visual_config) class TestParseTime(unittest.TestCase): @@ -451,8 +570,8 @@ def test_extract_schedule_config_invalid_graph(self): workflow = Mock(spec=Workflow) workflow.graph_dict = None - config = ScheduleService.extract_schedule_config(workflow) - assert config is None + with pytest.raises(ScheduleConfigError, match="Workflow graph is empty"): + ScheduleService.extract_schedule_config(workflow) class TestScheduleWithTimezone(unittest.TestCase): @@ -465,10 +584,10 @@ def test_visual_schedule_with_timezone_integration(self): it runs at 10:30 AM Shanghai time, not 10:30 AM UTC. """ # User in Shanghai wants to run a task at 10:30 AM local time - visual_config = { - "time": "10:30 AM", # This is Shanghai time - "monthly_days": [1], - } + visual_config = VisualConfig( + time="10:30 AM", # This is Shanghai time + monthly_days=[1], + ) # Convert to cron expression cron_expr = ScheduleService.visual_to_cron("monthly", visual_config) @@ -499,10 +618,10 @@ def test_visual_schedule_different_timezones_same_local_time(self): This verifies that a schedule set for "9:00 AM" runs at 9 AM local time regardless of the timezone. """ - visual_config = { - "time": "9:00 AM", - "weekdays": ["mon"], - } + visual_config = VisualConfig( + time="9:00 AM", + weekdays=["mon"], + ) cron_expr = ScheduleService.visual_to_cron("weekly", visual_config) assert cron_expr is not None @@ -531,9 +650,9 @@ def test_visual_schedule_daily_across_dst_change(self): A schedule set for "10:00 AM" should always run at 10 AM local time, even when DST changes. """ - visual_config = { - "time": "10:00 AM", - } + visual_config = VisualConfig( + time="10:00 AM", + ) cron_expr = ScheduleService.visual_to_cron("daily", visual_config) assert cron_expr is not None From 30e562d08a988a3f298a85cb4803792254477e82 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Tue, 9 Sep 2025 03:15:50 +0800 Subject: [PATCH 25/26] feat: improve schedule dispatch with streaming and parallel processing --- api/.env.example | 2 + api/configs/feature/__init__.py | 4 + api/docker/entrypoint.sh | 4 +- api/schedule/workflow_schedule_task.py | 77 +++++++++++++------ api/tasks/workflow_schedule_tasks.py | 2 +- .../unit_tests/extensions/test_celery_ssl.py | 1 + dev/start-worker | 7 +- docker/.env.example | 1 + docker/docker-compose.yaml | 1 + 9 files changed, 69 insertions(+), 30 deletions(-) diff --git a/api/.env.example b/api/.env.example index 152af119844993..336b1f47bd90eb 100644 --- a/api/.env.example +++ b/api/.env.example @@ -509,6 +509,8 @@ ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK=true # Interval time in minutes for polling scheduled workflows(default: 1 min) WORKFLOW_SCHEDULE_POLLER_INTERVAL=1 WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE=100 +# Maximum number of scheduled workflows to dispatch per tick (0 for unlimited) +WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK=0 # Position configuration POSITION_TOOL_PINS= diff --git a/api/configs/feature/__init__.py b/api/configs/feature/__init__.py index 9d398741864aa3..986c13085cd769 100644 --- a/api/configs/feature/__init__.py +++ b/api/configs/feature/__init__.py @@ -894,6 +894,10 @@ class CeleryScheduleTasksConfig(BaseSettings): description="Maximum number of schedules to process in each poll batch", default=100, ) + WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK: int = Field( + description="Maximum schedules to dispatch per tick (0=unlimited, circuit breaker)", + default=0, + ) class PositionConfig(BaseSettings): diff --git a/api/docker/entrypoint.sh b/api/docker/entrypoint.sh index 6580ea9c13db42..25d1a6d34d4838 100755 --- a/api/docker/entrypoint.sh +++ b/api/docker/entrypoint.sh @@ -34,10 +34,10 @@ if [[ "${MODE}" == "worker" ]]; then if [[ -z "${CELERY_QUEUES}" ]]; then if [[ "${EDITION}" == "CLOUD" ]]; then # Cloud edition: separate queues for dataset and trigger tasks - DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow_professional,workflow_team,workflow_sandbox,schedule" + DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow_professional,workflow_team,workflow_sandbox,schedule_poller,schedule_executor" else # Community edition (SELF_HOSTED): dataset and workflow have separate queues - DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow,schedule" + DEFAULT_QUEUES="dataset,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow,schedule_poller,schedule_executor" fi else DEFAULT_QUEUES="${CELERY_QUEUES}" diff --git a/api/schedule/workflow_schedule_task.py b/api/schedule/workflow_schedule_task.py index e03bac76e5d915..30e00ee27cb97f 100644 --- a/api/schedule/workflow_schedule_task.py +++ b/api/schedule/workflow_schedule_task.py @@ -1,6 +1,6 @@ import logging -from celery import shared_task +from celery import group, shared_task from sqlalchemy import and_, select from sqlalchemy.orm import Session, sessionmaker @@ -8,39 +8,64 @@ from extensions.ext_database import db from libs.datetime_utils import naive_utc_now from libs.schedule_utils import calculate_next_run_at -from models.workflow import AppTrigger, AppTriggerStatus, WorkflowSchedulePlan +from models.workflow import AppTrigger, AppTriggerStatus, AppTriggerType, WorkflowSchedulePlan from services.workflow.queue_dispatcher import QueueDispatcherManager from tasks.workflow_schedule_tasks import run_schedule_trigger logger = logging.getLogger(__name__) -@shared_task(queue="schedule") +@shared_task(queue="schedule_poller") def poll_workflow_schedules() -> None: """ Poll and process due workflow schedules. - Simple 2-step flow: - 1. Fetch due schedules - 2. Process valid schedules + Streaming flow: + 1. Fetch due schedules in batches + 2. Process each batch until all due schedules are handled + 3. Optional: Limit total dispatches per tick as a circuit breaker """ session_factory = sessionmaker(bind=db.engine, expire_on_commit=False) with session_factory() as session: - due_schedules = _fetch_due_schedules(session) + total_dispatched = 0 + total_rate_limited = 0 + + # Process in batches until we've handled all due schedules or hit the limit + while True: + due_schedules = _fetch_due_schedules(session) + + if not due_schedules: + break - if due_schedules: dispatched_count, rate_limited_count = _process_schedules(session, due_schedules) - logger.info( - "Processed %d/%d schedules (%d skipped due to rate limit)", - dispatched_count, - len(due_schedules), - rate_limited_count, - ) + total_dispatched += dispatched_count + total_rate_limited += rate_limited_count + + logger.debug("Batch processed: %d dispatched, %d rate limited", dispatched_count, rate_limited_count) + + # Circuit breaker: check if we've hit the per-tick limit (if enabled) + if ( + dify_config.WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK > 0 + and total_dispatched >= dify_config.WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK + ): + logger.warning( + "Circuit breaker activated: reached dispatch limit (%d), will continue next tick", + dify_config.WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK, + ) + break + + if total_dispatched > 0 or total_rate_limited > 0: + logger.info("Total processed: %d dispatched, %d rate limited", total_dispatched, total_rate_limited) def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: - """Fetch all schedules that are due for execution.""" + """ + Fetch a batch of due schedules, sorted by most overdue first. + + Returns up to WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE schedules per call. + Used in a loop to progressively process all due schedules. + """ now = naive_utc_now() due_schedules = session.scalars( @@ -51,7 +76,7 @@ def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: and_( AppTrigger.app_id == WorkflowSchedulePlan.app_id, AppTrigger.node_id == WorkflowSchedulePlan.node_id, - AppTrigger.trigger_type == "trigger-schedule", + AppTrigger.trigger_type == AppTriggerType.TRIGGER_SCHEDULE, ), ) .where( @@ -60,6 +85,7 @@ def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: AppTrigger.status == AppTriggerStatus.ENABLED, ) ) + .order_by(WorkflowSchedulePlan.next_run_at.asc()) .with_for_update(skip_locked=True) .limit(dify_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE) ) @@ -68,31 +94,34 @@ def _fetch_due_schedules(session: Session) -> list[WorkflowSchedulePlan]: def _process_schedules(session: Session, schedules: list[WorkflowSchedulePlan]) -> tuple[int, int]: - """Process schedules: check quota, update next run time and dispatch to Celery.""" + """Process schedules: check quota, update next run time and dispatch to Celery in parallel.""" if not schedules: return 0, 0 - dispatched_count = 0 - rate_limited_count = 0 dispatcher_manager = QueueDispatcherManager() + tasks_to_dispatch = [] + rate_limited_count = 0 for schedule in schedules: next_run_at = calculate_next_run_at( schedule.cron_expression, schedule.timezone, ) - schedule.next_run_at = next_run_at dispatcher = dispatcher_manager.get_dispatcher(schedule.tenant_id) - if not dispatcher.check_daily_quota(schedule.tenant_id): logger.info("Tenant %s rate limited, skipping schedule_plan %s", schedule.tenant_id, schedule.id) rate_limited_count += 1 else: - run_schedule_trigger.delay(schedule.id) - dispatched_count += 1 + tasks_to_dispatch.append(schedule.id) + + if tasks_to_dispatch: + job = group(run_schedule_trigger.s(schedule_id) for schedule_id in tasks_to_dispatch) + job.apply_async() + + logger.debug("Dispatched %d tasks in parallel", len(tasks_to_dispatch)) session.commit() - return dispatched_count, rate_limited_count + return len(tasks_to_dispatch), rate_limited_count diff --git a/api/tasks/workflow_schedule_tasks.py b/api/tasks/workflow_schedule_tasks.py index e2eeee86cc3250..17f4e0751c9eee 100644 --- a/api/tasks/workflow_schedule_tasks.py +++ b/api/tasks/workflow_schedule_tasks.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) -@shared_task(queue="schedule") +@shared_task(queue="schedule_executor") def run_schedule_trigger(schedule_id: str) -> None: """ Execute a scheduled workflow trigger. diff --git a/api/tests/unit_tests/extensions/test_celery_ssl.py b/api/tests/unit_tests/extensions/test_celery_ssl.py index 31719fc05ee877..d33b7eaf23f1bf 100644 --- a/api/tests/unit_tests/extensions/test_celery_ssl.py +++ b/api/tests/unit_tests/extensions/test_celery_ssl.py @@ -134,6 +134,7 @@ def test_celery_init_applies_ssl_to_broker_and_backend(self): mock_config.ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK = False mock_config.WORKFLOW_SCHEDULE_POLLER_INTERVAL = 1 mock_config.WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE = 100 + mock_config.WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK = 0 with patch("extensions.ext_celery.dify_config", mock_config): from dify_app import DifyApp diff --git a/dev/start-worker b/dev/start-worker index 8334af51538725..37dd62ea0b4ba4 100755 --- a/dev/start-worker +++ b/dev/start-worker @@ -24,7 +24,8 @@ show_help() { echo " workflow_professional - Professional tier workflows (cloud edition)" echo " workflow_team - Team tier workflows (cloud edition)" echo " workflow_sandbox - Sandbox tier workflows (cloud edition)" - echo " schedule - Scheduled workflow executions" + echo " schedule_poller - Schedule polling tasks" + echo " schedule_executor - Schedule execution tasks" echo " generation - Content generation tasks" echo " mail - Email notifications" echo " ops_trace - Operations tracing" @@ -80,10 +81,10 @@ if [[ -z "${QUEUES}" ]]; then # Configure queues based on edition if [[ "${EDITION}" == "CLOUD" ]]; then # Cloud edition: separate queues for dataset and trigger tasks - QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow_professional,workflow_team,workflow_sandbox,schedule" + QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow_professional,workflow_team,workflow_sandbox,schedule_poller,schedule_executor" else # Community edition (SELF_HOSTED): dataset and workflow have separate queues - QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow,schedule" + QUEUES="dataset,generation,mail,ops_trace,app_deletion,plugin,workflow_storage,conversation,workflow,schedule_poller,schedule_executor" fi echo "No queues specified, using edition-based defaults: ${QUEUES}" diff --git a/docker/.env.example b/docker/.env.example index 0001ffe49bc268..cb1cac78416972 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -1275,3 +1275,4 @@ ENABLE_CHECK_UPGRADABLE_PLUGIN_TASK=true ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK=true WORKFLOW_SCHEDULE_POLLER_INTERVAL=1 WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE=100 +WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK=0 diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index ffa372854d73ef..b2529fdbef811c 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -582,6 +582,7 @@ x-shared-env: &shared-api-worker-env ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK: ${ENABLE_WORKFLOW_SCHEDULE_POLLER_TASK:-true} WORKFLOW_SCHEDULE_POLLER_INTERVAL: ${WORKFLOW_SCHEDULE_POLLER_INTERVAL:-1} WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE: ${WORKFLOW_SCHEDULE_POLLER_BATCH_SIZE:-100} + WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK: ${WORKFLOW_SCHEDULE_MAX_DISPATCH_PER_TICK:-0} services: # API service From 3500d4729b2583e35ef6cfc42bd5b0db17c7aae5 Mon Sep 17 00:00:00 2001 From: ACAne0320 Date: Tue, 9 Sep 2025 03:16:52 +0800 Subject: [PATCH 26/26] chore: remove redundant return statements in account retrieval and schedule config extraction --- api/services/schedule_service.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/api/services/schedule_service.py b/api/services/schedule_service.py index 5f71ed58e6536a..333eeb2cc46e43 100644 --- a/api/services/schedule_service.py +++ b/api/services/schedule_service.py @@ -144,8 +144,6 @@ def get_tenant_owner(session: Session, tenant_id: str) -> Optional[Account]: if result: return session.get(Account, result.account_id) - return None - @staticmethod def update_next_run_at( session: Session, @@ -224,8 +222,6 @@ def extract_schedule_config(workflow: Workflow) -> Optional[ScheduleConfig]: return ScheduleConfig(node_id=node_id, cron_expression=cron_expression, timezone=timezone) - return None - @staticmethod def visual_to_cron(frequency: str, visual_config: VisualConfig) -> str: """ @@ -267,7 +263,7 @@ def visual_to_cron(frequency: str, visual_config: VisualConfig) -> str: has_last = True else: numeric_days.append(day) - + result_days = [str(d) for d in sorted(set(numeric_days))] if has_last: result_days.append("L")