jscheffl commented on code in PR #41414:
URL: https://github.com/apache/airflow/pull/41414#discussion_r1715616252
##########
airflow/api_internal/endpoints/rpc_api_endpoint.py:
##########
@@ -237,7 +237,8 @@ def internal_airflow_api(body: dict[str, Any]) ->
APIResponse:
response = json.dumps(output_json) if output_json is not None else
None
log.info("Sending response: %s", response)
return Response(response=response, headers={"Content-Type":
"application/json"})
- except AirflowException as e: # In case of AirflowException transport the
exception class back to caller
+ # In case of AirflowException or other selective known types, transport
the exception class back to caller
+ except (KeyError, AttributeError, AirflowException) as e:
Review Comment:
We need to transport backend Exceptions to the API client and before we
decided to go for all `AirflowException` types. All other (before this PR)
exceptions are generating an HTTP 500 Internal Server error when raising.
Unfortunately the Variable class which needs to be placed behind the
internal API uses `KeyError` and `AttributeError` on the surface and (1) we
don't want to introduce a breaking change to all consumers and (2) we don't
want to just mask these exceptions in the backend and behaviour would change.
So we need to transport (at least) these two additional exception types via the
internal APIs. It is not a random selection but exactly cut to the needs we
(minimal) need to have. All other exceptions again raise a HTTP 500 behind the
API.
If there are further methods needed for functional cases an Exception (have
not seen more until now) the list might need to be extended.
--
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]
