----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71425/#review217548 -----------------------------------------------------------
core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 53 (patched) <https://reviews.apache.org/r/71425/#comment304813> Do we need this? core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Line 593 (original), 596 (patched) <https://reviews.apache.org/r/71425/#comment304807> Why do we need this modification? This test is unrelated. core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 610-611 (patched) <https://reviews.apache.org/r/71425/#comment304809> This is not true for this method. Probably block-copy error. core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 613 (patched) <https://reviews.apache.org/r/71425/#comment304810> Were you able to run this test? The whole class is ignored right now: https://issues.apache.org/jira/browse/OOZIE-3391 core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 629 (patched) <https://reviews.apache.org/r/71425/#comment304808> Could we use a less personal e-mail address here? core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 630 (patched) <https://reviews.apache.org/r/71425/#comment304811> script.sh is created at the beginning of the test method, but now used here. If we don't need it, then please delete all the related code. core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 631 (patched) <https://reviews.apache.org/r/71425/#comment304812> Do we need the semicolon? core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java Lines 647-649 (patched) <https://reviews.apache.org/r/71425/#comment304814> Can you please add assert messages. - Andras Salamon On Sept. 3, 2019, 1:10 p.m., Mate Juhasz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71425/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2019, 1:10 p.m.) > > > Review request for oozie, Andras Salamon, Denes Bodo, and Kinga Marton. > > > Bugs: OOZIE-3405 > https://issues.apache.org/jira/browse/OOZIE-3405 > > > Repository: oozie-git > > > Description > ------- > > Currently, when an SSH action fails the only message that is returned is the > Status. Neither the error Message nor Error code fields are filled. This > makes reporting on the causes of SSH Action failures via Oozie highly > impractical: the only meaningful bit of information there is on a failed SSH > Action is the Status. > > The Status is filled based on the presence (or lack of) the .error file that > is produced in case the user submitted script returns with any other value > than 0. > > SshActionExecutor#getActionStatus > ... > String outFile = getRemoteFileName(context, action, "error", false, true); > String checkErrorCmd = SSH_COMMAND_BASE + action.getTrackerUri() + " ls " + > outFile; > int retVal = getReturnValue(checkErrorCmd); > ... > > User requirement is to provide some more detailed information on the > success/failure of the user-submitted script. That could be at a minimum the > return value, optionally the last ~1K of the stderr that is drained. This > information could then be communicated via errorMessage and ErrorCode > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/ErrorCode.java 6b0ce4700 > core/src/main/java/org/apache/oozie/action/ssh/SshActionExecutor.java > 6956cba7b > core/src/main/resources/ssh-wrapper.sh e2e6f7f9b > core/src/test/java/org/apache/oozie/action/ssh/TestSshActionExecutor.java > d68aed094 > > > Diff: https://reviews.apache.org/r/71425/diff/1/ > > > Testing > ------- > > Unit testing is a bit awkward in thise case (I would like to ask for some > advice), although I have managed to test on a live cluster. > > > Thanks, > > Mate Juhasz > >
