zohaibfast99 commented on issue #72144: URL: https://github.com/apache/airflow/issues/72144#issuecomment-5554710761
Thanks! That's clear. I'll leave the two data-quality sensors out; they need the structural change in #72171, and keeping this to the two Shape A sites is what makes it reviewable on its own. Happy to rebase if the merge order flips. On Sat, 5 Sept 2026 at 18:46, Sepuri Sai Krishna ***@***.***> wrote: > *SEPURI-SAI-KRISHNA* left a comment (apache/airflow#72144) > <https://github.com/apache/airflow/issues/72144#issuecomment-5552235780> > > On the ECS rows, you're right, and the "operator-side fix only" label on > them is wrong. > > Both triggers do take **kwargs and forward to AwsBaseWaiterTrigger, so > verify and botocore_config serialize and round-trip fine. But each one > then overrides hook(): > > # triggers/ecs.py on main, lines 47-48 and 87-88def hook(self) -> AwsGenericHook: > return EcsHook(aws_conn_id=self.aws_conn_id, region_name=self.region_name) > > So the parameters arrive on the trigger and are dropped when the hook is > built. Passing them at the call site alone changes nothing observable. > Those two rows belong under Shape C, not Shape B, I grouped the table by > whether the trigger's __init__ accepts the parameters, when the question > that actually matters is whether hook() consumes them. I'll correct the > table. > > Your caveat is right too: #72171 > <https://github.com/apache/airflow/pull/72171> already covers both. It > sets aws_hook_class = EcsHook on ClusterActiveTrigger and > ClusterInactiveTrigger and deletes the hook() override, so the base class > builds the hook from all three serialized parameters. > ------------------------------ > > On glue, a heads-up so a reviewer doesn't meet it cold: #72557 > <https://github.com/apache/airflow/pull/72557> and #72171 > <https://github.com/apache/airflow/pull/72171> overlap on the same lines. > #72171 <https://github.com/apache/airflow/pull/72171> touches > operators/glue.py and sensors/glue.py for the two Shape A sites you > picked up, and also widens triggers/glue.py. > > Worth knowing that the two Shape A sites aren't all of glue. > GlueDataQualityRuleSetEvaluationRunSensor and > GlueDataQualityRuleRecommendationRunSensor also defer, and their triggers > are Shape C, closed __init__ signatures that never accept the three > parameters, so those two can't be fixed at the call site and aren't covered > by #72557 <https://github.com/apache/airflow/pull/72557>. > > Your PR is smaller and self-contained, so I'm happy for it to land first; > I'll rebase #72171 <https://github.com/apache/airflow/pull/72171> and > drop the four overlapping lines. Nothing needed from you, just flagging the > overlap. > > Separately, and not aimed at anyone in particular: I've added a status > block at the top of the issue. Between #72171 > <https://github.com/apache/airflow/pull/72171>, #72449 > <https://github.com/apache/airflow/pull/72449> and #72472 > <https://github.com/apache/airflow/pull/72472> the sites in those tables > are now all spoken for, so if you are reading this and thinking of picking > up a service, please comment here first. I would much rather make room for > you than have you write something that ends up closed as a duplicate. > ------------------------------ > > Drafted-by: Claude Code (Opus 5); reviewed by @SEPURI-SAI-KRISHNA > <https://github.com/SEPURI-SAI-KRISHNA> before posting > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/airflow/issues/72144?email_source=notifications&email_token=A52Z7NQOAJMLYDOUIHDIHM35NQKNHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJVGIZDGNJXHAYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5552235780>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/A52Z7NRKSNUO6CG2OCBLFLT5NQKNHAVCNFSNUABEKJSXA33TNF2G64TZHMZTGOBYGQ4DSMJ3JFZXG5LFHM2TENRWG42DQNBRG2QXMAQ> > . > Triage notifications, keep track of coding agent tasks and review pull > requests on the go with GitHub Mobile for iOS > <https://github.com/notifications/mobile/ios/A52Z7NWYTHR2BLM7XF5YNVL5NQKNHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJVGIZDGNJXHAYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJKTGN5XXIZLSL5UW64Y> > and Android > <https://github.com/notifications/mobile/android/A52Z7NRUD4TNLV36FCM2HCT5NQKNHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJVGIZDGNJXHAYKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. > Download it today! > You are receiving this because you commented.Message ID: > ***@***.***> > -- 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]
