Bisk1 commented on code in PR #35712: URL: https://github.com/apache/airflow/pull/35712#discussion_r1397943452
########## airflow/providers/apache/livy/hooks/livy.py: ########## @@ -575,10 +575,11 @@ def _generate_base_url(self, conn: Connection) -> str: if conn.host and "://" in conn.host: base_url: str = conn.host else: - # schema defaults to HTTP - schema = conn.schema if conn.schema else "http" + scheme = conn.conn_type Review Comment: It could use connection in a different way, but this difference doesn't change the connection type. It so happens that my org also uses Airbyte hook - I just checked and we use `http` connection type (not `airbyte`) for AirbyteHook and it works perfectly fine. It was accidental but it actually makes sense to me - just because AirbyteHook has some custom logic on top of HTTP connection doesn't make this connection anything else than HTTP because that handling is a higher level of abstraction than connection. I can see a couple of benefits now: 1) The tactical one is that this change fixes a bug https://github.com/apache/airflow/issues/10913 where you can't correctly set path for HTTP connection if creating connection from Airflow URI because path from URI is used as protocol in the hook. In the process of fixing it, you can 2) More importantly - UX: a) easier connection creation for new users - even though Airflow URI in principle is not a valid HTTP(S) URI, users can expect if to be interpreted as such for HTTP connections - nothing in the URI syntax in docs suggests that connection to https://example.com should be defined http://example.com/https so it's completely counter-intuitive b) wouldn't it be easier for users to have fewer connection types to choose in the drop-down list? In practice all they need to choose for those cases is HTTP/HTTPS -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org