wombatu-kun commented on code in PR #16793:
URL: https://github.com/apache/iceberg/pull/16793#discussion_r3407336885
##########
common/src/main/java/org/apache/iceberg/common/DynClasses.java:
##########
@@ -66,7 +66,7 @@ public Builder impl(String className) {
try {
this.foundClass = Class.forName(className, true, loader);
- } catch (ClassNotFoundException e) {
+ } catch (ClassNotFoundException | NoClassDefFoundError e) {
Review Comment:
With the catch now also covering `NoClassDefFoundError`, the `// not the
right implementation` comment below no longer fits that case: a
`NoClassDefFoundError` means the class was found but could not be loaded
(missing transitive dependency), not that it is the wrong implementation. The
codebase already uses the accurate wording for the same situation at
DynConstructors.java:148 (`// cannot load this implementation`), which @anoopj
suggested on #16611. Same applies to the widened catches at DynFields.java:238,
DynFields.java:287, DynMethods.java:255, and DynMethods.java:336.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
