jroachgolf84 commented on code in PR #51893:
URL: https://github.com/apache/airflow/pull/51893#discussion_r2155795973
##########
providers/http/src/airflow/providers/http/hooks/http.py:
##########
@@ -194,21 +215,33 @@ def _extract_auth(self, connection: Connection) -> Any |
None:
return None
def _configure_session_from_extra(
- self, session: Session, connection: Connection, extra_options:
dict[str, Any] | None = None
+ self, session: Session, connection: Connection, extra_options:
dict[str, Any]
) -> Session:
- if extra_options is None:
- extra_options = {}
- headers = _process_extra_options_from_connection(connection,
extra_options)
- session.proxies = extra_options.pop("proxies",
extra_options.pop("proxy", {}))
- session.stream = extra_options.pop("stream", False)
- session.verify = extra_options.pop("verify",
extra_options.pop("verify_ssl", True))
- session.cert = extra_options.pop("cert", None)
- session.max_redirects = extra_options.pop("max_redirects",
DEFAULT_REDIRECT_LIMIT)
- session.trust_env = extra_options.pop("trust_env", True)
+ """
+ Configure the session using both the extra field from the Connection
and passed in extra_options.
+
+ :param session: (Session)
+ :param connection: HTTP Connection passed into Hook
+ :param extra_options: (dict)
+ :return: (Session)
+ """
+ # This is going to update self.merged_extra, which will be used below
+ conn_extra_options, self.merged_extra =
_process_extra_options_from_connection(
+ connection, extra_options
+ )
+
+ session.proxies = self.merged_extra.pop("proxies",
self.merged_extra.pop("proxy", {}))
Review Comment:
This is the reason that a change needed to be made. Previously, the original
`extra_options` that was being passed in was being manipulated such that when
the `run` method of the hook was called again, the dictionary did not maintain
its original key-value pairs. However, you do raise a good point.
`self.merged_extra` needs to be defined so that the merged dictionary can be
passed to the `run_and_check(...)` method when called in `run`? Why's that?
Changing the return value of `get_conn` would be a breaking change, hence the
need for an instance-level variable to be leveraged.
You do make a good point, and that is that I do not need to `pop` values
from `self.merged_extra`. I'll go ahead and plan on making that change.
--
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]
