Repository navigation
Pass verify and botocore_config to GlueJobCompleteTrigger - #72557
Merged
Merged
Conversation
boring-cyborg
Bot
added
area:providers
provider:amazon
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
2 tasks done
potiuk
approved these changes
Sep 9, 2026
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
SEPURI-SAI-KRISHNA
added a commit
to SEPURI-SAI-KRISHNA/airflow
that referenced
this pull request
Sep 10, 2026
Merging main brought in apache#72557, which passes verify and botocore_config to GlueJobCompleteTrigger at the same two call sites this branch already widened. Git combined both insertions without reporting a conflict, leaving each call with the arguments repeated, which Python rejects at import time.
imrichardwu
pushed a commit
to imrichardwu/airflow
that referenced
this pull request
Sep 11, 2026
potiuk
added a commit
that referenced
this pull request
Sep 21, 2026
* Use the operator's AWS settings for deferred Neptune, MWAA, SSM tasks An AwsBaseOperator/AwsBaseSensor subclass resolves region_name, verify and botocore_config in __init__, but did not hand them to the trigger it defers to. The trigger builds its own hook, so the deferred half of the task reached AWS with the default region, SSL verification silently re-enabled, and any custom botocore timeouts or retries discarded. The triggers already accept all three, so only the call sites were missing. * Document the verify parameter for Neptune Analytics operators Every Neptune Analytics operator accepts `verify` through the shared AWS base class, but none of the seven class docstrings mentioned it, so the rendered provider docs gave users no way to discover it. Two of those docstrings even carried a stray blank line where the entry belonged. The deferral tests now compare the trigger's serialized payload rather than its attributes. Serialization is what actually crosses into the triggerer process, and it passes values through `prune_dict`, so an attribute-level assertion can pass while the setting is silently dropped on the way there. This matches the assertion style already used for the Neptune cluster operators. * Build deferred AWS hooks from the operator's own settings An AWS operator always carries region_name, verify and botocore_config, but on deferral the trigger builds its own hook. Most triggers accepted none of those parameters and constructed the hook from aws_conn_id alone, so the triggerer silently fell back to boto3 defaults: a different region, default SSL verification, and none of the configured timeouts or retry policy. The task changed behaviour purely by virtue of deferring, and did so without any error. Fixing this service by service would have meant editing every trigger signature as well as every call site, so the hook is now built in one place from the parameters the base trigger already serializes. Subclasses name the hook they need instead of constructing it, which is the same arrangement the operators use. The accompanying invariant test walks every defer site in the provider and fails if one does not hand its hook configuration to the trigger, so an operator added later cannot reintroduce the gap unnoticed. Three services are deliberately left for the Contributors Workshop and are named in the test's allowlist rather than skipped silently. * Drop the duplicated Glue trigger arguments after merging main Merging main brought in #72557, which passes verify and botocore_config to GlueJobCompleteTrigger at the same two call sites this branch already widened. Git combined both insertions without reporting a conflict, leaving each call with the arguments repeated, which Python rejects at import time. * Fail fast when an AWS trigger cannot build its hook hook() is only reached from run(), which executes in the triggerer, so a subclass declaring neither aws_hook_class nor its own hook() would defer successfully and fail later, out of sight of the task that deferred. The operator side already gets this guarantee from validate_attributes. Checked on class creation rather than in __init__ because subclasses such as EksDeleteClusterTrigger never call super().__init__(). Also records the behaviour change in the provider changelog, since the triggerer now uses the operator's region, SSL verification and botocore configuration rather than falling back to boto3 defaults. * Treat an unresolvable trigger expression as unreadable The sweep resolved a trigger= expression to the constructions it can evaluate to and returned an empty list when it could not read one. A bare reference was caught separately by matching ast.Name, but an attribute, a subscript, a conditional with one unreadable branch, or a construction whose callee cannot be named all produced nothing the checks could act on, and nothing reported their absence. The conditional was the worst of them: it still yielded the readable branch, so the site appeared in the parametrized run and read as covered. The directory filter had the same shape of problem, silently ignoring any defer site that did not sit directly in operators/ or sensors/. Reported in review by a contributor on the pull request. * Resolve the trigger class name where the construction is accepted The MyPy providers job is the only red check on the branch. The sweep looked up a construction's class name separately from deciding that the construction was readable, so the name stayed optional at every use even though an unnameable callee is already rejected, and the conditional branch walk could not be narrowed. Pairing each construction with the name that made it readable removes the optionality instead of asserting it away. * Update providers/amazon/tests/unit/amazon/aws/test_deferred_hook_configuration.py * Keep the deferred-hook warning in the pending changelog slot The 2026-09-09 provider release cut 9.36.0 and moved the Comprehend warning under that heading. Merging main carries this branch's warning down with it, attributing a change 9.36.0 does not contain to a released version. Put it back above the first version heading so it lands in the release that actually ships it. Generated-by: Claude Opus 5 --------- Co-authored-by: Niko Oliveira <onikolas@apache.org> Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #72144. Covers the two "Shape A" call sites from that issue's table.
GlueJobOperatorandGlueJobSensorareAwsBaseOperator/AwsBaseSensorsubclasses, so they always carryregion_name,verifyandbotocore_config. Both forward onlyregion_namewhen they defer, so the triggerer rebuilds its hook with default SSL verification and default botocore timeouts/retries. A deployment that setsverify=False, pointsverifyat a private CA bundle, or tunesbotocore_configgets those settings on the synchronous path and silently loses them the moment the task defers.GlueJobCompleteTriggeralready accepts all three parameters, and itshook()already forwards all three intoGlueJobHook, so this is a call-site-only fix and is complete end to end as it stands. It does not depend on #72171.related: #72144
Tests
One regression test per call site. Both assert on
trigger.serialize()[1]rather than on trigger attributes, since the serialized payload is what actually reaches the triggerer process.verify=Falseis used deliberately, as a falsy value is the one most likely to be dropped by mistake.Verified both tests fail without the source change (
KeyError: 'verify') and pass with it. Full Glue operator/sensor/trigger suites pass (141 passed).A possible correction to the ECS rows in #72144
While tracing this I think the issue's table may be slightly off for ECS. It lists
EcsCreateClusterOperatorandEcsDeleteClusterOperatorunder "trigger takes**kwargs; operator-side fix only", butClusterActiveTrigger.hook()andClusterInactiveTrigger.hook()are:The
**kwargsdo reachAwsBaseWaiterTrigger, so the two parameters would serialize correctly, but the bespokehook()builds the hook without them. An operator-side-only change there would look correct and serialize correctly while still polling with defaultverify/botocore_config. Those two sites look like they also need theirhook()widened, or need #72171 to land first. Happy to open a separate issue if that's useful.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code following the guidelines