feat(gtf): add task dependencies (DAG) with chain-icon Task List display - #43408
Merged
villebro merged 6 commits intoAug 22, 2026
Merged
Conversation
Contributor
|
Bito Automatic Review Skipped - Branch Excluded |
github-actions
Bot
added
risk:db-migration
9 tasks
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## gaq-to-gtf #43408 +/- ##
==============================================
+ Coverage 78.83% 78.92% +0.08%
==============================================
Files 2880 2887 +7
Lines 164437 165286 +849
Branches 37943 38023 +80
==============================================
+ Hits 129639 130448 +809
- Misses 32353 32391 +38
- Partials 2445 2447 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
villebro
force-pushed
the
villebro/gtf-task-dependencies
branch
from
August 22, 2026 00:25
d1b2e1f to
d936b14
Compare
Add a first-class, optional task-dependency capability to the Global Task Framework: a task can declare prerequisite tasks and only runs once they all reach terminal SUCCESS, failing fast (cascading down the DAG) if any prerequisite ends non-SUCCESS (all_success semantics). - New task_dependencies junction table + migration (FK ON DELETE CASCADE so edges are pruned with their tasks); Task.dependencies exposes the prerequisite Task entities via a self-referential many-to-many (selectin, viewonly). - TaskOptions.depends_on accepts Task entities (canonical), UUIDs, or UUID strings; threaded through @task.schedule -> TaskManager.submit_task -> SubmitTaskCommand, which persists edges under the dedup lock with a cycle guard (rejects self- and transitive cycles). - Scheduler gate (Model A block-and-wait): a dependent stays PENDING and waits on each prerequisite via TaskManager.wait_for_completion, then runs iff all succeeded, else transitions to FAILURE naming the failed prerequisite. - REST API exposes depends_on (uuid/task_name/status) via TaskResponseSchema. - Task List UI: chain-icon (LinkOutlined) dependency popover listing each prerequisite with its status, plus a "waiting on N prerequisites" indicator. - superset-core: abstract TaskDependency model + TaskOptions.depends_on. - Docs, unit + integration + frontend tests. Note: the pre-existing whole-project frontend type-check failures (FoldersEditor, softDeleteCopy, etc.) are unrelated to this change.
villebro
force-pushed
the
villebro/gtf-task-dependencies
branch
from
August 22, 2026 00:26
d936b14 to
2d588dd
Compare
Member
Author
|
/review |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #ecd633
Actionable Suggestions - 1
-
superset/models/task_dependencies.py - 1
- Missing unit tests for TaskDependency model · Line 1-69
Additional Suggestions - 1
-
tests/integration_tests/tasks/api_tests.py - 1
-
Incomplete test assertions · Line 502-504Test docstring claims to verify `uuid/name/status` but only checks `uuid` and `status`. Missing assertion for `task_name` field returned by `get_depends_on` in `superset/tasks/schemas.py:183`. Without this, a future regression that drops `task_name` would go undetected.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/tasks/scheduler.py - 1
- Unguarded publish_completion after failed transition · Line 399-399
-
superset-frontend/src/features/tasks/TaskDependenciesPopover.tsx - 2
- Dead state blocks popover hover · Line 65-65
- Dead unused React state variable · Line 65-65
-
superset-core/src/superset_core/tasks/models.py - 1
- Missing __all__ for new model · Line 166-191
Review Details
-
Files reviewed - 21 · Commit Range:
2d588dd..2d588dd- superset-core/src/superset_core/tasks/models.py
- superset-core/src/superset_core/tasks/types.py
- superset-frontend/src/features/tasks/TaskDependenciesPopover.test.tsx
- superset-frontend/src/features/tasks/TaskDependenciesPopover.tsx
- superset-frontend/src/features/tasks/types.ts
- superset-frontend/src/pages/TaskList/TaskList.test.tsx
- superset-frontend/src/pages/TaskList/index.tsx
- superset/commands/tasks/exceptions.py
- superset/commands/tasks/submit.py
- superset/daos/tasks.py
- superset/migrations/versions/2026-08-21_12-00_7e2c9a4f1b83_create_task_dependencies_table.py
- superset/models/task_dependencies.py
- superset/models/tasks.py
- superset/tasks/decorators.py
- superset/tasks/manager.py
- superset/tasks/scheduler.py
- superset/tasks/schemas.py
- tests/integration_tests/tasks/api_tests.py
- tests/integration_tests/tasks/commands/test_submit.py
- tests/unit_tests/tasks/test_decorators.py
- tests/unit_tests/tasks/test_dependencies.py
-
Files skipped - 1
- docs/developer_docs/extensions/tasks.md - Reason: Filter setting
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers an incremental AI Review. -
/review full- Manually triggers a full AI Review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Comment on lines
+1
to
+69
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| """TaskDependency model for the Global Task Framework (GTF) task DAG""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from flask_appbuilder import Model | ||
| from sqlalchemy import Column, ForeignKey, Integer, UniqueConstraint | ||
| from superset_core.tasks.models import TaskDependency as CoreTaskDependency | ||
|
|
||
| from superset.models.helpers import AuditMixinNullable | ||
|
|
||
|
|
||
| class TaskDependency(CoreTaskDependency, AuditMixinNullable, Model): | ||
| """ | ||
| A directed edge in the task dependency graph (DAG). | ||
|
|
||
| The dependent task (``task_id``) waits for the prerequisite task | ||
| (``depends_on_task_id``) to reach a terminal state before it runs. A task | ||
| only executes once *every* prerequisite has reached a terminal SUCCESS; if | ||
| any prerequisite ends in a non-SUCCESS terminal state the dependent fails | ||
| without running (``all_success`` semantics), which cascades transitively to | ||
| its own dependents. | ||
|
|
||
| This is a pure edge table: prerequisite ``Task`` entities are read through | ||
| ``Task.dependencies`` (a self-referential many-to-many over this table). | ||
| Both foreign keys use ``ON DELETE CASCADE`` so edges are cleaned up when | ||
| either endpoint task is pruned. This is required because | ||
| ``TaskPruneCommand`` deletes tasks via a bulk core ``DELETE`` that bypasses | ||
| the ORM cascade. | ||
| """ | ||
|
|
||
| __tablename__ = "task_dependencies" | ||
|
|
||
| id = Column(Integer, primary_key=True) | ||
| task_id = Column( | ||
| Integer, ForeignKey("tasks.id", ondelete="CASCADE"), nullable=False | ||
| ) | ||
| depends_on_task_id = Column( | ||
| Integer, ForeignKey("tasks.id", ondelete="CASCADE"), nullable=False | ||
| ) | ||
|
|
||
| __table_args__ = ( | ||
| UniqueConstraint( | ||
| "task_id", | ||
| "depends_on_task_id", | ||
| name="uq_task_dependencies_task_depends_on", | ||
| ), | ||
| ) | ||
|
|
||
| def __repr__(self) -> str: | ||
| return ( | ||
| f"<TaskDependency task_id={self.task_id} " | ||
| f"depends_on_task_id={self.depends_on_task_id}>" | ||
| ) |
Contributor
There was a problem hiding this comment.
Missing unit tests for TaskDependency model
The new TaskDependency model has no unit test coverage. BITO.md rule [11730] requires comprehensive unit tests for new tools covering success paths, error scenarios, validation failures, and edge cases. At minimum, add tests for TaskDAO.add_dependency (success and idempotent duplicate) and get_prerequisite_ids.
Code Review Run #ecd633
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
villebro
marked this pull request as ready for review
August 22, 2026 00:50
…nds_on Addresses PR apache#43408 review nits: add unit tests for TaskDAO.add_dependency (success + idempotent duplicate) and get_prerequisite_ids, and assert the task_name field in the depends_on API serialization test.
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Comment on lines
+411
to
+414
| const unmet = depends_on.filter( | ||
| dep => dep.status !== TaskStatus.Success, | ||
| ).length; | ||
| const showWaiting = status === TaskStatus.Pending && unmet > 0; |
Contributor
There was a problem hiding this comment.
Suggestion: The unmet count treats every non-success status, including terminal FAILURE, ABORTED, and TIMED_OUT, as still waiting. When a prerequisite has already failed, the dependent is not waiting for it to finish; it is being failed by the DAG worker, so this displays a misleading “Waiting on N” state. Count only non-terminal prerequisite states for the waiting indicator, and handle failed prerequisites separately. [incorrect condition logic]
Severity Level: Minor 🧹
- ⚠️ Task List shows failed prerequisites as still pending.
- ⚠️ DAG failure state is temporarily misrepresented to operators.
- ⚠️ Users may expect prerequisite completion that cannot occur.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/src/pages/TaskList/index.tsx
**Line:** 411:414
**Comment:**
*Incorrect Condition Logic: The unmet count treats every non-success status, including terminal `FAILURE`, `ABORTED`, and `TIMED_OUT`, as still waiting. When a prerequisite has already failed, the dependent is not waiting for it to finish; it is being failed by the DAG worker, so this displays a misleading “Waiting on N” state. Count only non-terminal prerequisite states for the waiting indicator, and handle failed prerequisites separately.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+124
to
+126
| depends_on = Method( | ||
| "get_depends_on", metadata={"description": depends_on_description} | ||
| ) |
Contributor
There was a problem hiding this comment.
Suggestion: The new schema field is not included in TaskRestApi.list_columns or show_columns, so the REST API will omit depends_on from task list and detail responses. The frontend therefore never receives dependency data in production and cannot render the waiting indicator or chain popover. Add this field to the API column configuration as well as the schema. [api mismatch]
Severity Level: Major ⚠️
- ❌ Task List dependency display is absent for DAG tasks.
- ❌ Task detail responses omit prerequisite task information.
- ⚠️ Waiting-state indicators cannot show unmet prerequisites.
- ⚠️ Existing dependency serialization integration tests fail.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/tasks/schemas.py
**Line:** 124:126
**Comment:**
*Api Mismatch: The new schema field is not included in `TaskRestApi.list_columns` or `show_columns`, so the REST API will omit `depends_on` from task list and detail responses. The frontend therefore never receives dependency data in production and cannot render the waiting indicator or chain popover. Add this field to the API column configuration as well as the schema.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| """ | ||
| if not uuids: | ||
| return [] | ||
| return db.session.query(Task).filter(Task.uuid.in_(uuids)).all() |
Contributor
There was a problem hiding this comment.
Suggestion: This lookup deliberately bypasses the request-scoped TaskFilter, allowing dependency edges to resolve tasks that the submitting user is not authorized to see. Once the relationship is loaded for a visible task, prerequisite UUIDs, names, and statuses can be exposed through task responses, and callers who know a private task UUID can also attach their task to that private task. Enforce visibility or ownership for user-initiated dependency references, while keeping an explicitly separate unrestricted path for internal scheduler reads. [security]
Severity Level: Major ⚠️
- ❌ Private task names and statuses can leak through dependent-task responses.
- ⚠️ Cross-user dependency edges bypass TaskFilter visibility rules.
- ⚠️ User-submitted DAGs can reference tasks outside their subscriptions.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/daos/tasks.py
**Line:** 355:355
**Comment:**
*Security: This lookup deliberately bypasses the request-scoped `TaskFilter`, allowing dependency edges to resolve tasks that the submitting user is not authorized to see. Once the relationship is loaded for a visible task, prerequisite UUIDs, names, and statuses can be exposed through task responses, and callers who know a private task UUID can also attach their task to that private task. Enforce visibility or ownership for user-initiated dependency references, while keeping an explicitly separate unrestricted path for internal scheduler reads.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
|
||
| for prerequisite in prerequisites: | ||
| try: | ||
| final = TaskManager.wait_for_completion(prerequisite.uuid) |
Contributor
There was a problem hiding this comment.
Suggestion: wait_for_completion is called with its unlimited default timeout while the dependent occupies a Celery worker in PENDING. A stalled prerequisite, or a scheduling order that places dependents ahead of their prerequisites, can permanently consume worker slots and deadlock the DAG because the prerequisite may never get a worker to complete. [performance]
Severity Level: Critical 🚨
- ❌ Wide DAGs can exhaust Celery worker capacity.
- ❌ Dependents can prevent queued prerequisites from executing.
- ⚠️ Tasks remain permanently pending without timeout recovery.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/tasks/scheduler.py
**Line:** 303:303
**Comment:**
*Performance: `wait_for_completion` is called with its unlimited default timeout while the dependent occupies a Celery worker in `PENDING`. A stalled prerequisite, or a scheduling order that places dependents ahead of their prerequisites, can permanently consume worker slots and deadlock the DAG because the prerequisite may never get a worker to complete.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+387
to
+391
| InternalStatusTransitionCommand( | ||
| task_uuid=native_uuid, | ||
| new_status=TaskStatus.FAILURE, | ||
| expected_status=[TaskStatus.PENDING, TaskStatus.ABORTING], | ||
| set_ended_at=True, |
Contributor
There was a problem hiding this comment.
Suggestion: When an abort changes the task to ABORTING while the worker is waiting on prerequisites, this transition permits the worker to overwrite that state with FAILURE. The task can therefore be reported as dependency-failed instead of ABORTED, and the completion notification also publishes the wrong terminal status. Do not allow an aborting task to transition to dependency failure, or re-check and preserve the abort state before publishing completion. [race condition]
Severity Level: Major ⚠️
- ❌ Cancelled DAG tasks can be reported as dependency failures.
- ⚠️ Completion notifications publish the wrong terminal status.
- ⚠️ Downstream dependents receive incorrect failure propagation.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/tasks/scheduler.py
**Line:** 387:391
**Comment:**
*Race Condition: When an abort changes the task to `ABORTING` while the worker is waiting on prerequisites, this transition permits the worker to overwrite that state with `FAILURE`. The task can therefore be reported as dependency-failed instead of `ABORTED`, and the completion notification also publishes the wrong terminal status. Do not allow an aborting task to transition to dependency failure, or re-check and preserve the abort state before publishing completion.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixPer review preference: assert depends_on/dependencies defaults into existing tests rather than standalone duplicates. Merge the depends_on merge/forward checks into the existing TaskOptions merge + schedule tests; assert empty dependencies in the pre-existing DAO find, submit-success, and API response tests; consolidate the DAO add_dependency coverage into one test; and move the popover hover-content assertion to TaskDependenciesPopover.test.tsx (dropping the duplicate in TaskList.test.tsx).
The prerequisite gate reused wait_for_completion per dependency, which does its own fail-fast + predicate reads even when task.dependencies (already selectin- loaded with the task in one query) shows the prerequisite is terminal. Trust the loaded snapshot for already-terminal prerequisites (a terminal status never changes) and only wait on the ones still running. Common FIFO case now does zero extra reads in the gate.
The submit path did 1 + 3N queries for N deps: a per-node transitive cycle DFS (N queries) plus per-edge check-then-insert (2N). The cycle DFS is a no-op on the create path — a brand-new task has no incoming edges, so its new edges cannot close a cycle; only a direct self-dependency is possible, which is rejected in-memory. And the per-edge existence check is unnecessary since UUIDs are already de-duplicated and the task is new. Replace with a single find_by_uuids resolve + one bulk add_dependencies insert = 2 round-trips regardless of N. Drops _check_no_cycle and get_prerequisite_ids (a future edge-mutation API on existing tasks would need transitive cycle detection re-added).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
Adds a first-class, optional task-dependency (DAG) capability to the Global Task Framework (GTF): a task can declare prerequisite tasks and only runs once they all reach a terminal
SUCCESS. If any prerequisite ends in a non-SUCCESSterminal state (FAILURE/ABORTED/TIMED_OUT), the dependent fails without running (all_successsemantics). Failure propagates transitively down the DAG (a failed dependent is itself non-SUCCESS, so its dependents fail in turn — no explicit cascade walk). Needed by a later step (contribution-totals ordering) and broadly useful for any multi-step task workflow.Data model. New
task_dependenciesjunction table (M:N edgestask_id → depends_on_task_id, both FKsON DELETE CASCADE, unique per pair).Task.dependenciesexposes the prerequisiteTaskentities via a self-referential many-to-many (selectin,viewonly), so the full prerequisites load in one fetch. Edges are written throughTaskDAO.add_dependency. Cleanup relies on the DB-level FK cascade becauseTaskPruneCommandbulk-deletes via a coreDELETE.Public API.
TaskOptions.depends_onaccepts scheduledTaskobjects (canonical),UUIDs, or UUID strings. Threaded through@task.schedule→TaskManager.submit_task→SubmitTaskCommand, which persists edges under the existing dedup lock with a cycle guard (rejects self-dependencies and transitive cycles).Scheduling (Model A, block-and-wait). All DAG tasks are enqueued up front. A dependent's worker stays
PENDINGand blocks on each prerequisite viaTaskManager.wait_for_completion, then runs iff all succeeded, else transitions toFAILUREnaming the failed prerequisite. Ops note (documented): deep/wide DAGs hold worker slots, so size the Celery fleet accordingly.REST API.
TaskResponseSchemaexposesdepends_onas[{uuid, task_name, status}](mirrors thesubscriberspattern).Task List UI. New chain-icon (
LinkOutlined) Dependencies column: a hover popover lists each prerequisite with its status icon, plus a "waiting on N prerequisites" indicator for aPENDINGtask with unmet prerequisites.superset-core. Abstract
TaskDependencymodel +TaskOptions.depends_onadded to the extension SDK surface.BEFORE/AFTER
Task List gains a "Dependencies" column (chain icon → popover of prerequisites + statuses; "waiting on N" for blocked pending tasks). No change to existing task behavior when a task has no dependencies.
TESTING INSTRUCTIONS
tests/unit_tests/tasks/test_dependencies.py(scheduler gate, cycle guard, edge normalization incl. Task entities),test_decorators.py(depends_onmerge + forwarding).tests/integration_tests/tasks/commands/test_submit.py(edge persistence via Task entity, unknown-prerequisite rejection),tests/integration_tests/tasks/api_tests.py(depends_onserialization).superset-frontend/src/pages/TaskList/TaskList.test.tsx(chain icon, popover, "waiting on N"). All 7 pass locally.docs/developer_docs/extensions/tasks.md→ new Task Dependencies section.Performance — DB round-trips
Round-trips are kept minimal (in the spirit of the GTF query-count work):
find_by_uuidsresolve + one bulkadd_dependenciesinsert (independent of N)selectinper page (batched across rows), not per rowselectin-loaded with the task (the common FIFO case)Notes:
selectinrelations) when onlystatusis needed. Redis Pub/Sub is documented as at-most-once / fire-and-forget (messages "forever lost" on disconnect), which is why the DB re-check exists as a backstop. The follow-up will move completion/abort signalling onto Redis Streams (persisted, at-least-once — Redis's own recommendation for stronger delivery) with a blockingXREAD, making it genuinely event-driven and removing the busy-poll, plus switching the fallback poll to a status-only batched read. Tracked separately since it changes the shared coordination primitive.ADDITIONAL INFORMATION
GLOBAL_TASK_FRAMEWORKtask_dependencies)depends_on)Note: the whole-project frontend type-check has pre-existing failures unrelated to this change (
FoldersEditor,softDeleteCopy, etc.).