Skip to content

Commit ff782ff

Browse files
committed
Preserve _HiveClient.__init__ transport setup; reinit on reuse via flag
1 parent a2b9c3f commit ff782ff

2 files changed

Lines changed: 12 additions & 31 deletions

File tree

pyiceberg/catalog/hive.py

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,11 @@
142142

143143

144144
class _HiveClient:
145-
"""Helper class to nicely open and close the transport.
145+
"""Helper class to nicely open and close the transport."""
146146

147-
``TSaslClientTransport`` instances are single-use: ``close()`` disposes
148-
the SASL session and the same instance cannot be reopened. The
149-
transport is therefore created lazily in ``__enter__`` rather than held
150-
on the class, which lets the ``_HiveClient`` itself live for the
151-
duration of a ``HiveCatalog`` while still honouring the SASL transport
152-
lifecycle.
153-
"""
154-
155-
_transport: "TTransport | None"
147+
_transport: TTransport
156148
_ugi: list[str] | None
149+
_was_opened: bool
157150

158151
def __init__(
159152
self,
@@ -166,7 +159,8 @@ def __init__(
166159
self._kerberos_auth = kerberos_auth
167160
self._kerberos_service_name = kerberos_service_name
168161
self._ugi = ugi.split(":") if ugi else None
169-
self._transport = None
162+
self._transport = self._init_thrift_transport()
163+
self._was_opened = False
170164

171165
def _init_thrift_transport(self) -> TTransport:
172166
url_parts = urlparse(self._uri)
@@ -177,28 +171,23 @@ def _init_thrift_transport(self) -> TTransport:
177171
return TTransport.TSaslClientTransport(socket, host=url_parts.hostname, service=self._kerberos_service_name)
178172

179173
def _client(self) -> Client:
180-
assert self._transport is not None # set by __enter__
181174
protocol = TBinaryProtocol.TBinaryProtocol(self._transport)
182175
client = Client(protocol)
183176
if self._ugi:
184177
client.set_ugi(*self._ugi)
185178
return client
186179

187180
def __enter__(self) -> Client:
188-
"""Initialize and open a fresh transport for this entry.
189-
190-
A new ``TSaslClientTransport`` is created on every entry because
191-
the underlying SASL session cannot be reused after ``close()``.
192-
Reusing a closed transport would leave a half-opened TCP socket
193-
and SASL handshake EOF noise on the server.
194-
"""
195-
self._transport = self._init_thrift_transport()
181+
"""Make sure the transport is initialized and open."""
182+
if self._was_opened:
183+
self._transport = self._init_thrift_transport()
196184
self._transport.open()
185+
self._was_opened = True
197186
return self._client()
198187

199188
def __exit__(self, exctype: type[BaseException] | None, excinst: BaseException | None, exctb: TracebackType | None) -> None:
200189
"""Close transport if it was opened."""
201-
if self._transport is not None and self._transport.isOpen():
190+
if self._transport.isOpen():
202191
self._transport.close()
203192

204193

tests/catalog/test_hive.py

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,22 +1412,14 @@ def test_create_hive_client_with_kerberos_using_context_manager(
14121412
assert open_client._iprot.trans.isOpen()
14131413

14141414

1415-
def test_kerberized_client_uses_fresh_transport_per_entry(
1415+
def test_kerberized_client_uses_fresh_transport_on_reuse(
14161416
kerberized_hive_metastore_fake_url: str,
14171417
) -> None:
1418-
"""TSaslClientTransport is single-use — each entry must create a new one.
1419-
1420-
The underlying SASL session cannot be reused after close(). Each
1421-
context-manager entry must therefore produce a fresh transport
1422-
instance rather than reopening the previous one (which would leave
1423-
a half-opened TCP socket and SASL handshake EOF noise on the server).
1424-
"""
1418+
"""Reusing the context manager must reinitialize the transport."""
14251419
client = _HiveClient(
14261420
uri=kerberized_hive_metastore_fake_url,
14271421
kerberos_auth=True,
14281422
)
1429-
# No transport should exist until the context manager is entered.
1430-
assert client._transport is None
14311423
with (
14321424
patch(
14331425
"puresasl.mechanisms.kerberos.authGSSClientStep",

0 commit comments

Comments
 (0)