K3UL commented on a change in pull request #6145: [AIRFLOW-5518] Use correct 
scheme for Http connection parsed from URI
URL: https://github.com/apache/airflow/pull/6145#discussion_r325780382
 
 

 ##########
 File path: tests/models/test_connection.py
 ##########
 @@ -212,31 +212,31 @@ def test_connection_from_uri_with_path(self):
             (
                 "http://:password@host:80/database";,
                 ConnectionParts(
-                    conn_type="http", login='', password="password", 
host="host", port=80, schema="database"
+                    conn_type="http", login='', password="password", 
host="host", port=80, schema="http"
 
 Review comment:
   While I agree that schema does not equal scheme, it is what is used by the 
HttpHook, and it is where HTTP/HTTPS goes if you put it through the UI.
   As you mentioned in the Jira, this should be refactored deeper, but IMO this 
fix brings to the parser a behavior that is consistent with what happens 
through the UI.
   
   The alternative is to edit the HttpHook to change `schema = conn.schema if 
conn.schema else "http"` with `schema = conn.schema or conn.conn_type or 
'http'`.
   I'm open to it if you think it's better, but I think it's worse, as it 
invents a new Connection type (https) that is not referenced in `_types`.

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to