yihua commented on code in PR #8732:
URL: https://github.com/apache/hudi/pull/8732#discussion_r1197105077
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ReflectionUtils.java:
##########
@@ -83,19 +83,40 @@ public static Object loadClass(String clazz, Class<?>[]
constructorArgTypes, Obj
}
/**
- * Check if the clazz has the target constructor or not.
+ * Check if the clazz has the target constructor or not, without throwing
warn-level log.
*
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @return
+ */
+ public static boolean hasConstructor(String clazz, Class<?>[]
constructorArgTypes) {
+ return hasConstructor(clazz, constructorArgTypes, true);
+ }
+
+ /**
+ * Check if the clazz has the target constructor or not.
+ * <p>
* When catch {@link HoodieException} from {@link #loadClass}, it's
inconvenient to say if the exception was thrown
* due to the instantiation's own logic or missing constructor.
- *
+ * <p>
* TODO: ReflectionUtils should throw a specific exception to indicate
Reflection problem.
+ *
+ * @param clazz Class name.
+ * @param constructorArgTypes Argument types of the constructor.
+ * @param silenceWarning {@code true} to use debug-level logging;
otherwise, use warn-level logging.
+ * @return {@code true} if the constructor exists; {@code false} otherwise.
*/
- public static boolean hasConstructor(String clazz, Class<?>[]
constructorArgTypes) {
+ public static boolean hasConstructor(String clazz, Class<?>[]
constructorArgTypes, boolean silenceWarning) {
Review Comment:
Not sure if I follow you. Production code in
`SyncUtilHelpers.instantiateMetaSyncTool` uses this method and throws warning
logs which are noises to users (this tries all possible arg combinations until
the right one is found; the wrong arg combination throws noisy warning log
before this PR). This is irrelevant to tests.
```
if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class, Configuration.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class, Configuration.class},
properties, hadoopConfig));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class},
properties));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {TypedProperties.class, Configuration.class,
FileSystem.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {TypedProperties.class, Configuration.class,
FileSystem.class},
properties, hadoopConfig, fs));
} else if (ReflectionUtils.hasConstructor(syncToolClassName,
new Class<?>[] {Properties.class, FileSystem.class})) {
return ((HoodieSyncTool) ReflectionUtils.loadClass(syncToolClassName,
new Class<?>[] {Properties.class, FileSystem.class},
properties, fs));
} else {
throw new HoodieException("Could not load meta sync class " +
syncToolClassName
+ ": no valid constructor found.");
}
```
--
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]
