amoghrajesh commented on code in PR #70298:
URL: https://github.com/apache/airflow/pull/70298#discussion_r3774733900
##########
dev/registry/extract_parameters.py:
##########
@@ -432,6 +433,11 @@ def is_durable_capable(cls: type, resumable_mixin: type |
None) -> bool:
return "execute_resumable" in source
+def supports_deferrable(cls: type) -> bool:
+ """Return True if a class exposes a `deferrable` constructor parameter."""
Review Comment:
Replaced the whole approach now. Instead of checking whether a deferrable
parameter exists anywhere in the class's ancestry, `supports_deferrable()` now
resolves execute via getattr (the same MRO-following resolution
is_durable_capable already uses) and checks that method's actual source for
`self.deferrable` or `self.defer`. That directly answers "does the code that
runs for this class use deferral" instead of proxying through whether a setting
merely exists somewhere in the hierarchy.
Verified against every class you named, and a couple more from the same
family, using the real code (not just fixtures): HttpOperator -> True,
DiscordWebhookOperator -> False (the false positive is gone),
DateTimeSensorAsync/TimeDeltaSensorAsync/AwaitMessageSensor/HITLOperator/MSGraphAsyncOperator/SSHRemoteJobOperator
-> all True. TimeDeltaSensorAsync in particular is now handled correctly
because it resolves through the real MRO rather than only checking each class's
own body and that's also why it doesn't fall into the same trap as the "grep
whole class body" option you ruled out.
`TriggerDagRunOperator` still can't be found by source grep alone, since its
`self.defer` call is one hop away inside `_trigger_dag_af_2`. Added it to a
small, explicitly commented `_DEFERRABLE_EXCEPTIONS` set rather than trying to
chase call graphs. Went with this over the full per-class opt-in marker you
mentioned, since the source grep now has zero known false positives and covers
everything except this one exception.
Added the Discord shape regression test you asked for, plus five more
covering each shape above. Ran it against every provider in the tree (1954
modules, zero errors) as a sanity check at scale.
--
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]
