Periodic workflows#41
Conversation
88ef818 to
142fe8b
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces periodic workflow execution by splitting workflow definition/schedule (Workflow) from workflow execution state (WorkflowRun), allowing a workflow to create recurring runs when dispatch_interval is set.
Changes:
- Added
WorkflowRunmodel + API endpoint and moved execution state (state, timestamps,error,current_task,task_group) fromWorkflowontoWorkflowRun. - Implemented periodic scheduling via
dispatch_intervalby schedulingstart_workflow_run, which creates a run and dispatches execution. - Updated cancellation semantics (stop workflow vs cancel run) and adjusted callback execution/environment to be run-aware; expanded unit + functional coverage.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pulp_workflow/app/models.py | Introduces WorkflowRun and moves execution-state semantics off Workflow. |
| pulp_workflow/app/serializers.py | Adds dispatch_interval to workflows and introduces WorkflowRunSerializer; schedules start_workflow_run. |
| pulp_workflow/app/tasks.py | Adds start_workflow_run; updates execution/callback dispatch to be run-based. |
| pulp_workflow/app/viewsets.py | Adds WorkflowRunViewSet; changes workflow PATCH to “stop schedule + cancel in-flight runs”. |
| pulp_workflow/app/signals.py | Updates TaskGroup cancel propagation to operate on WorkflowRun. |
| pulp_workflow/app/migrations/0005_workflowrun.py | Schema + data migration to add runs, remove old workflow execution fields, and rewrite schedules. |
| pulp_workflow/pytest_plugin.py | Updates fixtures/helpers to monitor runs instead of workflows. |
| pulp_workflow/tests/unit/test_viewsets.py | Adds unit coverage for run permissions/access policy behavior. |
| pulp_workflow/tests/unit/test_tasks.py | Updates unit tests for run-based task execution and start_workflow_run. |
| pulp_workflow/tests/unit/test_models.py | Adjusts callback env tests to use a run wrapper (run state + workflow metadata). |
| pulp_workflow/tests/functional/api/test_execute_workflow.py | Refactors functional assertions to target runs + TaskGroups rather than workflow state. |
| pulp_workflow/tests/functional/api/test_crud_workflows.py | Updates CRUD expectations and stop semantics to be run-based. |
| pulp_workflow/tests/functional/api/test_workflow_callbacks.py | Updates callback functional tests and adds cancellation callback coverage. |
| pulp_workflow/tests/functional/api/test_periodic_workflow.py | Adds functional coverage for periodic run creation and non-overlap behavior. |
| docs/design.md | Updates design documentation to describe definition vs run split and scheduling/cancel flows. |
| CHANGES/20.feature | Changelog entry for periodic workflows + new WorkflowRun resource. |
| CHANGES/20.removal | Changelog entry for removed fields from Workflow / WorkflowTask. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
docs/design.md:17
- The design doc says modeling runs separately lets “overlapping runs of the same workflow coexist”, but the next section states
start_workflow_runskips starting a new run when an unfinished run exists (no overlap). These statements conflict; update this paragraph to match the implemented behavior.
most-recently-dispatched `current_task`, and its own `task_group`. Modeling
runs separately lets a periodic workflow accumulate a history of runs and lets
overlapping runs of the same workflow coexist.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
docs/design.md:17
- The "Definitions vs. runs" section says modeling runs separately "lets overlapping runs of the same workflow coexist", but the implementation (and the next section) explicitly skips starting a new run while a previous run is unfinished. This is contradictory and may confuse readers about the overlap guarantees.
most-recently-dispatched `current_task`, and its own `task_group`. Modeling
runs separately lets a periodic workflow accumulate a history of runs and lets
overlapping runs of the same workflow coexist.
b0f35e0 to
2264d2d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
pulp_workflow/app/serializers.py:202
- With periodic workflows, callbacks fire once per
WorkflowRun, butWorkflowCallback.dispatched_taskis a single field shared across all runs. New runs will overwrite the previous run’s callback task href, so clients cannot reliably audit/monitor callback results per run (and overlapping callback execution across runs can make this misleading). Consider modeling dispatched callback tasks per run (e.g., aWorkflowRunCallbackjoin model) or otherwise persisting callback dispatch results onWorkflowRuninstead of the workflow-level callback definition.
dispatched_task = RelatedField(
view_name="tasks-detail",
read_only=True,
help_text=_("Href of the most recently dispatched callback task, if any."),
)
docs/design.md:17
- This paragraph says separating runs “lets overlapping runs of the same workflow coexist”, but the next section explicitly states overlapping runs are not created (the scheduler skips while an unfinished run exists). Consider rewording to avoid the contradiction (either explain that the model could support overlaps but the scheduler prevents them, or remove the overlap claim).
Each execution is a separate `WorkflowRun`. A run carries the state that used
to live on the workflow: `state`, `started_at`, `finished_at`, `error`, the
most-recently-dispatched `current_task`, and its own `task_group`. Modeling
runs separately lets a periodic workflow accumulate a history of runs and lets
overlapping runs of the same workflow coexist.
29277f6 to
eb50063
Compare
eb50063 to
a1b02b3
Compare
a1b02b3 to
3f4bd32
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
docs/design.md:17
- The "Definitions vs. runs" section says separating runs "lets overlapping runs of the same workflow coexist", but the very next section states overlapping runs are skipped/not created. This is internally inconsistent and contradicts the current implementation (start_workflow_run skips when an unfinished run exists).
most-recently-dispatched `current_task`, and its own `task_group`. Modeling
runs separately lets a periodic workflow accumulate a history of runs and lets
overlapping runs of the same workflow coexist.
pulp_workflow/app/serializers.py:389
WorkflowCancelSerializeris used for both stopping a Workflow and canceling a WorkflowRun, but thehelp_textcurrently says "desired state of the workflow". This will surface in the API schema/docs and is misleading for the WorkflowRun cancel endpoint.
state = serializers.ChoiceField(
choices=[(TASK_STATES.CANCELED, "Canceled")],
help_text=_("The desired state of the workflow. Only 'canceled' is accepted."),
required=True,
)
e12b3f2 to
d592073
Compare
d592073 to
fc73224
Compare
fc73224 to
152246d
Compare
152246d to
868bd6f
Compare
868bd6f to
4b98d55
Compare
4b98d55 to
5f579a6
Compare
Added support for periodic workflows. A `Workflow` can now be given a `dispatch_interval`, in which case it re-runs on that recurring schedule instead of running just once at `start_time`. fixes pulp#20 Assisted-by: GitHub Copilot (Claude Opus 4.8)
5f579a6 to
893f3da
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
pulp_workflow/app/serializers.py:445
WorkflowCancelSerializeris used for both stopping a workflow and canceling an individualWorkflowRun, but thestatefield help text still says “desired state of the workflow”, which is inaccurate for the run-cancel endpoint and will show up in generated API docs.
state = serializers.ChoiceField(
choices=[(TASK_STATES.CANCELED, "Canceled")],
help_text=_("The desired state of the workflow. Only 'canceled' is accepted."),
required=True,
)
Added support for periodic workflows. A
Workflowcan now be given adispatch_interval, in which case it re-runs on that recurring schedule instead of running just once atstart_time.fixes #20
Assisted-by: GitHub Copilot (Claude Opus 4.8)
📜 Checklist
See: Pull Request Walkthrough