dabla commented on issue #45237:
URL: https://github.com/apache/airflow/issues/45237#issuecomment-2564829415
> Just dropping some thoughts -
>
> I agree with the OP that this is unclear and confusing.
>
> It looks like `response_check` is designed to be a callback that examines
the text/bytes from a successful (2xx/3xx) response, and under normal use, the
hook will raise an exception when a 4xx/5xx status is received from the server.
This has an unfortunate name collision with `HttpHook`'s `check_response`,
which passes through to the `Response` object's `raise_for_status` method and
then massages a raised exception for 4xx/5xx responses (checking only status
code and not the response content).
>
> There's a lot of leaky API between the `HttpOperator` which wraps the
`HttpHook` which wraps the `requests` library, and the operator user/DAG writer
has to be aware of all of them to really understand how the operator is going
to behave. (And keep aware if those wrapped components change behavior.)
>
> One simple way around this particular issue might be to have the
HttpOperator take an optional argument like `raise_for_status` whose behavior
is to pass `{'check_response': False}` to the `HttpHook`, and document the
parameter and the `response_check` parameter better so it's clear how to use
them if you want to do what the OP does - interrogate the response code
yourself.
>
> A more fluent alternative might be to have the Operator take a
`check_status` parameter that is a callback function to examine the response
code, and if it is set, the operator code will (a) set the Hook to not do its
own status check, and (b) get called after the hook is run and before any
`response_check` function is called. That way `response_check` can preserve its
semantics of only getting called for a successful response, and DAG writer can
either use the hook's status check or substitute their own function (but never
both).
Completely agree with this detailed elaboration.
I would suggest that when a custom check_response callback is specified
within the operator, it would then in turn be passed through the HttpHook run
method so that it doesn’t use the default check_status. Also specifying the
check_response in the extra options didn’t work and only makes the useage
overly complicated for nothing.
--
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]
