wypoon commented on code in PR #12721:
URL: https://github.com/apache/iceberg/pull/12721#discussion_r2032172207


##########
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java:
##########
@@ -186,9 +189,9 @@ public void stop() throws Exception {
     if (executorService != null) {
       executorService.shutdown();
     }
-    if (baseHandler != null) {
-      baseHandler.shutdown();
-    }
+    // if (baseHandler != null) {
+    //   baseHandler.shutdown();
+    // }

Review Comment:
   The problem is that `baseHandler` needs to be a `IHMSHandler` for the 
following code in `TestHiveMetastore::newThriftServer`:
   ```
       baseHandler = HMS_HANDLER_CTOR.newInstance("new db based metaserver", 
serverConf);
       IHMSHandler handler = GET_BASE_HMS_HANDLER.invoke(serverConf, 
baseHandler, false);
   ```
   and then it needs to be a `FacebookBase` for `baseHandler.shutdown()` to be 
called.
   `HMSHandler` extends `FacebookBase` and implements `IHMSHandler`, but 
`FacebookBase` and `IHMSHandler` do not have a common parent interface. 
(`FacebookService.Iface` is **not** such an interface.)
   
   On the other hand, the tests seem to all pass even though we don't perform 
this cleanup.



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