zach-overflow commented on code in PR #73116:
URL: https://github.com/apache/airflow/pull/73116#discussion_r4122839343
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -1336,18 +1336,19 @@ def __repr__(self):
return repr(self.detail)
-class ServerResponseError(httpx.HTTPStatusError):
- def __init__(self, message: str, *, request: httpx.Request, response:
httpx.Response):
+class ServerResponseError(httpx2.HTTPStatusError):
Review Comment:
I had to do a bit of digging on this one, but my understanding is that
`Client` and `ServerResponseError` are components of the Task sdk's
_internals_. Neither of those classes are listed in the [public API
reference](https://airflow.apache.org/docs/task-sdk/stable/api.html), and
`airflow.sdk.api` is explicitly excluded from the generated docs. During normal
task execution, the supervisor converts those `ServerResponseError` failures
into `AirflowRuntimeError` in the task process before returning an error
response.
The `devel-common` helpers do need to support both stacks because the compat
jobs run against released SDK versions. That’s handled by conditionally
selecting the right `httpx*` package which the installed `Client` subclasses.
I’d prefer to keep the SDK itself consistently on `httpx2` if possible. Is
there a supported use case for directly constructing `Client` or catching its
HTTP exceptions which I’m missing? If so, maybe we should account for that
explicitly? A conditional import alone wouldn’t preserve existing `httpx`
exception catches or make its transports compatible.
--
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]
