SEPURI-SAI-KRISHNA opened a new pull request, #70430:
URL: https://github.com/apache/airflow/pull/70430

   `CloudComposerExecutionTrigger.run()` checked `operation.done` before 
`operation.error`, in an `if`/`elif`. Per the [long-running operation 
contract](https://cloud.google.com/composer/docs/reference/rest/v1/projects.locations.operations),
 "if `done` == `false`, neither `error` nor `response` is set", and a finished 
operation has `done=True` with exactly one of `error` or `response` populated.
   
   So the two branches were inverted relative to the API:
   
   - the `elif operation.error.message` branch was only ever reached while 
`done` was `false` — the one state where `error` is guaranteed unset, making it 
dead code;
   - a genuinely failed operation (`done=True` with `error` set) broke out of 
the loop and yielded `operation_done: True`, the success shape.
   
   The consequence differs per operator, all three of which defer with this 
trigger:
   
   | Operator | `execute_complete` | Result when the operation failed |
   |---|---|---|
   | `CloudComposerUpdateEnvironmentOperator` | returns on `operation_done`, no 
state check | task marked **success** |
   | `CloudComposerDeleteEnvironmentOperator` | `pass` | task marked 
**success** |
   | `CloudComposerCreateEnvironmentOperator` | re-fetches the environment and 
validates its state | task fails, but with a misleading `NotFound`/wrong-state 
error rather than the GCP failure reason |
   
   This also made the deferrable path disagree with the synchronous one: with 
`deferrable=False` the operators call `hook.wait_for_operation()` → 
`operation.result()`, which raises on a failed operation.
   
   Checking `error` inside the `done` branch fixes all three. The existing 
`raise` is relocated, not new.
   
   `CloudComposerExecutionTrigger` had no test coverage, so this adds two 
cases: a finished-with-error operation must raise, and a finished-without-error 
operation must still yield `operation_done: True`. The first fails without this 
change; the second guards the new nested condition against over-raising on 
success.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Opus 5)
   
   Generated-by: Claude Code (Opus 5) following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   


-- 
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]

Reply via email to