abstractdog commented on code in PR #5859: URL: https://github.com/apache/hive/pull/5859#discussion_r2157433154
########## ql/pom.xml: ########## @@ -1142,6 +1143,12 @@ <exclude>META-INF/licenses/slf4j*/**</exclude> </excludes> </filter> + <filter> + <artifact>org.apache.tez:tez-protobuf-history-plugin</artifact> Review Comment: hm, I don't get why we wouldn't simply add a few classes to hive-exec jar? that's what we've been doing for a long time: https://github.com/apache/hive/blob/0c69f10f14b0f4b1983093de105ed6b35d4d483b/ql/pom.xml#L1047-L1106 basically, here we have 2 options: 1) add this include into hive-exec, making the classes available everywhere where hive ql is available 2) or take care of the classpath everywhere, this is at least 2-3 places: 2a) upstream packaging and/or Docker 2b) the classpath of the downstream process (schematool with hive ql usage embedded), where this issue was originally reported 2c) downstream HS2 classpaths theoretically: important to note that downstream clusters haven't started to fail immediately after HIVE-28028 just because (e.g. on the cluster I was working on) the HS2 classpath included tez-protobuf-history-plugin, no matter why it was done like that, looked strange as I expect only tez-api to be present on HS2 classpath -- 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: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org