abstractdog commented on PR #4683: URL: https://github.com/apache/hive/pull/4683#issuecomment-1792033122
> @abstractdog Can you please check and confirm I still don't get this whole thing currently, we're defining the method signature as: ``` throws IOException, InterruptedException ``` but we intentionally catch them and wrap them into a RuntimeException: ``` } catch (IOException | InterruptedException exception) { // we need to catch only IO exception/Interrupted Exception here, // as part of HIVE-27029 we are swallowing all exceptions and ignoring those causing yarn jobs are submitted LOG.error("Cannot check queue access against UGI: " + ugi, exception); throw new RuntimeException(exception); } ``` so I don't really get the point of the method signature :) also, if you choose to comment instead of unit test, that's fine, but I expect this patch to at least define what an IOException means in this scope, you know: "need to catch only IO exception" tells nothing about this whole problem I'm assuming that by working on HIVE-27679 and HIVE-26352 you guys have a solid context about this area, so I want that to be reflected in the code as much as possible -- 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