dongjoon-hyun commented on PR #58438: URL: https://github.com/apache/spark/pull/58438#issuecomment-5784360654
Thank you for making a softer version for `branch-4.x`, @holdenk. Since this is the `branch-4.x` counterpart of #58414, it would be great to keep the two consistent. Currently there are a few divergences: 1. **Unrecognized auth types**: #58414 uses an allow-list (`LDAP/KERBEROS/CUSTOM/PAM`) and leaves unknown values to `HiveAuthFactory`'s own error, while this PR uses a deny-list (everything except `NONE/NOSASL`), and the test `an unrecognized auth type warns` pins the opposite behavior. 4.x should warn on exactly the configurations that 5.0 will reject. 2. **Message accuracy**: #58414 says impersonation applies on the driver (metastore calls *and* driver-side file system access) and executor-side access runs as the service identity. This PR says only metastore calls are impersonated. 3. **Null comment**: `authType is null when explicitly set empty` contradicts #58414 (`getVar` returns `""`, never null). With the allow-list approach, this comment becomes unnecessary. 4. **PR description**: It says "Explicit warning unless config flag is disabled", but there is no such flag in this PR. The only way to silence it is `doAs=false`, which also disables metastore impersonation. Do we want to add the same static conf to 4.x (e.g., default `true`) so that users can acknowledge the limitation without changing behavior? Also, the warning message is quite long for a single log line. It might be better to keep it concise and move the details to the migration guide. -- 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]
