SEZ9 commented on issue #12707: URL: https://github.com/apache/seatunnel/issues/12707#issuecomment-6183179417
@karthik-0306 thanks for the follow-up — that answers my question. Glad both pieces (the JAVA_HOME value surfaced in the error with a fix/unset hint, plus a test for it) are in #12740; nothing further is needed from you on this issue thread for those two asks. On the one design choice you flagged — a bad JAVA_HOME raising an error instead of falling back to PATH — I'm fine with that direction: a wrong JAVA_HOME is almost always a misconfiguration the user wants to know about, and silently picking a different Java from PATH would hide it. With the explicit value and the fix/unset hint now in the message, that trade-off is well covered. Two small things so I review the right state: - CryoThrust noted the PR head as `8e75f8cf` when linking it. If the JAVA_HOME error/test changes landed after that, just confirm they're pushed and I'll review the latest revision in the PR rather than here. - CryoThrust's run covered the detector tests and the Java 8/11/17 cases on macOS only. If you've run the new JAVA_HOME test locally as well, a quick note in the PR is enough; I'll take care of the remaining review there. I'll leave any further comments on #12740 itself. <!-- streview-comment:1759 --> -- 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]
