[ https://issues.apache.org/jira/browse/PIG-682?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12674378#action_12674378 ]
sms edited comment on PIG-682 at 2/17/09 2:37 PM: ------------------------------------------------------------------ Review comments: General note: 1. The list of error codes and associated strings are documented on the wiki at: http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification Look for the section titled "Compendium of error messages" src/org/apache/pig/Main.java ===================== 1. The following code is not required. ExecException is now subclassed from PigException. {code} + } catch (ExecException e) { + System.err.println("Exec error: " + e.getMessage()); + rc = 2; {code} 2. We should not be printing stack traces anymore. Exceptions that occur in main (within the execution part are already handled by Grunt.java. I could not think of use cases where there was an unhandled exception within Grunt and we did not report it as such. {code} + } catch (Throwable e) { rc = 2; + System.err.println("Unrecoverable error: " + e.getMessage()); + e.printStackTrace(); {code} Index: lib-src/shock/org/apache/pig/shock/SSHSocketImplFactory.java =================================================================== 1. If its a non-fatal error, log.warn is appropriate and not log.error {code} public SocketImpl createSocketImpl() { - return new SSHSocketImpl(session); - + try { + return new SSHSocketImpl(session); + } catch(Throwable e) { + log.error("Couldn't create impl", e); + return null; + } } {code} Index: src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java =================================================================== The setSSHFactory() method catches the SocketException but does nothing. Is this behaviour expected? {code} catch (SocketException e) {} {code} was (Author: sms): Review comments: General note: 1. The list of error codes and associated strings are documented on the wiki at: http://wiki.apache.org/pig/PigErrorHandlingFunctionalSpecification Look for the section titled "Compendium of error messages" src/org/apache/pig/Main.java ===================== 1. The following code is not required. ExecException is now subclassed from PigException. + } catch (ExecException e) { + System.err.println("Exec error: " + e.getMessage()); + rc = 2; 2. We should not be printing stack traces anymore. Exceptions that occur in main (within the execution part are already handled by Grunt.java. I could not think of use cases where there was an unhandled exception within Grunt and we did not report it as such. + } catch (Throwable e) { rc = 2; + System.err.println("Unrecoverable error: " + e.getMessage()); + e.printStackTrace(); Index: lib-src/shock/org/apache/pig/shock/SSHSocketImplFactory.java =================================================================== 1. If its a non-fatal error, log.warn is appropriate and not log.error public SocketImpl createSocketImpl() { - return new SSHSocketImpl(session); - + try { + return new SSHSocketImpl(session); + } catch(Throwable e) { + log.error("Couldn't create impl", e); + return null; + } } Index: src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java =================================================================== The setSSHFactory() method catches the SocketException but does nothing. Is this behaviour expected? catch (SocketException e) {} > Fix the ssh tunneling code > -------------------------- > > Key: PIG-682 > URL: https://issues.apache.org/jira/browse/PIG-682 > Project: Pig > Issue Type: Bug > Components: impl > Reporter: Benjamin Reed > Attachments: jsch-0.1.41.jar, PIG-682.patch > > > Hadoop has changed a bit and the ssh-gateway code no longer works. pig needs > to be updated to register with the new socket framework. reporting of > problems also needs to be better. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.