Taragolis commented on code in PR #35712:
URL: https://github.com/apache/airflow/pull/35712#discussion_r1397953807
##########
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:
> I just checked and we use http connection type (not airbyte)
The main point here "you use", don't forget about other users or some one
who create Connection from the UI or use JSON connection. Some of the
connections also use additional fields, which not provided by HTTP connection,
in additional HTTP connection provide additional configuration thought Extra
field
> easier connection creation for new users
URI not the only one way to define connection, and to be honest not he
easiest way to create a connection, due to requirements of decode it
> 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
And we return to initial comment
https://github.com/apache/airflow/pull/35712#pullrequestreview-1737927952
This mentioned into the documentation. and maybe better to move it into the
[HTTP
operator](https://airflow.apache.org/docs/apache-airflow-providers-http/stable/operators.html#httpoperator),
maybe better move or duplicate it into the Connection.
> 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 action is required to create additional Hook for handle this "feature".
--
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]
