Add druid ingestion connection test

[jaegwonseo]

expose druid connection and implement test

closes: #33795

[potiuk]

I think better approach woudl be to keep one of the connections (DBApi ?) as "druid". It will at least make existing DBApi connections work.

[potiuk]

Also - this one woll need a changelog entry describing what changed and how users should modify their connections

[jaegwonseo]

I think better approach woudl be to keep one of the connections (DBApi ?) as "druid". It will at least make existing DBApi connections work.

Also - this one woll need a changelog entry describing what changed and how users should modify their connections

@potiuk
In order to execute queries within a distributed system, Druid utilizes broker nodes, and for data ingestion, it leverages ingest nodes. Since each node has distinct roles and invocation addresses, maintaining the current setup with these two separate nodes would be preferable.
and I do not know the exact criteria for releasing new versions in the airflow provider, so I assumed 3.6.0 for now and added the change log.

[potiuk]

In order to execute queries within a distributed system, Druid utilizes broker nodes, and for data ingestion, it leverages ingest nodes. Since each node has distinct roles and invocation addresses, maintaining the current setup with these two separate nodes would be preferable.

I see. Ok then it should be a breaking change - but we let release manager decide on that - entry in changeloge should not contain the version (see suggestion).

[eladkal]

Left some comments.

We need also to have connections.rst in druid provider.
See example in other provider:
https://github.com/apache/airflow/blob/main/docs/apache-airflow-providers-apache-hdfs/connections.rst

Related to #28790

[eladkal]

Please elaborate on this one. We should explain to users how to mitigate the issue (leave instructions how to migrate to this new version)

[jaegwonseo]

@eladkal
I'm not an English speaker, so I'm not sure if the following mention is accurate. Please review this

The connection type of Druid has been separated into two(Druid Broker, Druid Ingest). Please perform one of the two methods listed below.
  1. update druid connect type to (Druid Broker, Druid Ingest) on UI screen (Admin/Connections)
  2. run command airflow db migrate

but in second case, airflow db migrate doesn't support update connect type so we need implement this.
my suggestion is write first option only, and add an implementation to check the version of the Druid provider in the db migrate process. how about do this in other issue

[Taragolis]

Nope, we don't have connection migrations, and with current release model (that doesn't mean it is bad) I don't think it even possible to implement it. All migration happen only on upgrade Airflow Core, not Providers.

Just some examples of the past breaking changes and their descriptions:

1. Remove AWS S3 Connection (spoiler alert, it does impact some users)
   https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/changelog.html#id66

2. Removed deprecated fields in Slack Provider (still wait when users starts complain)
   https://airflow.apache.org/docs/apache-airflow-providers-slack/stable/changelog.html#breaking-changes

3. Deprecated parameter removed from Google Provider
   https://airflow.apache.org/docs/apache-airflow-providers-google/stable/changelog.html#breaking-changes

The main point of this breaking changes. When user open issue/discussion in Github or write in slack it much easier to send them link to Breaking Changes, rather then try to figure out how to solve their issue.

[jaegwonseo]

@eladkal @Taragolis
Taragolis thanks for adding example link
in this pr the only concern point is changing connection type. So, I think this mention is enough.

In this version of provider Apache Druid Connection(conn_type="druid") has been separated into two(Druid Broker, Druid Ingest)
Update connect type(conn_type = "druid") to druid broker, druid ingest 

[eladkal]

The goal of the note is to explain what actions needed to be done in order to migrate.
The note you listed says the conn is split in two so:

1. How do I choose which one? (Or/And what is the motivation to the split)
2. If I want to have my code work exactly as it is before what steps I need to do?

These are the two questions the notes must answer

[eladkal]

It's almost ready :)

[hussein-awala]

One of the main reasons for connection and test_connection method is storing the credentials securely and test them via this method, so IMHO this method should test if the credentials are valid or not. You can try something like:

Suggested change

	response = requests.get(f"http://{host}:{port}/druid/indexer/v1/tasks")
	response = requests.get(
	f"http://{host}:{port}/druid/indexer/v1/tasks",
	auth=self.get_auth(),
	)

[jaegwonseo]

@hussein-awala
The Druid Broker Hook overrides the DbApiHook, but the test connection is not implemented to use authentication information
https://github.com/apache/airflow/blob/main/airflow/providers/common/sql/hooks/sql.py#L550

So, I think adding authentication information to the Druid hook (broker, ingest) is handled in another issue

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.