[HUDI-14922] Fix Windows path separator in DFSPropertiesConfiguration.getConfPathFromEnv#18454
Conversation
….getConfPathFromEnv Replace File.separator with StoragePath.SEPARATOR in DFSPropertiesConfiguration to ensure cross-platform compatibility. File.separator returns backslash (\) on Windows, which is invalid for HDFS/cloud storage paths that always require forward slash (/). StoragePath.SEPARATOR is the correct constant for storage paths in Hudi. Fixes apache#14922
yihua
left a comment
yihua
left a comment
There was a problem hiding this comment.
🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.
LGTM — clean, targeted fix that correctly swaps the OS-dependent File.separator for the storage-layer constant StoragePath.SEPARATOR, ensuring HDFS/cloud paths always use / even when the agent runs on Windows. CI passes and the change is minimal with no behavioral impact on non-Windows platforms.
| confDir = "file://" + confDir; | ||
| } | ||
| return Option.of(new StoragePath(confDir + File.separator + DEFAULT_PROPERTIES_FILE)); | ||
| return Option.of(new StoragePath(confDir + StoragePath.SEPARATOR + DEFAULT_PROPERTIES_FILE)); |
There was a problem hiding this comment.
🤖 This fixes the join separator correctly. One thing I'm wondering about: on Windows, HUDI_CONF_DIR would typically be set to something like C:\hudi\conf, so after the file:// prefix is added you'd end up with file://C:\hudi\conf/hudi-defaults.conf — mixed separators. Is there a follow-up needed to normalize the backslashes in confDir itself before constructing the StoragePath? Or does StoragePath's URI parsing handle that gracefully?
|
Pinging for committer approval. This PR (HUDI-14922) fixes the Windows path separator issue in |
3 participants
Replace File.separator with StoragePath.SEPARATOR in DFSPropertiesConfiguration to ensure cross-platform compatibility. File.separator returns backslash () on Windows, which is invalid for HDFS/cloud storage paths that always require forward slash (/). StoragePath.SEPARATOR is the correct constant for storage paths in Hudi.
Fixes #14922
Describe the issue this Pull Request addresses
Summary and Changelog
Impact
Risk Level
Documentation Update
Contributor's checklist