SEPURI-SAI-KRISHNA commented on issue #72144:
URL: https://github.com/apache/airflow/issues/72144#issuecomment-5443098667
Please do, and thanks for picking them up. I've pulled those three back out
of my branch, so the PR I open won't touch them.
To make the sub-issues easier to write, one thing worth knowing: those three
are not the same size, because two of the triggers already accept the
parameters and only the call site is wrong.
**`sensors/batch.py:101` -> `BatchJobTrigger`**, call site only.
`BatchJobTrigger.__init__` takes `**kwargs` and forwards to
`AwsBaseWaiterTrigger`, and `operators/batch.py:242` already passes all three
today. The sensor just passes `region_name` and stops. Add `verify=self.verify`
and `botocore_config=self.botocore_config` and it's done.
**`operators/sagemaker_unified_studio_notebook.py:192` ->
`SageMakerUnifiedStudioNotebookTrigger`**, call site only, same reason. That
trigger also takes `**kwargs`, and its `hook()` already reads all three off
`self`; they're just never populated because the operator doesn't send them.
Three lines at the call site.
**`sensors/opensearch_serverless.py:116` ->
`OpenSearchServerlessCollectionActiveTrigger`**, the full shape, and the better
teaching example. The trigger's `__init__` is closed (no `**kwargs`), and its
`hook()` is `OpenSearchServerlessHook(aws_conn_id=self.aws_conn_id)`, so it
discards the configuration even if you pass it. This one needs the signature
widened, `aws_hook_class = OpenSearchServerlessHook` set, the bespoke `hook()`
deleted, and then the call site fixed.
So if you want an ordering: batch and sagemaker are genuinely a first-PR
size, opensearch_serverless is the one that shows the whole pattern.
On the mechanism side, the PR carries an allowlist in the invariant test:
```python
PENDING_MIGRATION = frozenset(
{
("operators/sagemaker_unified_studio_notebook.py",
"SageMakerUnifiedStudioNotebookTrigger"),
("sensors/batch.py", "BatchJobTrigger"),
("sensors/opensearch_serverless.py",
"OpenSearchServerlessCollectionActiveTrigger"),
}
)
```
Each task is then: make the change, delete your line, watch the test go
green. The test asserts that an entry is still *needed*, so a stale line fails
the suite rather than quietly masking a site, the allowlist can't outlive the
work it tracks. That should keep the three sub-issues independent of each other
and of my PR.
I'll link the PR here once it's up.
---
Drafted-by: Claude Code (Opus 5); reviewed by @SEPURI-SAI-KRISHNA before
posting
--
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]
