Taragolis commented on code in PR #35244:
URL: https://github.com/apache/airflow/pull/35244#discussion_r1382568165
##########
airflow/utils/db.py:
##########
@@ -235,7 +235,7 @@ def create_default_connections(session: Session =
NEW_SESSION):
merge_conn(
Connection(
conn_id="druid_ingest_default",
- conn_type="druid",
+ conn_type="druid_ingest",
Review Comment:
Unfortunetly this not affect already existed connection ID
##########
airflow/utils/db.py:
##########
@@ -225,7 +225,7 @@ def create_default_connections(session: Session =
NEW_SESSION):
merge_conn(
Connection(
conn_id="druid_broker_default",
- conn_type="druid",
+ conn_type="druid_broker",
Review Comment:
Unfortunetly this not affect already existed connection ID
##########
airflow/providers/apache/druid/hooks/druid.py:
##########
@@ -163,8 +167,8 @@ class DruidDbApiHook(DbApiHook):
conn_name_attr = "druid_broker_conn_id"
default_conn_name = "druid_broker_default"
- conn_type = "druid"
- hook_name = "Druid"
+ conn_type = "druid_broker"
Review Comment:
I assume that this might be a breaking changes. Because if you rename
`conn_type` then previously created connection IDs will keep `conn_type=druid`
as result Airflow UI have no idea how to deal with this connection type
And fallback to the first available Connection type into the list
So we need to write down information into the change log how to resolve this
situation.
##########
docs/apache-airflow-providers-apache-druid/connections/index.rst:
##########
@@ -0,0 +1,31 @@
+ .. Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ .. http://www.apache.org/licenses/LICENSE-2.0
+
+ .. Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+Connection Types
+================
+
+Apache Airflow implemented connection to Apache Druid in different ways that
listed here:
+ * Connection that uses the druid ingestion type that makes it possible to
submit index/ingestion jobs.
+ To use the ingestion job, choose ``Druid Ingest Connection``
+ * Connection that uses to Query data using SQL.
+ To Query data use ``Druid Broker Connection``
+
Review Comment:
Looks kind of hard to read in additional to repeatable bullet lists
If the difference mentioned into the connections, maybe better remove
section and keep only toctree? WDYT @eladkal @blcksrx
--
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
