Hadoop Common
  1. Hadoop Common
  2. HADOOP-8362

Improve exception message when Configuration.set() is called with a null key or value

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.2-alpha
    • Component/s: conf
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      Currently, calling Configuration.set(...) with a null value results in a NullPointerException within Properties.setProperty. We should check for null key/value and throw a better exception.

      1. HADOOP-8362.10.patch
        3 kB
        Harsh J
      2. HADOOP-8362.9.patch
        2 kB
        Suresh Srinivas
      3. HADOOP-8362-8.patch
        2 kB
        madhukara phatak
      4. HADOOP-8362-7.patch
        2 kB
        madhukara phatak
      5. HADOOP-8362-6.patch
        2 kB
        madhukara phatak
      6. HADOOP-8362-5.patch
        3 kB
        madhukara phatak
      7. HADOOP-8362-4.patch
        2 kB
        madhukara phatak
      8. HADOOP-8362-3.patch
        2 kB
        madhukara phatak
      9. HADOOP-8362-2.patch
        2 kB
        madhukara phatak
      10. HADOOP-8362-1.patch
        2 kB
        madhukara phatak
      11. HADOOP-8362.patch
        0.7 kB
        madhukara phatak

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1138 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1138/)
        HADOOP-8362. Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712)

        Result = FAILURE
        harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1138 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1138/ ) HADOOP-8362 . Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1105 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1105/)
        HADOOP-8362. Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712)

        Result = FAILURE
        harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1105 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1105/ ) HADOOP-8362 . Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2492 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2492/)
        HADOOP-8362. Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712)

        Result = FAILURE
        harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2492 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2492/ ) HADOOP-8362 . Improve exception message when Configuration.set() is called with a null key or value. Contributed by Madhukara Phatak and Suresh Srinivas (harsh) (Revision 1361712) Result = FAILURE harsh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1361712 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
        Hide
        Harsh J added a comment -

        Committed to branch-2 and trunk. Thanks Madhukara and Suresh!

        Show
        Harsh J added a comment - Committed to branch-2 and trunk. Thanks Madhukara and Suresh!
        Hide
        Harsh J added a comment -

        Had to rebase this after HADOOP-8525 made some changes.

        Also fixed an extra space in midst of a javadoc sentence, and made the error more assertive, and descriptive of the context.

        Added tests were unmodified pass as before:

        Running org.apache.hadoop.conf.TestConfiguration
        Tests run: 50, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.544 sec
        

        Committing this version.

        Show
        Harsh J added a comment - Had to rebase this after HADOOP-8525 made some changes. Also fixed an extra space in midst of a javadoc sentence, and made the error more assertive, and descriptive of the context. Added tests were unmodified pass as before: Running org.apache.hadoop.conf.TestConfiguration Tests run: 50, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.544 sec Committing this version.
        Hide
        Harsh J added a comment -
        ➜  hadoop-common git:(trunk) grep 8362 CHANGES.txt
        ➜  hadoop-common git:(trunk)
        
        Show
        Harsh J added a comment - ➜ hadoop-common git:(trunk) grep 8362 CHANGES.txt ➜ hadoop-common git:(trunk)
        Hide
        Harsh J added a comment -

        Hi Suresh,

        Looks like this wasn't committed? I'm going ahead and committing it in. Reopening for until it is done.

        Show
        Harsh J added a comment - Hi Suresh, Looks like this wasn't committed? I'm going ahead and committing it in. Reopening for until it is done.
        Hide
        Suresh Srinivas added a comment -

        Committed the patch. Thank you Madhukara.

        Show
        Suresh Srinivas added a comment - Committed the patch. Thank you Madhukara.
        Hide
        Suresh Srinivas added a comment -

        Minor changes to fix indentation issues.

        Madhukara, in future, please follow the coding guidelines as Colin had suggested.

        Show
        Suresh Srinivas added a comment - Minor changes to fix indentation issues. Madhukara, in future, please follow the coding guidelines as Colin had suggested.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12532674/HADOOP-8362-8.patch
        against trunk revision .

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

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

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

        -1 javadoc. The javadoc tool appears to have generated 13 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 unit tests in hadoop-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1125//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1125//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12532674/HADOOP-8362-8.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 javadoc. The javadoc tool appears to have generated 13 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 unit tests in hadoop-common-project/hadoop-common. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1125//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1125//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        fixed patch issues

        Show
        madhukara phatak added a comment - fixed patch issues
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12532650/HADOOP-8362-7.patch
        against trunk revision .

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1124//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12532650/HADOOP-8362-7.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1124//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        updated patch with style fixes

        Show
        madhukara phatak added a comment - updated patch with style fixes
        Hide
        Colin Patrick McCabe added a comment -

        Hi Madhukara,

        The patch looks pretty good. Just one small criticism: you should put the braces on the same line as the 'if' or 'else' statement.

        See http://wiki.apache.org/hadoop/CodeReviewChecklist for details-- basically, we try to follow Sun's coding conventions, except with 2-space indentation instead of 4.
        (See http://java.sun.com/docs/codeconv/)

        Show
        Colin Patrick McCabe added a comment - Hi Madhukara, The patch looks pretty good. Just one small criticism: you should put the braces on the same line as the 'if' or 'else' statement. See http://wiki.apache.org/hadoop/CodeReviewChecklist for details-- basically, we try to follow Sun's coding conventions, except with 2-space indentation instead of 4. (See http://java.sun.com/docs/codeconv/ )
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530159/HADOOP-8362-6.patch
        against trunk revision .

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

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

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

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

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.fs.viewfs.TestViewFsTrash
        org.apache.hadoop.ha.TestZKFailoverController

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1055//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1055//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12530159/HADOOP-8362-6.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.fs.viewfs.TestViewFsTrash org.apache.hadoop.ha.TestZKFailoverController +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1055//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1055//console This message is automatically generated.
        Hide
        Harsh J added a comment -

        No need to be sorry! I just wanted to note that. Thanks for your continuing contributions! Let us know if you need help with test-patch/etc.

        Show
        Harsh J added a comment - No need to be sorry! I just wanted to note that. Thanks for your continuing contributions! Let us know if you need help with test-patch/etc.
        Hide
        madhukara phatak added a comment -

        Hi Harsh,
        Sorry for too many patches ..was not sure about when to use submit patch button yeah will use the test-patch shell script going forward ..thank you.

        Show
        madhukara phatak added a comment - Hi Harsh, Sorry for too many patches ..was not sure about when to use submit patch button yeah will use the test-patch shell script going forward ..thank you.
        Hide
        Harsh J added a comment -

        Hi Madhukara,

        I do appreciate (and love) your enthusiasm in providing new, review-fixing patches but please do test your patch locally for simple things such as compiler errors and test runs before you send it across. Surely a local compile can help us save a lot of time.

        You can also run a test-patch.sh run locally to test your patch yourself first, instead of relying on the QA queue, which is for reviewed patches alone.

        Thanks and please keep contributing!
        Harsh

        Show
        Harsh J added a comment - Hi Madhukara, I do appreciate (and love) your enthusiasm in providing new, review-fixing patches but please do test your patch locally for simple things such as compiler errors and test runs before you send it across. Surely a local compile can help us save a lot of time. You can also run a test-patch.sh run locally to test your patch yourself first, instead of relying on the QA queue, which is for reviewed patches alone. Thanks and please keep contributing! Harsh
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530036/HADOOP-8362-5.patch
        against trunk revision .

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

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

        -1 javac. The patch appears to cause the build to fail.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1047//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12530036/HADOOP-8362-5.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. -1 javac. The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1047//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        fixed apply patch issue

        Show
        madhukara phatak added a comment - fixed apply patch issue
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12530034/HADOOP-8362-4.patch
        against trunk revision .

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1046//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12530034/HADOOP-8362-4.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1046//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        updated patch to have two separate pre-conditions.

        Show
        madhukara phatak added a comment - updated patch to have two separate pre-conditions.
        Hide
        Harsh J added a comment -

        Hi,

        Sorry for the late response here. I failed to add another comment earlier:

        Can you separate the key==null and value==null checks? We shouldn't have the users determine which one of them being null caused the exception as is currently the case anyway. Two preconditions would be good (same for tests).

        Thanks!

        Show
        Harsh J added a comment - Hi, Sorry for the late response here. I failed to add another comment earlier: Can you separate the key==null and value==null checks? We shouldn't have the users determine which one of them being null caused the exception as is currently the case anyway. Two preconditions would be good (same for tests). Thanks!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12528093/HADOOP-8362-3.patch
        against trunk revision .

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

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

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

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.fs.viewfs.TestViewFsTrash

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1009//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1009//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12528093/HADOOP-8362-3.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 javadoc. The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1009//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1009//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        fixed patch for the core-tests

        Show
        madhukara phatak added a comment - fixed patch for the core-tests
        Hide
        Hadoop QA added a comment -

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

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

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

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

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 failed these unit tests in hadoop-common-project/hadoop-common:

        org.apache.hadoop.io.TestMapFile
        org.apache.hadoop.fs.TestFilterFileSystem
        org.apache.hadoop.fs.viewfs.TestViewFileSystemDelegation
        org.apache.hadoop.http.lib.TestStaticUserWebFilter
        org.apache.hadoop.fs.viewfs.TestViewFsTrash
        org.apache.hadoop.io.file.tfile.TestTFileStreams
        org.apache.hadoop.util.TestGenericOptionsParser
        org.apache.hadoop.ipc.TestSaslRPC
        org.apache.hadoop.ipc.TestMultipleProtocolServer
        org.apache.hadoop.conf.TestReconfiguration
        org.apache.hadoop.ipc.TestSocketFactory
        org.apache.hadoop.fs.viewfs.TestViewfsFileStatus
        org.apache.hadoop.io.file.tfile.TestTFile
        org.apache.hadoop.ipc.TestProtoBufRpc
        org.apache.hadoop.fs.TestFsShellReturnCode
        org.apache.hadoop.conf.TestConfigurationDeprecation
        org.apache.hadoop.io.serializer.TestWritableSerialization
        org.apache.hadoop.io.TestGenericWritable
        org.apache.hadoop.io.file.tfile.TestTFileSplit
        org.apache.hadoop.fs.s3native.TestInMemoryNativeS3FileSystemContract
        org.apache.hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs
        org.apache.hadoop.net.TestScriptBasedMapping
        org.apache.hadoop.fs.TestS3_LocalFileContextURI
        org.apache.hadoop.security.TestSecurityUtil
        org.apache.hadoop.util.TestGenericsUtil
        org.apache.hadoop.fs.viewfs.TestFcPermissionsLocalFs
        org.apache.hadoop.fs.TestTruncatedInputBug
        org.apache.hadoop.ipc.TestRPCCallBenchmark
        org.apache.hadoop.http.TestHttpServerLifecycle
        org.apache.hadoop.fs.TestTrash
        org.apache.hadoop.security.TestGroupsCaching
        org.apache.hadoop.io.TestDefaultStringifier
        org.apache.hadoop.security.TestUserGroupInformation
        org.apache.hadoop.log.TestLogLevel
        org.apache.hadoop.fs.viewfs.TestFSMainOperationsLocalFileSystem
        org.apache.hadoop.io.file.tfile.TestTFileNoneCodecsByteArrays
        org.apache.hadoop.conf.TestGetInstances
        org.apache.hadoop.ha.TestZKFailoverController
        org.apache.hadoop.io.file.tfile.TestTFileSeqFileComparison
        org.apache.hadoop.ha.TestHealthMonitor
        org.apache.hadoop.fs.permission.TestFsPermission
        org.apache.hadoop.io.compress.TestCodec
        org.apache.hadoop.fs.TestFileSystemCanonicalization
        org.apache.hadoop.net.TestTableMapping
        org.apache.hadoop.security.TestLdapGroupsMapping
        org.apache.hadoop.fs.shell.TestPathData
        org.apache.hadoop.ipc.TestServer
        org.apache.hadoop.fs.viewfs.TestFcMainOperationsLocalFs
        org.apache.hadoop.io.file.tfile.TestTFileNoneCodecsJClassComparatorByteArrays
        org.apache.hadoop.http.TestHttpServerWebapps
        org.apache.hadoop.fs.viewfs.TestViewFileSystemLocalFileSystem
        org.apache.hadoop.ipc.TestRPC
        org.apache.hadoop.http.TestGlobalFilter
        org.apache.hadoop.security.authorize.TestProxyUsers
        org.apache.hadoop.io.file.tfile.TestTFileSeek
        org.apache.hadoop.fs.viewfs.TestViewFsConfig
        org.apache.hadoop.fs.viewfs.TestFcCreateMkdirLocalFs
        org.apache.hadoop.fs.TestLocal_S3FileContextURI
        org.apache.hadoop.io.file.tfile.TestTFileByteArrays
        org.apache.hadoop.ha.TestSshFenceByTcpPort
        org.apache.hadoop.io.file.tfile.TestTFileUnsortedByteArrays
        org.apache.hadoop.ipc.TestMiniRPCBenchmark
        org.apache.hadoop.fs.s3.TestInMemoryS3FileSystemContract
        org.apache.hadoop.ipc.TestIPC
        org.apache.hadoop.conf.TestConfServlet
        org.apache.hadoop.cli.TestCLI
        org.apache.hadoop.io.file.tfile.TestTFileComparator2
        org.apache.hadoop.security.TestAuthenticationFilter
        org.apache.hadoop.fs.viewfs.TestViewFileSystemDelegationTokenSupport
        org.apache.hadoop.fs.viewfs.TestViewFsLocalFs
        org.apache.hadoop.conf.TestDeprecatedKeys
        org.apache.hadoop.conf.TestConfiguration
        org.apache.hadoop.io.serializer.avro.TestAvroSerialization
        org.apache.hadoop.fs.TestLocalDirAllocator
        org.apache.hadoop.net.TestSwitchMapping
        org.apache.hadoop.ha.TestShellCommandFencer
        org.apache.hadoop.jmx.TestJMXJsonServlet
        org.apache.hadoop.io.TestBloomMapFile
        org.apache.hadoop.io.compress.TestCodecFactory
        org.apache.hadoop.io.TestSequenceFileSerialization
        org.apache.hadoop.ipc.TestRPCCompatibility
        org.apache.hadoop.fs.viewfs.TestViewFileSystemWithAuthorityLocalFileSystem
        org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays
        org.apache.hadoop.http.TestPathFilter
        org.apache.hadoop.net.TestStaticMapping
        org.apache.hadoop.ipc.TestIPCServerResponder
        org.apache.hadoop.security.TestDoAsEffectiveUser
        org.apache.hadoop.http.TestServletFilter
        org.apache.hadoop.http.TestHttpServer

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1008//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1008//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12528070/HADOOP-8362-2.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 javadoc. The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.io.TestMapFile org.apache.hadoop.fs.TestFilterFileSystem org.apache.hadoop.fs.viewfs.TestViewFileSystemDelegation org.apache.hadoop.http.lib.TestStaticUserWebFilter org.apache.hadoop.fs.viewfs.TestViewFsTrash org.apache.hadoop.io.file.tfile.TestTFileStreams org.apache.hadoop.util.TestGenericOptionsParser org.apache.hadoop.ipc.TestSaslRPC org.apache.hadoop.ipc.TestMultipleProtocolServer org.apache.hadoop.conf.TestReconfiguration org.apache.hadoop.ipc.TestSocketFactory org.apache.hadoop.fs.viewfs.TestViewfsFileStatus org.apache.hadoop.io.file.tfile.TestTFile org.apache.hadoop.ipc.TestProtoBufRpc org.apache.hadoop.fs.TestFsShellReturnCode org.apache.hadoop.conf.TestConfigurationDeprecation org.apache.hadoop.io.serializer.TestWritableSerialization org.apache.hadoop.io.TestGenericWritable org.apache.hadoop.io.file.tfile.TestTFileSplit org.apache.hadoop.fs.s3native.TestInMemoryNativeS3FileSystemContract org.apache.hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs org.apache.hadoop.net.TestScriptBasedMapping org.apache.hadoop.fs.TestS3_LocalFileContextURI org.apache.hadoop.security.TestSecurityUtil org.apache.hadoop.util.TestGenericsUtil org.apache.hadoop.fs.viewfs.TestFcPermissionsLocalFs org.apache.hadoop.fs.TestTruncatedInputBug org.apache.hadoop.ipc.TestRPCCallBenchmark org.apache.hadoop.http.TestHttpServerLifecycle org.apache.hadoop.fs.TestTrash org.apache.hadoop.security.TestGroupsCaching org.apache.hadoop.io.TestDefaultStringifier org.apache.hadoop.security.TestUserGroupInformation org.apache.hadoop.log.TestLogLevel org.apache.hadoop.fs.viewfs.TestFSMainOperationsLocalFileSystem org.apache.hadoop.io.file.tfile.TestTFileNoneCodecsByteArrays org.apache.hadoop.conf.TestGetInstances org.apache.hadoop.ha.TestZKFailoverController org.apache.hadoop.io.file.tfile.TestTFileSeqFileComparison org.apache.hadoop.ha.TestHealthMonitor org.apache.hadoop.fs.permission.TestFsPermission org.apache.hadoop.io.compress.TestCodec org.apache.hadoop.fs.TestFileSystemCanonicalization org.apache.hadoop.net.TestTableMapping org.apache.hadoop.security.TestLdapGroupsMapping org.apache.hadoop.fs.shell.TestPathData org.apache.hadoop.ipc.TestServer org.apache.hadoop.fs.viewfs.TestFcMainOperationsLocalFs org.apache.hadoop.io.file.tfile.TestTFileNoneCodecsJClassComparatorByteArrays org.apache.hadoop.http.TestHttpServerWebapps org.apache.hadoop.fs.viewfs.TestViewFileSystemLocalFileSystem org.apache.hadoop.ipc.TestRPC org.apache.hadoop.http.TestGlobalFilter org.apache.hadoop.security.authorize.TestProxyUsers org.apache.hadoop.io.file.tfile.TestTFileSeek org.apache.hadoop.fs.viewfs.TestViewFsConfig org.apache.hadoop.fs.viewfs.TestFcCreateMkdirLocalFs org.apache.hadoop.fs.TestLocal_S3FileContextURI org.apache.hadoop.io.file.tfile.TestTFileByteArrays org.apache.hadoop.ha.TestSshFenceByTcpPort org.apache.hadoop.io.file.tfile.TestTFileUnsortedByteArrays org.apache.hadoop.ipc.TestMiniRPCBenchmark org.apache.hadoop.fs.s3.TestInMemoryS3FileSystemContract org.apache.hadoop.ipc.TestIPC org.apache.hadoop.conf.TestConfServlet org.apache.hadoop.cli.TestCLI org.apache.hadoop.io.file.tfile.TestTFileComparator2 org.apache.hadoop.security.TestAuthenticationFilter org.apache.hadoop.fs.viewfs.TestViewFileSystemDelegationTokenSupport org.apache.hadoop.fs.viewfs.TestViewFsLocalFs org.apache.hadoop.conf.TestDeprecatedKeys org.apache.hadoop.conf.TestConfiguration org.apache.hadoop.io.serializer.avro.TestAvroSerialization org.apache.hadoop.fs.TestLocalDirAllocator org.apache.hadoop.net.TestSwitchMapping org.apache.hadoop.ha.TestShellCommandFencer org.apache.hadoop.jmx.TestJMXJsonServlet org.apache.hadoop.io.TestBloomMapFile org.apache.hadoop.io.compress.TestCodecFactory org.apache.hadoop.io.TestSequenceFileSerialization org.apache.hadoop.ipc.TestRPCCompatibility org.apache.hadoop.fs.viewfs.TestViewFileSystemWithAuthorityLocalFileSystem org.apache.hadoop.io.file.tfile.TestTFileJClassComparatorByteArrays org.apache.hadoop.http.TestPathFilter org.apache.hadoop.net.TestStaticMapping org.apache.hadoop.ipc.TestIPCServerResponder org.apache.hadoop.security.TestDoAsEffectiveUser org.apache.hadoop.http.TestServletFilter org.apache.hadoop.http.TestHttpServer +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1008//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1008//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        updated the patch according to harsh's comments.

        Show
        madhukara phatak added a comment - updated the patch according to harsh's comments.
        Hide
        Harsh J added a comment -

        Hi,

        Thanks for the patches!

        Some comments from my side:

        • Better to use IllegalArgumentException instead of the raw RuntimeException.
        • Perhaps we can check for this very early (first step) into the set() call. Perhaps better to be done via Guava's http://google-collections.googlecode.com/svn/trunk/javadoc/com/google/common/base/Preconditions.html#checkArgument(boolean,%20java.lang.Object) call, rather than duplicating code. We already have Guava added in as a dependency, makes sense to leverage it where we can for its free benefits.
        • Please also document the behavior of sending in nulls inside the set call's javadocs, so users can benefit from it (by knowing what to expect).
        • Test is good, but also good to assert that the exception object is the right one, and that the message is proper. Helps avoid regressions in future.
        Show
        Harsh J added a comment - Hi, Thanks for the patches! Some comments from my side: Better to use IllegalArgumentException instead of the raw RuntimeException. Perhaps we can check for this very early (first step) into the set() call. Perhaps better to be done via Guava's http://google-collections.googlecode.com/svn/trunk/javadoc/com/google/common/base/Preconditions.html#checkArgument(boolean,%20java.lang.Object ) call, rather than duplicating code. We already have Guava added in as a dependency, makes sense to leverage it where we can for its free benefits. Please also document the behavior of sending in nulls inside the set call's javadocs, so users can benefit from it (by knowing what to expect). Test is good, but also good to assert that the exception object is the right one, and that the message is proper. Helps avoid regressions in future.
        Hide
        Hadoop QA added a comment -

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

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

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

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

        -1 javadoc. The javadoc tool appears to have generated 2 warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +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 unit tests in hadoop-common-project/hadoop-common.

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

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1007//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1007//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12528034/HADOOP-8362-1.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 javadoc. The javadoc tool appears to have generated 2 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 unit tests in hadoop-common-project/hadoop-common. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1007//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1007//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        updated patch with testcase

        Show
        madhukara phatak added a comment - updated patch with testcase
        Hide
        Hadoop QA added a comment -

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

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 javac. The patch appears to cause the build to fail.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/993//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12526719/HADOOP-8362.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javac. The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/993//console This message is automatically generated.
        Hide
        madhukara phatak added a comment -

        null check added

        Show
        madhukara phatak added a comment - null check added

          People

          • Assignee:
            madhukara phatak
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            10 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development