brightwon commented on code in PR #3864:
URL: https://github.com/apache/iceberg-python/pull/3864#discussion_r3905365492
##########
pyiceberg/catalog/hive.py:
##########
@@ -316,6 +320,7 @@ def _create_hive_client(properties: dict[str, str]) ->
_HiveClient:
properties.get("ugi"),
property_as_bool(properties, HIVE_KERBEROS_AUTH,
HIVE_KERBEROS_AUTH_DEFAULT),
properties.get(HIVE_KERBEROS_SERVICE_NAME,
HIVE_KERBEROS_SERVICE_NAME_DEFAULT),
+ properties.get(HIVE_KERBEROS_SERVICE_HOST),
Review Comment:
@rambleraptor Thanks for the review! Good idea, I moved the branching out of
_init_thrift_transport.
([80571f7](https://github.com/apache/iceberg-python/pull/3864/commits/80571f786e5f96ae4f553e6087508a8e875d2d38))
I put the default in `_HiveClient.__init__` instead of in
`_create_hive_client`, because `_HiveClient` can also be constructed directly.
With the default only at the call site,
`test_create_hive_client_with_kerberos_using_context_manager` and
`test_kerberized_client_uses_fresh_transport_on_reuse` fail, since they
construct `_HiveClient` without a host.
Happy to move it to the call site if you prefer. That would just need those
two tests to pass the host explicitly.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
