[ 
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.

Reply via email to