zhangshenghang commented on PR #10347: URL: https://github.com/apache/seatunnel/pull/10347#issuecomment-3755220113
<!-- code-pr-reviewer --> Thanks for the contribution! I have a couple of blocking concerns: - **EnvCommonOptions.java:126-128** - METALAKE_TYPE is typed as `Option<MetaLakeType>` with `.enumType(MetaLakeType.class)`, but MetalakeConfigUtils.java:62 reads it via `getString()`, bypassing the enum deserialization. This type contract mismatch can cause `ConfigException$WrongType` if other code reads this option in a type-safe way. Consider changing the type to `Option<String>` to align with the actual usage and `MetalakeClientFactory.create(String type, ...)` signature. - **MetalakeConfigUtils.java:60-68** - Missing null checks when reading `metalakeType` and `metalakeUrl` from environment variables. If `System.getenv()` returns null, the values are passed directly to `MetalakeClientFactory.create()`, resulting in unfriendly errors that don't clearly indicate which required config is missing. Please add explicit validation to provide clear error messages when metalake is enabled but required configs are unset. -- 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]
