SEZ9 commented on issue #12707: URL: https://github.com/apache/seatunnel/issues/12707#issuecomment-6159383760
Thanks @karthik-0306, got the link now — #12740 (head `8e75f8cf`) is on my list and I'll review it there. On the design choice you flagged: I'm fine with a bad `JAVA_HOME` raising an error instead of silently falling back to `PATH`. If a user set `JAVA_HOME` explicitly, falling back would just hide the misconfiguration. Two asks to keep that consistent with the "clear error" point we agreed on: - The error should say what `JAVA_HOME` value was tried and why it was rejected (nothing found there, or version below 11), and point the user to fix or unset `JAVA_HOME`. - Please make sure that case is covered by a pytest test alongside the mocked `java -version` cases. Also appreciated @CryoThrust running the detector tests and checking Java 8/11/17 on macOS — good coverage for the detection piece. Anything else I have will go as comments on the PR itself. <!-- streview-comment:1730 --> -- 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]
