mnzpk commented on code in PR #1747: URL: https://github.com/apache/iceberg-python/pull/1747#discussion_r2052355193
########## pyiceberg/catalog/hive.py: ########## @@ -143,40 +144,47 @@ class _HiveClient: """Helper class to nicely open and close the transport.""" _transport: TTransport - _client: Client _ugi: Optional[List[str]] def __init__(self, uri: str, ugi: Optional[str] = None, kerberos_auth: Optional[bool] = HIVE_KERBEROS_AUTH_DEFAULT): self._uri = uri self._kerberos_auth = kerberos_auth self._ugi = ugi.split(":") if ugi else None + self._transport = self._init_thrift_transport() - self._init_thrift_client() - - def _init_thrift_client(self) -> None: + def _init_thrift_transport(self) -> TTransport: url_parts = urlparse(self._uri) - socket = TSocket.TSocket(url_parts.hostname, url_parts.port) - if not self._kerberos_auth: - self._transport = TTransport.TBufferedTransport(socket) + return TTransport.TBufferedTransport(socket) else: - self._transport = TTransport.TSaslClientTransport(socket, host=url_parts.hostname, service="hive") + return TTransport.TSaslClientTransport(socket, host=url_parts.hostname, service="hive") + @cached_property + def _client(self) -> Client: protocol = TBinaryProtocol.TBinaryProtocol(self._transport) - - self._client = Client(protocol) + client = Client(protocol) + if self._ugi: + client.set_ugi(*self._ugi) + return client def __enter__(self) -> Client: - self._transport.open() - if self._ugi: - self._client.set_ugi(*self._ugi) + """Make sure the transport is initialized and open.""" + if not self._transport.isOpen(): + try: + self._transport.open() + except TTransport.TTransportException: + # reinitialize _transport + self._transport = self._init_thrift_transport() Review Comment: We're re-initializing the transport but since `self._client` is a `cached_property`, it'd still point to the old (and now closed) transport so I think we'd also want to `del`ete `self._client` so that it gets re-created? ########## pyiceberg/catalog/hive.py: ########## @@ -143,40 +144,47 @@ class _HiveClient: """Helper class to nicely open and close the transport.""" _transport: TTransport - _client: Client _ugi: Optional[List[str]] def __init__(self, uri: str, ugi: Optional[str] = None, kerberos_auth: Optional[bool] = HIVE_KERBEROS_AUTH_DEFAULT): self._uri = uri self._kerberos_auth = kerberos_auth self._ugi = ugi.split(":") if ugi else None + self._transport = self._init_thrift_transport() - self._init_thrift_client() - - def _init_thrift_client(self) -> None: + def _init_thrift_transport(self) -> TTransport: url_parts = urlparse(self._uri) - socket = TSocket.TSocket(url_parts.hostname, url_parts.port) - if not self._kerberos_auth: - self._transport = TTransport.TBufferedTransport(socket) + return TTransport.TBufferedTransport(socket) else: - self._transport = TTransport.TSaslClientTransport(socket, host=url_parts.hostname, service="hive") + return TTransport.TSaslClientTransport(socket, host=url_parts.hostname, service="hive") + @cached_property + def _client(self) -> Client: protocol = TBinaryProtocol.TBinaryProtocol(self._transport) - - self._client = Client(protocol) + client = Client(protocol) + if self._ugi: + client.set_ugi(*self._ugi) + return client def __enter__(self) -> Client: - self._transport.open() - if self._ugi: - self._client.set_ugi(*self._ugi) + """Make sure the transport is initialized and open.""" + if not self._transport.isOpen(): + try: + self._transport.open() Review Comment: It seems that we're still going to try re-opening the transport once it has been closed and since the exception raised [in that case would be a `TypeError`](https://github.com/apache/iceberg-python/issues/1744#issue-2887097609), it would not be caught by the `except` below. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org