wombatu-kun commented on code in PR #18805:
URL: https://github.com/apache/hudi/pull/18805#discussion_r3408880278
##########
hudi-hadoop-common/src/main/java/org/apache/hudi/common/config/DFSPropertiesConfiguration.java:
##########
@@ -156,8 +156,8 @@ public void addPropsFromFile(StoragePath filePath) {
);
try {
- if (filePath.equals(DEFAULT_PATH) && !storage.exists(filePath)) {
- log.debug("Properties file {} not found. Ignoring to load props file",
filePath);
+ if (!storage.exists(filePath)) {
Review Comment:
One caveat on the scoping: the HUDI_CONF_DIR path at loadGlobalProps:129 is
also a top-level call (not include= recursion) but should stay tolerant like
DEFAULT_PATH, since it is optional global config. Pre-change it actually threw
when HUDI_CONF_DIR pointed at a dir without hudi-defaults.conf -
testClassInitializationNeverThrows:260 wraps exactly that case. So an
isInclude-style flag should keep both DEFAULT_PATH and the env conf path
skippable, and fail fast only for the explicit constructor --props paths;
otherwise this trades the FileNotFoundException for a new regression on line
129.
--
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]
