kevinjqliu commented on code in PR #1747:
URL: https://github.com/apache/iceberg-python/pull/1747#discussion_r1976692000


##########
pyiceberg/catalog/hive.py:
##########


Review Comment:
   I wonder if we can just move `_init_thrift_client` out of the `__init__` and 
into the context manager's `__enter__`.
   



##########
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:
   if i just do 
   ```
   hive_client = _HiveClient()
   ```
   is `hive_client._transport.isOpen() == True`? 
   
   Otherwise, we'll always recreate the transport and client when using context 
manager. 
   
   



-- 
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

Reply via email to