arham0254A commented on PR #56736: URL: https://github.com/apache/spark/pull/56736#issuecomment-4801290303
Thanks so much for the thorough review, @uros-b! I really appreciate the guidance here. 1. Jira Ticket: you are completely right. I accidentally used the GitHub issue number instead of the corresponding Jira ticket in the title. I will track down the correct SPARK ID and update that. 2. Untestable `System.exit`: That makes perfect sense regarding `ClientSuite.scala`. Hardcoding `System.exit` definitely breaks the testability. I will refactor this to use an injectable `exitFn` hook and add a proper unit test covering the terminal-state to exit-code mapping. 3. Logs & Docs: I will fix the indentation nit, update the failure-arm log message to be explicitly distinct for operators, and update the method docstring to reflect the new exit paths. I'll get to work on these updates and push a new commit to this branch soon -- 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]
