kaxil commented on code in PR #70298:
URL: https://github.com/apache/airflow/pull/70298#discussion_r3993088684
##########
dev/registry/extract_parameters.py:
##########
@@ -391,28 +392,138 @@ def load_resumable_job_mixin() -> type | None:
return None
+# Matches an actual self.defer() call or self.deferrable attribute read, but
not
+# self.defer_for_approval(). TaskDeferred catches operators that raise it
directly instead of
Review Comment:
Widening this before you pick a fix, because a `defer_for_approval`-shaped
fix does not close it.
The same dead-branch shape is in the standard provider.
`HITLOperator.execute` raises `TaskAwaitingInput` under `if
AIRFLOW_V_3_3_PLUS:` (`operators/hitl.py:234-241`) and falls through to
`self.defer(trigger=HITLTrigger(...))` at :244 only on cores below 3.3.
`airflow.providers.standard.operators.hitl` is a registered operators module
(`provider.yaml:100`), so all four public classes are in the dataset.
Running HEAD's `supports_deferrable` directly against the installed
providers, with `AIRFLOW_V_3_3_PLUS` imported from
`airflow.providers.standard.version_compat` evaluating True:
```
supports_deferrable=True HITLOperator
supports_deferrable=True ApprovalOperator
supports_deferrable=True HITLBranchOperator
supports_deferrable=True HITLEntryOperator
supports_deferrable=True LLMOperator
supports_deferrable=True LLMBranchOperator
supports_deferrable=True LLMSQLQueryOperator
supports_deferrable=True LLMSchemaCompareOperator
supports_deferrable=True LLMFileAnalysisOperator
```
Nine classes across two providers, all claiming Deferrable off a branch that
cannot execute where the extractor runs.
The standard four are why a name-based exclusion does not close this: their
`self.defer(` is in the class's own `execute` source, so
`_DEFERRAL_TOKEN_RE.search(source)` matches on the first line of
`_references_deferral`, before any walking happens. Whatever you land on has to
reason about the branch, not about the name of the call site.
Two landings, and I would take the first:
1. Do not count a `self.defer(` that is only reachable on a core below the
one the extractor is running on. Both sites sit behind an
`AIRFLOW_V_3_3_PLUS`-style flag, so this is checkable rather than heuristic,
and the registry generates each page against a single core anyway.
2. Or decide the badge means "defers on some supported core" and keep all
nine, in which case the tooltip has to stop saying another Triggerer picks the
work up, because on 3.3+ nothing does: the task parks in `AWAITING_INPUT` and
the Core API response handler or the scheduler timeout sweep resumes it.
Either way `test_defer_for_approval_does_not_false_match` still needs the
stub fixed, and a second case pinning the HITL shape would keep this from
coming back.
--
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]
