codeant-ai-for-open-source[bot] commented on code in PR #43768:
URL: https://github.com/apache/superset/pull/43768#discussion_r3906193377
##########
superset/tasks/decorators.py:
##########
@@ -427,6 +429,132 @@ def _wait_for_existing_task(self, task: "Task", timeout:
int | None) -> "Task":
refreshed = TaskDAO.find_one_or_none(uuid=task.uuid,
skip_base_filter=True)
return refreshed if refreshed else task
+ def _gate_on_prerequisites(self, task: "Task") -> "Task | None":
+ """Block the inline caller until this task's prerequisites are
terminal.
+
+ Same ``all_success`` DAG semantics as the async path (shared via
+ :mod:`superset.tasks.dependencies`): ready โ returns ``None``
(proceed); a
+ prerequisite still running โ block on the coordination completion
signal
+ (the sync analogue of the async defer โ the caller *is* the executor
and
+ has no worker slot to free), then re-check; a prerequisite that ended
+ non-success โ fail this dependent and return its terminal ``Task`` for
the
+ caller to return.
+ """
+ from flask import current_app
+
+ from superset.daos.tasks import TaskDAO
+ from superset.tasks.dependencies import (
+ DAG_WAITING,
+ fail_dependent_on_unmet_prerequisite,
+ unmet_prerequisite,
+ )
+
+ try:
+ app = current_app._get_current_object() # noqa: SLF001
+ except RuntimeError:
+ app = None
+
+ # Re-read fresh so ``depends_on`` reflects committed edges (the
just-created
+ # task's selectin collection can be stale), matching the async path's
fresh
+ # worker load.
+ current = (
+ TaskDAO.find_one_or_none(uuid=task.uuid, skip_base_filter=True) or
task
+ )
+ unmet = unmet_prerequisite(current)
+ while unmet is DAG_WAITING:
+ for prerequisite in current.depends_on:
+ if prerequisite.status not in TERMINAL_STATES:
+ TaskManager.wait_for_completion(
+ task_uuid=prerequisite.uuid, poll_interval=1.0, app=app
+ )
+ current = (
Review Comment:
**Suggestion:** If one prerequisite has already failed while another remains
running, this loop waits for the running task instead of failing the dependent
immediately. [incorrect condition logic]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=260df4b0642c4cbbae61d9dabab2a684&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=260df4b0642c4cbbae61d9dabab2a684&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/decorators.py
**Line:** 465:470
**Comment:**
*Incorrect Condition Logic: If one prerequisite has already failed
while another remains running, this loop waits for the running task instead of
failing the dependent immediately.
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43768&comment_hash=7114d3772d7b23fd6c17f6e10d8b9a14914f84032964548f7e57ac9223ee53d8&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43768&comment_hash=7114d3772d7b23fd6c17f6e10d8b9a14914f84032964548f7e57ac9223ee53d8&reaction=dislike'>๐</a>
##########
superset/tasks/context.py:
##########
@@ -582,22 +610,64 @@ def _write_handler_failures_to_db(self) -> None:
stack_trace = handler_stack_trace
# Update task with combined error info. error_message is
public;
- # exception_type/stack_trace go under private["framework"]
(merged
- # recursively, so task/other framework handles are preserved).
- UpdateTaskCommand(
- self._task_uuid,
- status=TaskStatus.FAILURE.value,
- properties={
- "error_message": error_msg,
- "private": {
- "framework": {
- "exception_type": exception_type,
- "stack_trace": stack_trace,
- }
+ # exception_type/stack_trace go under private["framework"].
Merge
+ # onto the executor's property cache so the write preserves
runtime
+ # state instead of replacing the whole column.
+ failure_props = merge_properties(
+ self._properties_cache,
+ cast(
+ TaskProperties,
+ {
+ "error_message": error_msg,
+ "private": {
+ "framework": {
+ "exception_type": exception_type,
+ "stack_trace": stack_trace,
+ }
+ },
},
- },
- skip_security_check=True,
+ ),
+ )
+ # Conditional transition (like every other executor path): only
+ # flip to FAILURE from a non-terminal state. Cleanup runs in
the
+ # executor's ``finally`` โ *after* a SUCCESS commit โ so an
+ # unconditional write here would rewrite a committed terminal
+ # result and make a waiter discard a valid, successful payload.
+ transitioned = InternalStatusTransitionCommand(
+ task_uuid=self._task_uuid,
+ new_status=TaskStatus.FAILURE,
+ expected_status=[TaskStatus.IN_PROGRESS,
TaskStatus.ABORTING],
+ properties=failure_props,
+ set_ended_at=True,
).run()
+ if not transitioned:
+ # Task already reached a terminal state (e.g. SUCCESS
committed
+ # before cleanup ran). Preserve that status; record the
handler
+ # failure as debug-only detail so it isn't lost.
+ logger.warning(
+ "Handler failure for task %s after it reached a
terminal "
+ "state; recording detail without changing status: %s",
+ self._task_uuid,
+ error_msg,
+ )
+ InternalUpdateTaskCommand(
+ task_uuid=self._task_uuid,
+ properties=merge_properties(
Review Comment:
**Suggestion:** The failure transition writes error details without updating
`_task` or `_properties_cache`, so cleanup can overwrite the original task
exception with only handler-failure details. [stale reference]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0d457dd7882f467386e83fd2643fbc3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0d457dd7882f467386e83fd2643fbc3b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/tasks/context.py
**Line:** 655:655
**Comment:**
*Stale Reference: The failure transition writes error details without
updating `_task` or `_properties_cache`, so cleanup can overwrite the original
task exception with only handler-failure details.
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43768&comment_hash=2b43ce0e22a644509cd03c046718f1ef896f5564afa88ae73cf87d86e0d7f77b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43768&comment_hash=2b43ce0e22a644509cd03c046718f1ef896f5564afa88ae73cf87d86e0d7f77b&reaction=dislike'>๐</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
