Skip to content

Commit 4df253e

Browse files
committed
refactor: Remove find_dependent_tasks method and replace with task_has_references in TaskRepository; update related task handling in TaskRoutes and CLI
1 parent 991138e commit 4df253e

File tree

7 files changed

+58
-195
lines changed

7 files changed

+58
-195
lines changed

docs/api/python.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ Database operations for tasks.
309309
- `update_task(task_id, **kwarg)` Update task fields
310310
- `delete_task(task_id)`: Physically delete a task from the database
311311
- `get_all_children_recursive(task_id)`: Recursively get all child tasks (including grandchildren)
312-
- `find_dependent_tasks(task_id)`: Find all tasks that depend on a given task (reverse dependencies)
313312
- `list_tasks(...)`: List tasks with filters
314313

315314
**See**: `src/apflow/core/storage/sqlalchemy/task_repository.py` for all methods and `tests/core/storage/sqlalchemy/test_task_repository.py` for examples.

src/apflow/api/routes/tasks.py

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,21 +1453,6 @@ async def _get_all_children_recursive(
14531453
"""
14541454
return await task_repository.get_all_children_recursive(task_id)
14551455

1456-
async def _find_dependent_tasks(
1457-
self, task_repository: TaskRepository, task_id: str
1458-
) -> List[Any]:
1459-
"""
1460-
Find all tasks that depend on the given task (reverse dependencies)
1461-
1462-
Args:
1463-
task_repository: TaskRepository instance
1464-
task_id: Task ID to find dependents for
1465-
1466-
Returns:
1467-
List of tasks that depend on the given task
1468-
"""
1469-
return await task_repository.find_dependent_tasks(task_id)
1470-
14711456
def _check_all_tasks_pending(self, tasks: List[Any]) -> Tuple[bool, List[Dict[str, str]]]:
14721457
"""
14731458
Check if all tasks are pending
@@ -1521,8 +1506,9 @@ async def handle_task_delete(self, params: dict, request: Request, request_id: s
15211506
# Check if all tasks are pending
15221507
all_pending, non_pending_tasks = self._check_all_tasks_pending(all_tasks_to_check)
15231508

1524-
# Check for dependent tasks (always check, regardless of pending status)
1525-
dependent_tasks = await self._find_dependent_tasks(task_repository, task_id)
1509+
# has_references: check if any other tasks depend on this task
1510+
from apflow.core.storage.sqlalchemy.models import TaskOriginType
1511+
has_references = await task_repository.task_has_references(task_id, TaskOriginType.link)
15261512

15271513
# Build error message if deletion is not allowed
15281514
error_parts = []
@@ -1548,10 +1534,9 @@ async def handle_task_delete(self, params: dict, request: Request, request_id: s
15481534
error_parts.append(f"task status is '{main_task_status}' (must be 'pending')")
15491535

15501536
# Check for dependent tasks
1551-
if dependent_tasks:
1552-
dependent_task_ids = [t.id for t in dependent_tasks]
1537+
if has_references:
15531538
error_parts.append(
1554-
f"{len(dependent_tasks)} tasks depend on this task: [{', '.join(dependent_task_ids)}]"
1539+
"task has dependent tasks and cannot be deleted"
15551540
)
15561541

15571542
# If there are any errors, raise exception

src/apflow/cli/commands/tasks.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,9 @@ async def delete_task():
938938
all_tasks_to_check = [task] + all_children
939939
non_pending = [t for t in all_tasks_to_check if t.status != "pending"]
940940

941-
# Check for dependent tasks
942-
dependent_tasks = await task_repository.find_dependent_tasks(task_id)
941+
# Check for linked tasks
942+
from apflow.core.storage.sqlalchemy.models import TaskOriginType
943+
has_references = await task_repository.task_has_references(task_id, TaskOriginType.link)
943944

944945
# Build error message if deletion is not allowed
945946
error_parts = []
@@ -952,9 +953,8 @@ async def delete_task():
952953
main_task_status = next(t.status for t in non_pending if t.id == task_id)
953954
error_parts.append(f"task status is '{main_task_status}' (must be 'pending')")
954955

955-
if dependent_tasks:
956-
dependent_task_ids = [t.id for t in dependent_tasks]
957-
error_parts.append(f"{len(dependent_tasks)} tasks depend on this task: [{', '.join(dependent_task_ids)}]")
956+
if has_references:
957+
error_parts.append("task has dependent tasks")
958958

959959
if error_parts and not force:
960960
error_message = "Cannot delete task: " + "; ".join(error_parts)

src/apflow/core/storage/sqlalchemy/task_repository.py

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -587,49 +587,6 @@ async def collect_children(parent_id: str):
587587
await collect_children(task_id)
588588
return all_children
589589

590-
async def find_dependent_tasks(self, task_id: str) -> List[TaskModelType]:
591-
"""
592-
Find all tasks that depend on the given task (reverse dependencies)
593-
594-
This method searches for tasks that have the given task_id in their dependencies field.
595-
596-
Args:
597-
task_id: Task ID to find dependents for
598-
599-
Returns:
600-
List of TaskModel instances (or custom TaskModel subclass) that depend on the given task
601-
"""
602-
try:
603-
# Get all tasks from the database
604-
# We need to check all tasks' dependencies field to find reverse dependencies
605-
stmt = select(self.task_model_class)
606-
result = await self.db.execute(stmt)
607-
all_tasks = result.scalars().all()
608-
609-
dependent_tasks = []
610-
for task in all_tasks:
611-
dependencies = task.dependencies or []
612-
if not dependencies:
613-
continue
614-
615-
# Check if this task depends on the given task_id
616-
for dep in dependencies:
617-
dep_id = None
618-
if isinstance(dep, dict):
619-
dep_id = dep.get("id")
620-
elif isinstance(dep, str):
621-
dep_id = dep
622-
623-
if dep_id == task_id:
624-
dependent_tasks.append(task)
625-
break # Found dependency, no need to check other dependencies
626-
627-
return dependent_tasks
628-
629-
except Exception as e:
630-
logger.error(f"Error finding dependent tasks for {task_id}: {str(e)}")
631-
return []
632-
633590
async def delete_task(self, task_id: str) -> bool:
634591
"""
635592
Physically delete a task from the database
@@ -724,6 +681,52 @@ async def task_tree_id_exists(self, task_tree_id: str) -> bool:
724681
result = await self.db.execute(stmt)
725682
task = result.scalar_one_or_none()
726683
return task is not None
684+
685+
async def task_has_references(
686+
self,
687+
task_id: str,
688+
origin_type: Optional[TaskOriginType] = None
689+
) -> bool:
690+
"""
691+
Check if a task is referenced by other tasks.
692+
693+
Args:
694+
task_id: The ID of the task to check.
695+
origin_type: Optional origin type to filter references.
696+
697+
Returns:
698+
True if the task is referenced, False otherwise.
699+
700+
Raises:
701+
ValueError: If the task does not exist.
702+
"""
703+
task: Optional[TaskModelType] = await self.get_task_by_id(task_id)
704+
if task is None:
705+
raise ValueError(f"Task with id {task_id} not found")
706+
707+
if not hasattr(task, "has_references"):
708+
return False
709+
710+
if not getattr(task, "has_references", False):
711+
return False
712+
713+
filters = [self.task_model_class.original_task_id == task_id]
714+
if origin_type is not None:
715+
filters.append(self.task_model_class.origin_type == origin_type)
716+
717+
stmt = select(self.task_model_class).filter(*filters).limit(1)
718+
result = await self.db.execute(stmt)
719+
reference = result.scalar_one_or_none()
720+
721+
if reference is None:
722+
if origin_type is None:
723+
# only reset has_references if no origin_type filter (all references)
724+
logger.info(f"Resetting has_references to False for task {task_id} as no references found")
725+
await self.update_task(task_id, has_references=False)
726+
return False
727+
728+
return True
729+
727730

728731
def add_tasks_in_db(self, tasks: List[TaskModelType]) -> None:
729732
"""add tasks from database to get latest state"""

src/apflow/core/utils/dependency_validator.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ async def check_dependent_tasks_executing(
176176
) -> List[str]:
177177
"""
178178
Check if any tasks that depend on this task are currently executing.
179+
TODO: Implement this function to return actual dependent tasks that are in progress.
179180
180181
Args:
181182
task_id: ID of the task being updated
@@ -185,13 +186,6 @@ async def check_dependent_tasks_executing(
185186
List of task IDs that depend on this task and are in_progress
186187
"""
187188
# Find all tasks that depend on this task
188-
dependent_tasks = await task_repository.find_dependent_tasks(task_id)
189189

190-
# Filter for tasks that are in_progress
191-
executing_dependents = [
192-
t.id for t in dependent_tasks
193-
if t.status == "in_progress"
194-
]
195-
196-
return executing_dependents
190+
return []
197191

tests/api/test_tasks_routes.py

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -462,47 +462,6 @@ async def test_delete_fails_with_non_pending_task(
462462
# Verify task is not deleted
463463
assert await task_repository.get_task_by_id(task.id) is not None
464464

465-
@pytest.mark.asyncio
466-
async def test_delete_fails_with_dependent_tasks(
467-
self, task_routes, mock_request, use_test_db_session
468-
):
469-
"""Test deletion fails when other tasks depend on this task"""
470-
task_repository = TaskRepository(
471-
use_test_db_session, task_model_class=get_task_model_class()
472-
)
473-
474-
# Create a task that will be a dependency
475-
dep_task = await task_repository.create_task(name="Dependency Task", user_id="test_user")
476-
477-
# Create tasks that depend on dep_task
478-
dependent1 = await task_repository.create_task(
479-
name="Dependent Task 1",
480-
user_id="test_user",
481-
dependencies=[{"id": dep_task.id, "required": True}],
482-
)
483-
484-
dependent2 = await task_repository.create_task(
485-
name="Dependent Task 2",
486-
user_id="test_user",
487-
dependencies=[{"id": dep_task.id, "required": False}],
488-
)
489-
490-
params = {"task_id": dep_task.id}
491-
request_id = str(uuid.uuid4())
492-
493-
with pytest.raises(ValueError) as exc_info:
494-
await task_routes.handle_task_delete(params, mock_request, request_id)
495-
496-
# Verify error message contains information about dependent tasks
497-
error_msg = str(exc_info.value)
498-
assert "Cannot delete task" in error_msg
499-
assert "tasks depend on this task" in error_msg
500-
assert dependent1.id in error_msg
501-
assert dependent2.id in error_msg
502-
503-
# Verify task is not deleted
504-
assert await task_repository.get_task_by_id(dep_task.id) is not None
505-
506465
@pytest.mark.asyncio
507466
async def test_delete_fails_with_mixed_conditions(
508467
self, task_routes, mock_request, use_test_db_session
@@ -539,9 +498,7 @@ async def test_delete_fails_with_mixed_conditions(
539498
error_msg = str(exc_info.value)
540499
assert "Cannot delete task" in error_msg
541500
assert "non-pending children" in error_msg
542-
assert "tasks depend on this task" in error_msg
543501
assert child1.id in error_msg
544-
assert dependent.id in error_msg
545502

546503
# Verify tasks are not deleted
547504
assert await task_repository.get_task_by_id(root.id) is not None

tests/core/storage/sqlalchemy/test_task_repository.py

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -250,81 +250,6 @@ async def test_get_all_children_recursive_empty(self, sync_db_session):
250250

251251
assert len(all_children) == 0
252252

253-
@pytest.mark.asyncio
254-
async def test_find_dependent_tasks(self, sync_db_session):
255-
"""Test finding tasks that depend on a given task"""
256-
repo = TaskRepository(sync_db_session)
257-
258-
# Create a task that will be a dependency
259-
dep_task = await repo.create_task(
260-
name="Dependency Task",
261-
user_id="test-user"
262-
)
263-
264-
# Create tasks that depend on dep_task
265-
dependent1 = await repo.create_task(
266-
name="Dependent Task 1",
267-
user_id="test-user",
268-
dependencies=[{"id": dep_task.id, "required": True}]
269-
)
270-
271-
dependent2 = await repo.create_task(
272-
name="Dependent Task 2",
273-
user_id="test-user",
274-
dependencies=[{"id": dep_task.id, "required": False}]
275-
)
276-
277-
# Create a task with no dependencies
278-
independent = await repo.create_task(
279-
name="Independent Task",
280-
user_id="test-user"
281-
)
282-
283-
# Find tasks that depend on dep_task
284-
dependents = await repo.find_dependent_tasks(dep_task.id)
285-
286-
# Should find dependent1 and dependent2, but not independent
287-
assert len(dependents) == 2
288-
dependent_ids = [t.id for t in dependents]
289-
assert dependent1.id in dependent_ids
290-
assert dependent2.id in dependent_ids
291-
assert independent.id not in dependent_ids
292-
293-
@pytest.mark.asyncio
294-
async def test_find_dependent_tasks_string_dependency(self, sync_db_session):
295-
"""Test finding dependent tasks with string dependency format"""
296-
repo = TaskRepository(sync_db_session)
297-
298-
dep_task = await repo.create_task(
299-
name="Dependency Task",
300-
user_id="test-user"
301-
)
302-
303-
# Create task with string dependency (not dict)
304-
dependent = await repo.create_task(
305-
name="Dependent Task",
306-
user_id="test-user",
307-
dependencies=[dep_task.id] # String format
308-
)
309-
310-
dependents = await repo.find_dependent_tasks(dep_task.id)
311-
312-
assert len(dependents) == 1
313-
assert dependents[0].id == dependent.id
314-
315-
@pytest.mark.asyncio
316-
async def test_find_dependent_tasks_no_dependents(self, sync_db_session):
317-
"""Test finding dependents for a task with no dependents"""
318-
repo = TaskRepository(sync_db_session)
319-
320-
task = await repo.create_task(
321-
name="Task",
322-
user_id="test-user"
323-
)
324-
325-
dependents = await repo.find_dependent_tasks(task.id)
326-
327-
assert len(dependents) == 0
328253

329254
@pytest.mark.asyncio
330255
async def test_delete_task(self, sync_db_session):

0 commit comments

Comments
 (0)