Cleanup exception handling in FsShell.run() method to print the stack trace of unexpected exceptions such as NPEs.

Converted -help & -usage into FsCommand subclasses to greatly simplify the aforementioned exception handling block. All conditional chains are now gone! Everything is dynamic.

A few trivial changes to Command & CommandFactory to support nested classes needed for -help/-usage to access the CommandFactory.

Break shell object instantiation, and population of commandFactory, into separate methods so DFSAdmin (already a subclass) can eventually take advantage of the modularity.

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12481372/HADOOP-7353.patch
against trunk revision 1130833.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

+1 system test framework. The patch passed system test framework compile.

Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/569//testReport/
Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/569//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/569//console

This message is automatically generated.

I also removed the slew of pre-existing warnings in FsShell...

Looks pretty good, Daryn. A few small nits:

• There's no need to expressly initialize the instance variables in the FsShell} ctor to {{null. Java will do that automatically.
• "The follow methods are syntactic sugar." - They're not really "syntactic" sugar, as much as they are just helpers.
• "if (instance == null) throw new UnknownCommandException(cmd);" - coding guidelines say to put always use braces on if statements, and always put the body on a new line.
• There seems to be an inconsistency with printing to stdout vs. stderr, or at least I can't tell why you chose to use one in some places, and the other in other places.
• "// historical abstract method in Command " - move this comment above the method.

Thanks Aaron!

There's no need to expressly initialize the instance variables in the FsShell ctor to null. Java will do that automatically.

Sure, I removed the initialization.

"The follow methods are syntactic sugar." - They're not really "syntactic" sugar, as much as they are just helpers.

Changed.

"if (instance == null) throw new UnknownCommandException(cmd);" - coding guidelines say to put always use braces on if statements, and always put the body on a new line.

Changed. I found a few more, so I changed them too.

There seems to be an inconsistency with printing to stdout vs. stderr, or at least I can't tell why you chose to use one in some places, and the other in other places.

The -help/-usage commands will print to stdout. The usage goes to stderr when no command is given, or because of an illegal argument.

"// historical abstract method in Command " - move this comment above the method.

Moved.

Thanks again!

+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12481578/HADOOP-7353-2.patch
against trunk revision 1132511.

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified tests.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

+1 system test framework. The patch passed system test framework compile.

Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/581//testReport/
Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/581//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/581//console

This message is automatically generated.

+1, latest patch looks good to me.

Committed to trunk based on atm's +1 and a quick sanity check. Thanks Daryn/Aaron.

Integrated in Hadoop-Common-trunk-Commit #643 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/643/)
HADOOP-7353. Cleanup FsShell and prevent masking of RTE stack traces. Contributed by Daryn Sharp.

todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132764
Files :

• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java
• /hadoop/common/trunk/CHANGES.txt
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java
• /hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java

Integrated in Hadoop-Common-trunk #712 (See https://builds.apache.org/job/Hadoop-Common-trunk/712/)
HADOOP-7353. Cleanup FsShell and prevent masking of RTE stack traces. Contributed by Daryn Sharp.

todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132764
Files :

• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java
• /hadoop/common/trunk/CHANGES.txt
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java
• /hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
• /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java