med9110 opened a new pull request, #74867:
URL: https://github.com/apache/airflow/pull/74867
<h2 data-pm-slice="0 0 []">Summary</h2><p>Fix incorrect error classification
in <code dir="ltr">SageMakerTrigger</code> when an AWS waiter exhausts its
attempts because credentials are unavailable.</p><h3>Problem</h3><p>PR #73870
introduced <code dir="ltr">WaiterNoCredentialsError</code> to distinguish AWS
credential failures from ordinary waiter exhaustion.</p><p>However, <code
dir="ltr">WaiterNoCredentialsError</code> inherits from <code
dir="ltr">WaiterMaxAttemptsError</code>.</p><p>In <code
dir="ltr">SageMakerTrigger._event_from_exception()</code>, the existing
exception handling checks:</p><pre data-composer-code-block="" dir="ltr"><code
class="hljs" spellcheck="false">if isinstance(error, WaiterMaxAttemptsError):
return TriggerEvent(
{
"status": "timeout",
"job_name": self.job_name,
"message": str(error),
}
)</code></pre><p>Because <code dir="ltr">isinstance()</code> also
matches subclasses, a <code dir="ltr">WaiterNoCredentialsError</code> enters
this branch and is incorrectly reported as a timeout.</p><p><strong>Current
behavior:</strong></p><ul data-spread="false"><li><p>AWS credentials are
missing during every waiter attempt.</p></li><li><p>The waiter raises <code
dir="ltr">WaiterNoCredentialsError</code>.</p></li><li><p><code
dir="ltr">SageMakerTrigger</code> emits a <code dir="ltr">TriggerEvent</code>
with <code dir="ltr">status="timeout"</code>.</p></li></ul><p><strong>Expected
behavior:</strong></p><ul data-spread="false"><li><p>Credential-only exhaustion
is reported with <code dir="ltr">status="error"</code>.</p></li><li><p>Ordinary
waiter exhaustion continues to produce <code
dir="ltr">status="timeout"</code>.</p></li></ul><p>This distinction matters
because missing credentials indicate a configuration or authentication problem,
not a job that simply exceeded its waitin
g period.</p><h3>Fix</h3><p>Handle <code
dir="ltr">WaiterNoCredentialsError</code> before the broader <code
dir="ltr">WaiterMaxAttemptsError</code> check:</p><pre
data-composer-code-block="" dir="ltr"><code class="hljs" spellcheck="false">if
isinstance(error, WaiterNoCredentialsError):
return super()._event_from_exception(error)</code></pre><p>Delegating to
<code dir="ltr">AwsBaseWaiterTrigger</code> reuses its existing error handling
rather than introducing duplicate logic.</p><p>The existing SageMaker-specific
handling for ordinary waiter timeouts and terminal failures remains
unchanged.</p><h3>Tests</h3><p>Added regression coverage at two levels:</p><ul
data-spread="false"><li><p><strong>Exception classification:</strong> Verify
that <code dir="ltr">WaiterNoCredentialsError</code> produces an error
event.</p></li><li><p><strong>Asynchronous trigger execution:</strong> Exercise
<code dir="ltr">SageMakerTrigger.run()</code> with mocked waiter failures,
including missing credentials, ordinary exhaustion, terminal failure, and
unexpected exceptions.</p></li></ul><h3>Verification</h3><p>Tested against the
Apache Airflow source checkout with Python 3.12.</p>
Check | Result
-- | --
Regression tests before the fix | 2 expected failures, 6 passed
Focused tests after the fix | 8 passed
SageMaker trigger and waiter tests | 46 passed
Ruff 0.16.8 lint | Passed
Ruff 0.16.8 formatting | Passed
<p>All AWS interactions were mocked; no live AWS infrastructure was used.
Full provider CI has not been run locally.</p><h3>Impact</h3><p>The change
improves failure classification and diagnostics without changing the existing
task failure behavior. The affected SageMaker operators already treat both
error and timeout events as failures.</p><p>Related to #73870.</p>
--
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]
