o-nikolas commented on code in PR #27583:
URL: https://github.com/apache/airflow/pull/27583#discussion_r1019743267
##########
airflow/providers/apache/druid/hooks/druid.py:
##########
@@ -82,12 +84,29 @@ def get_auth(self) -> requests.auth.HTTPBasicAuth | None:
else:
return None
+ def get_ssl_path(self) -> str | None:
+ """Get ssl path defined in extras from druid connection"""
+ conn = self.get_connection(self.druid_ingest_conn_id)
+ ssl_path = conn.extra_dejson.get("ssl_path", None)
Review Comment:
Is `None` an acceptable value to the `verify` arg on `requests.post(...)`?
If not, perhaps we should raise an exception at this point if the user is
trying to use ssl, but no path was provided.
##########
airflow/providers/apache/druid/hooks/druid.py:
##########
@@ -82,12 +84,29 @@ def get_auth(self) -> requests.auth.HTTPBasicAuth | None:
else:
return None
+ def get_ssl_path(self) -> str | None:
+ """Get ssl path defined in extras from druid connection"""
+ conn = self.get_connection(self.druid_ingest_conn_id)
+ ssl_path = conn.extra_dejson.get("ssl_path", None)
+ return ssl_path
+
def submit_indexing_job(self, json_index_spec: dict[str, Any] | str) ->
None:
"""Submit Druid ingestion job"""
url = self.get_conn_url()
self.log.info("Druid ingestion spec: %s", json_index_spec)
- req_index = requests.post(url, data=json_index_spec,
headers=self.header, auth=self.get_auth())
+
+ """Enable SSL Certification if needed."""
Review Comment:
```suggestion
# Enable SSL certification if requested.
```
##########
tests/providers/apache/druid/hooks/test_druid.py:
##########
@@ -148,13 +148,25 @@ def test_submit_timeout(self, m):
def test_get_conn_url(self, mock_get_connection):
get_conn_value = MagicMock()
get_conn_value.host = "test_host"
- get_conn_value.conn_type = "https"
+ get_conn_value.conn_type = "http"
get_conn_value.port = "1"
+ get_conn_value.schema = "https"
get_conn_value.extra_dejson = {"endpoint": "ingest"}
mock_get_connection.return_value = get_conn_value
hook = DruidHook(timeout=1, max_ingestion_time=5)
assert hook.get_conn_url() == "https://test_host:1/ingest";
+ @patch("airflow.providers.apache.druid.hooks.druid.DruidHook.get_ssl_path")
+ def test_get_ssl_path(self, mock_get_connection):
+ get_conn_value = MagicMock()
+ get_conn_value.host = "test_host"
+ get_conn_value.conn_type = "https"
Review Comment:
didn't you add the `schema` field for this?
##########
airflow/providers/apache/druid/hooks/druid.py:
##########
@@ -48,13 +48,15 @@ def __init__(
druid_ingest_conn_id: str = "druid_ingest_default",
timeout: int = 1,
max_ingestion_time: int | None = None,
+ use_ssl: bool = False,
Review Comment:
Can you update the doc string above to include this new param?
--
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]
