xkrogen commented on pull request #30284: URL: https://github.com/apache/spark/pull/30284#issuecomment-724816475
While I do agree that string-matching on `"hadoop"` is a bit sketchy, I don't think I agree with your conclusion about risk -- currently we only catch the `RuntimeException` if it contains `"hadoop"`, else we allow the exception to bubble up. Adding an exception throw at line 74 instead of calling `downloadVersion` again just aligns the behavior of "missing Hadoop JARs" with the behavior of a non-Hadoop exception. The current logic is more risky in my opinion, bringing unpredictable behavior. Given that this feature is mainly used in unit tests, it brings an increased degree of non-determinism to the unit tests. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
