Copilot commented on code in PR #64119:
URL: https://github.com/apache/airflow/pull/64119#discussion_r3066492529
##########
providers/airbyte/src/airflow/providers/airbyte/operators/airbyte.py:
##########
@@ -127,7 +127,7 @@ def execute_complete(self, context: Context, event: Any =
None) -> None:
"""
if event["status"] == "error":
self.log.debug("Error occurred with context: %s", context)
- raise AirflowException(event["message"])
+ raise RuntimeError(event["message"])
Review Comment:
`execute_complete` defaults `event` to `None`, but immediately subscripts
`event[...]`, which will raise `TypeError`/`KeyError` if the trigger sends
`None` or an unexpected payload. Consider validating the event shape up front
(e.g., explicit check for missing/invalid `event` and raising a clear
task-failing exception) so failures are actionable and consistent.
##########
providers/airbyte/tests/unit/airbyte/operators/test_airbyte.py:
##########
@@ -67,6 +68,52 @@ def test_execute(self, mock_wait_for_job,
mock_submit_sync_connection, create_co
job_id=self.job_id, wait_seconds=self.wait_seconds,
timeout=self.timeout
)
+ @pytest.mark.parametrize("status", ["success", "cancelled"])
+ def test_execute_complete_non_error_states(self, status,
create_connection_without_db):
Review Comment:
The test currently treats `status == \"cancelled\"` as a non-error outcome,
but in non-deferrable/inline execution `CANCELLED` results in an exception. If
the trigger can emit `\"cancelled\"` as a status, `execute_complete` would
incorrectly report success and the test would lock in that mismatch. Consider
either (a) updating `execute_complete` to treat cancelled/failed statuses as
errors (aligning with `execute()`), or (b) adjusting the trigger contract/tests
so cancellation is delivered as an `\"error\"` event.
--
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]
