topherinternational commented on issue #45237:
URL: https://github.com/apache/airflow/issues/45237#issuecomment-2573054118
> The issue is in this method, it pops all non related extra_options
parameters and keeps the rest to pass it as headers, an extra pop should be
needed on the check_response key to avoid the issue, I'll open a PR to fix that:
>
>
>
> ```
>
> def _configure_session_from_extra(
>
> self, session: requests.Session, connection: Connection
>
> ) -> requests.Session:
>
> extra = connection.extra_dejson
>
> extra.pop("timeout", None)
>
> extra.pop("allow_redirects", None)
>
> extra.pop("check_response", None) # this should be added
>
> session.proxies = extra.pop("proxies", extra.pop("proxy", {}))
>
> session.stream = extra.pop("stream", False)
>
> session.verify = extra.pop("verify", extra.pop("verify_ssl",
True))
>
> session.cert = extra.pop("cert", None)
>
> session.max_redirects = extra.pop("max_redirects",
DEFAULT_REDIRECT_LIMIT)
>
> session.trust_env = extra.pop("trust_env", True)
>
> try:
>
> session.headers.update(extra)
>
> except TypeError:
>
> self.log.warning("Connection to %s has invalid extra
field.", connection.host)
>
> return session
>
> ```
This is the call of someone in the core contributor club, so I'm just giving
my opinion -
This does look like a bug in the HTTP hook, but it seems like a separate
issue to this issue. I see the two breaking down like this:
- When I use the `HttpOperator`, I want to tell it to skip any status
checking and let me check status through the existing `response_check` callback
argument
- When I declare a connection to use with the HttpHook, I want to specify
that the hook should not do a status check on any call made with this connection
Using the hook-connection contract to configure the control flow of the
operator perpetuates the API leaking problem that spawned this issue in the
first place.
It's a separate issue/question whether the hook should check it status based
on an option in the connection object, or if that behavior should always be
configured by the caller (args to the constructor or run()). I lean towards yes
but that can be discussed in a new issue filing.
--
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]
