Vamsi-klu commented on code in PR #68144:
URL: https://github.com/apache/airflow/pull/68144#discussion_r3369963673
##########
providers/apache/hive/docs/connections/hive_cli.rst:
##########
@@ -80,9 +80,36 @@ High Availability (optional)
Ssl (optional)
Specify as ``True`` to enable SSL for your high availability connection.
+Transport Mode (optional)
+ Specify the JDBC ``transportMode`` parameter. Supported values are
``binary`` and ``http``.
Review Comment:
You're right that it was inconsistent with the other params. I removed the
special-cased `transport_mode` connection extra (and this doc entry) —
`transportMode` now goes through `jdbc_params` like any other parameter.
---
Drafted-by: Claude Code (Opus 4.8) (no human review before posting)
##########
providers/apache/hive/tests/unit/apache/hive/hooks/test_hive.py:
##########
@@ -619,6 +630,89 @@ def test_drop_partition(self, get_metastore_client_mock,
table_exist_mock):
assert metastore_mock.drop_partition(self.table, db=self.database,
part_vals=[DEFAULT_DATE_DS]), ret
+class TestHiveCliHookJdbcParams:
+ @mock.patch.object(HiveCliHook, "get_connection")
+ def test_jdbc_params_append_to_beeline_url(self, mock_get_connection):
+ mock_get_connection.return_value = get_hive_cli_connection()
+
+ hook = HiveCliHook(
+ jdbc_params={
+ "transportMode": "http",
+ "sslTrustStore": "/opt/hive/truststore.jks",
+ "trustStorePassword": "secret=ok",
+ }
+ )
+
+ assert hook._prepare_cli_cmd() == [
+ "beeline",
+ "-u",
+ '"jdbc:hive2://localhost:10000/default;'
+
'transportMode=http;sslTrustStore=/opt/hive/truststore.jks;trustStorePassword=secret=ok"',
+ ]
+
+ @mock.patch.object(HiveCliHook, "get_connection")
+ def test_jdbc_params_compose_with_auth_login_and_password(self,
mock_get_connection):
+ mock_get_connection.return_value =
get_hive_cli_connection(login="user", password="password")
+
+ hook = HiveCliHook(auth="LDAP", jdbc_params={"transportMode": "http"})
+
+ assert hook._prepare_cli_cmd() == [
+ "beeline",
+ "-u",
+
'"jdbc:hive2://localhost:10000/default;auth=LDAP;transportMode=http"',
+ "-n",
+ "user",
+ "-p",
+ "password",
+ ]
+
+ @mock.patch.object(HiveCliHook, "get_connection")
+ def test_connection_extra_and_hook_jdbc_params_compose_predictably(self,
mock_get_connection):
+ mock_get_connection.return_value =
get_hive_cli_connection(extra_dejson={"transport_mode": "binary"})
+
+ hook = HiveCliHook(
+ jdbc_params={
+ "transportMode": "http",
+ "sslTrustStore": "/opt/hive/truststore.jks",
+ }
+ )
+
+ assert hook._prepare_cli_cmd() == [
+ "beeline",
+ "-u",
+
'"jdbc:hive2://localhost:10000/default;transportMode=http;sslTrustStore=/opt/hive/truststore.jks"',
+ ]
+
+ @pytest.mark.parametrize(
+ ("jdbc_params", "message"),
+ [
+ ({"transportMode;ssl": "http"}, "JDBC parameter names"),
+ ({"transportMode=ssl": "http"}, "JDBC parameter names"),
+ ({"transport Mode": "http"}, "JDBC parameter names"),
+ ({"transportMode": "http;ssl=true"}, "JDBC parameter
'transportMode' value"),
+ ({"transportMode": None}, "JDBC parameter 'transportMode' value"),
+ ],
+ )
+ @mock.patch.object(HiveCliHook, "get_connection")
+ def test_invalid_jdbc_params_are_rejected(self, mock_get_connection,
jdbc_params, message):
+ mock_get_connection.return_value = get_hive_cli_connection()
+ hook = HiveCliHook(jdbc_params=jdbc_params)
+
+ with pytest.raises(ValueError, match=message):
+ hook._prepare_cli_cmd()
+
+ @pytest.mark.parametrize("transport_mode", ["invalid", "http;ssl=true"])
+ @mock.patch.object(HiveCliHook, "get_connection")
+ def test_invalid_transport_mode_connection_extra_is_rejected(self,
mock_get_connection, transport_mode):
+ mock_get_connection.return_value = get_hive_cli_connection(
+ extra_dejson={"transport_mode": transport_mode}
+ )
+ hook = HiveCliHook()
+
+ with pytest.raises(ValueError, match="transport_mode connection
extra"):
+ hook._prepare_cli_cmd()
Review Comment:
It's covered by `test_jdbc_params_append_to_beeline_url` (plus
`test_jdbc_params_compose_with_auth_login_and_password` and
`test_hook_jdbc_params_append_in_order`), which assert the fully assembled
Beeline `-u` URL. I've kept those as the valid-case assertions.
---
Drafted-by: Claude Code (Opus 4.8) (no human review before posting)
--
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]
