Nataneljpwd commented on code in PR #51893:
URL: https://github.com/apache/airflow/pull/51893#discussion_r2156692368


##########
providers/http/src/airflow/providers/http/hooks/http.py:
##########
@@ -48,37 +49,50 @@ def _url_from_endpoint(base_url: str | None, endpoint: str 
| None) -> str:
     return (base_url or "") + (endpoint or "")
 
 
-def _process_extra_options_from_connection(conn: Connection, extra_options: 
dict[str, Any]) -> dict:
-    extra = conn.extra_dejson
-    stream = extra.pop("stream", None)
-    cert = extra.pop("cert", None)
-    proxies = extra.pop("proxies", extra.pop("proxy", None))
-    timeout = extra.pop("timeout", None)
-    verify_ssl = extra.pop("verify", extra.pop("verify_ssl", None))
-    allow_redirects = extra.pop("allow_redirects", None)
-    max_redirects = extra.pop("max_redirects", None)
-    trust_env = extra.pop("trust_env", None)
-    check_response = extra.pop("check_response", None)
-
-    if stream is not None and "stream" not in extra_options:
-        extra_options["stream"] = stream
-    if cert is not None and "cert" not in extra_options:
-        extra_options["cert"] = cert
-    if proxies is not None and "proxy" not in extra_options:
-        extra_options["proxy"] = proxies
-    if timeout is not None and "timeout" not in extra_options:
-        extra_options["timeout"] = timeout
-    if verify_ssl is not None and "verify_ssl" not in extra_options:
-        extra_options["verify_ssl"] = verify_ssl
-    if allow_redirects is not None and "allow_redirects" not in extra_options:
-        extra_options["allow_redirects"] = allow_redirects
-    if max_redirects is not None and "max_redirects" not in extra_options:
-        extra_options["max_redirects"] = max_redirects
-    if trust_env is not None and "trust_env" not in extra_options:
-        extra_options["trust_env"] = trust_env
-    if check_response is not None and "check_response" not in extra_options:
-        extra_options["check_response"] = check_response
-    return extra
+def _process_extra_options_from_connection(
+    conn: Connection, extra_options: dict[str, Any]
+) -> tuple[dict[str, Any], dict[str, Any]]:
+    """
+    Return the updated extra options from the connection, as well as those 
passed.
+
+    :param conn: The HTTP Connection object passed to the Hook
+    :param extra_options: Use-defined extra options
+    :return: (tuple)
+    """
+    # Copy, to prevent changing conn.extra_dejson and extra_options
+    conn_extra_options: dict = copy.deepcopy(conn.extra_dejson)

Review Comment:
   I agree, a shallow copy will most likely work, however I do not see it 
becoming a significant performance issue as the object is quite small, though 
changing it to a shallow copy won't hurt as we only pop and don't modify the 
key values directly unless the nested dict is modified which will cause some 
troubles, and to avoid this issue it might be a good idea to keep it 



-- 
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]

Reply via email to