[GitHub] [airflow] youngyjd commented on a change in pull request #5066: [AIRFLOW-4262] Ensure SlackWebhookHook can take either http_conn_id or webhook_token, or both

[GitBox <gi...@apache.org>]

youngyjd commented on a change in pull request #5066: [AIRFLOW-4262] Ensure SlackWebhookHook can take either http_conn_id or webhook_token, or both
URL: https://github.com/apache/airflow/pull/5066#discussion_r273250884
 
 

 ##########
 File path: airflow/hooks/http_hook.py
 ##########
 @@ -57,25 +57,26 @@ def get_conn(self, headers=None):
         :param headers: additional headers to be passed through as a dictionary
         :type headers: dict
         """
-        conn = self.get_connection(self.http_conn_id)
         session = requests.Session()
-
-        if "://" in conn.host:
-            self.base_url = conn.host
-        else:
-            # schema defaults to HTTP
-            schema = conn.schema if conn.schema else "http"
-            self.base_url = schema + "://" + conn.host
-
-        if conn.port:
-            self.base_url = self.base_url + ":" + str(conn.port)
-        if conn.login:
-            session.auth = (conn.login, conn.password)
-        if conn.extra:
-            try:
-                session.headers.update(conn.extra_dejson)
-            except TypeError:
-                self.log.warn('Connection to %s has invalid extra field.', conn.host)
+        if self.http_conn_id:
+            conn = self.get_connection(self.http_conn_id)
 
 Review comment:
   If http_conn_id is provided but does not exist in Connection table, it will error out here: https://github.com/apache/airflow/blob/ab58eb67f63fa53b68bf50382f5eaf9303e3bdeb/airflow/hooks/base_hook.py#L58
   
   If user does not specify http_conn_id, `http_default` will be used https://github.com/apache/airflow/blob/ab58eb67f63fa53b68bf50382f5eaf9303e3bdeb/airflow/hooks/http_hook.py#L44. If user specifies `http_conn_id = None`, then `endpoint` must be the full path to make request work https://github.com/apache/airflow/blob/cc06fefce6b25ade0b04cc4ee20e51dd5b6d418b/airflow/hooks/http_hook.py#L84. 
   
   which means, either we provide full url path in `http_conn_id` or `endpoint`, or use `http_conn_id` as base_url and `endpoint` as relative path to make HttpHook work.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services