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


##########
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:
   @pvary I made two mistakes above.
   
   First, I made a mistake about the type hierarchies. _I had missed that_ 
`IHMSHandler` **is** a `FacebookService.Iface`. So I can use 
`FacebookService.Iface` for `Foo`. I.e., I can declare the `baseHandler` field 
to be of type `FacebookService.Iface`. (No wonder you didn't understand my 
thinking before, as it was wrong!)
   
   Second, I was thinking about what the actual static method (either 
`RetryingHMSHandler.getProxy` in the case of Hive 2/3 or 
`HMSHandlerProxyFactory.getProxy` in the case of Hive 4) needs as parameters. 
It needs an `IHMSHandler` as its second parameter, not a 
`FacebookService.Iface`. However, when using reflection 
(`GET_BASE_HMS_HANDLER.invoke`), the parameters (to the `invoke`) are 
`Object`s, so there is no type checking. That is why the code compiles (and at 
runtime, fortunately `HMSHandler` is the correct type of `Object`).
   
   I have pushed a new commit to fix this issue.



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