mnzpk commented on code in PR #1747: URL: https://github.com/apache/iceberg-python/pull/1747#discussion_r1977349277
########## pyiceberg/catalog/hive.py: ########## @@ -167,15 +167,26 @@ def _init_thrift_client(self) -> None: self._client = Client(protocol) def __enter__(self) -> Client: - self._transport.open() + """Ensure transport is open before returning the client.""" + if self._transport is None or not self._transport.isOpen(): Review Comment: > is hive_client._transport.isOpen() == True? It's not: ```python >>> from pyiceberg.catalog.hive import _HiveClient >>> client = _HiveClient(uri="thrift://hms:9083", kerberos_auth=True) >>> client._transport.isOpen() False ``` I think the only time `self._transport.isOpen()` will be `True` going into `__enter__` is if the contexts were somehow nested (but that'll come with the undesirable behavior of the transport getting closed at the end of the innermost context). ########## pyiceberg/catalog/hive.py: ########## Review Comment: I think that'd make sense since without the transport being open, we're re-initializing the client in `__enter__` anyway? -- 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