Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: ui
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      We should let users change the permissions of files and directories using the HDFS Web UI

      1. HDFS-7995.04.patch
        8 kB
        Ravi Prakash
      2. HDFS-7995.03.patch
        8 kB
        Ravi Prakash
      3. HDFS-7995.02.patch
        9 kB
        Ravi Prakash
      4. HDFS-7995.01.patch
        9 kB
        Ravi Prakash
      5. HDFS-7995.006.patch
        8 kB
        Ravi Prakash
      6. HDFS-7995.005.patch
        8 kB
        Haohui Mai

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2320 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2320/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2320 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2320/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2344 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2344/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2344 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2344/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #380 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/380/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #380 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/380/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #397 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/397/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #397 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/397/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #404 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/404/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #404 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/404/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Yarn-trunk #1138 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1138/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Yarn-trunk #1138 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1138/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        Hide
        raviprak Ravi Prakash added a comment -

        Thank you Haohui!

        Show
        raviprak Ravi Prakash added a comment - Thank you Haohui!
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8466 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8466/)
        HDFS-7995. Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8466 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8466/ ) HDFS-7995 . Implement chmod in the HDFS Web UI. Contributed by Ravi Prakash and Haohui Mai. (wheat9: rev 07f304467d317d0de2cdf6ddda625ca103153146) hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/static/hadoop.css hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.html hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/webapps/hdfs/explorer.js
        Hide
        wheat9 Haohui Mai added a comment -

        I've committed the patch to trunk and branch-2. Thanks Ravi Prakash for the contribution.

        Show
        wheat9 Haohui Mai added a comment - I've committed the patch to trunk and branch-2. Thanks Ravi Prakash for the contribution.
        Hide
        wheat9 Haohui Mai added a comment -

        +1. I'll commit it shortly.

        Show
        wheat9 Haohui Mai added a comment - +1. I'll commit it shortly.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 release audit 0m 16s The applied patch does not increase the total number of release audit warnings.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
            0m 19s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12756301/HDFS-7995.006.patch
        Optional Tests  
        git revision trunk / bf2f2b4
        Java 1.7.0_55
        uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12487/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 release audit 0m 16s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 0s The patch has no lines that end in whitespace.     0m 19s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12756301/HDFS-7995.006.patch Optional Tests   git revision trunk / bf2f2b4 Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12487/console This message was automatically generated.
        Hide
        raviprak Ravi Prakash added a comment -

        Thanks for those modifications Haohui! Here's another patch fixing indentation and replacing "File" with "User"

        Show
        raviprak Ravi Prakash added a comment - Thanks for those modifications Haohui! Here's another patch fixing indentation and replacing "File" with "User"
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 release audit 0m 15s The applied patch does not increase the total number of release audit warnings.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
            0m 19s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12756155/HDFS-7995.005.patch
        Optional Tests  
        git revision trunk / 2ffe2db
        Java 1.7.0_55
        uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12469/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 release audit 0m 15s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 0s The patch has no lines that end in whitespace.     0m 19s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12756155/HDFS-7995.005.patch Optional Tests   git revision trunk / 2ffe2db Java 1.7.0_55 uname Linux asf903.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12469/console This message was automatically generated.
        Hide
        wheat9 Haohui Mai added a comment -

        There is no need to give every single checkbox an id, and it can be annoying to use a modal dialog considering (1) there is not much information to show, and (2) the amount of mouse movement can be huge.

        I uploaded a patch to simplify the code and replace the modal dialog with a popover.

        Show
        wheat9 Haohui Mai added a comment - There is no need to give every single checkbox an id, and it can be annoying to use a modal dialog considering (1) there is not much information to show, and (2) the amount of mouse movement can be huge. I uploaded a patch to simplify the code and replace the modal dialog with a popover.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
            0m 27s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12756137/HDFS-7995.04.patch
        Optional Tests  
        git revision trunk / 77aaf4c
        Java 1.7.0_55
        uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12464/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 0s The patch has no lines that end in whitespace.     0m 27s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12756137/HDFS-7995.04.patch Optional Tests   git revision trunk / 77aaf4c Java 1.7.0_55 uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12464/console This message was automatically generated.
        Hide
        raviprak Ravi Prakash added a comment -

        Rebased patch after HDFS-7986 (delete)

        Show
        raviprak Ravi Prakash added a comment - Rebased patch after HDFS-7986 (delete)
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 release audit 0m 17s The applied patch does not increase the total number of release audit warnings.
        +1 whitespace 0m 1s The patch has no lines that end in whitespace.
            0m 21s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12754983/HDFS-7995.03.patch
        Optional Tests  
        git revision trunk / 4014ce5
        Java 1.7.0_55
        uname Linux asf907.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12361/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 release audit 0m 17s The applied patch does not increase the total number of release audit warnings. +1 whitespace 0m 1s The patch has no lines that end in whitespace.     0m 21s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12754983/HDFS-7995.03.patch Optional Tests   git revision trunk / 4014ce5 Java 1.7.0_55 uname Linux asf907.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12361/console This message was automatically generated.
        Hide
        raviprak Ravi Prakash added a comment -

        Thanks for your review Haohui Mai. Here is a new patch incorporating all your suggestions.

        Show
        raviprak Ravi Prakash added a comment - Thanks for your review Haohui Mai . Here is a new patch incorporating all your suggestions.
        Hide
        wheat9 Haohui Mai added a comment -

        Sorry for the late reply. Thanks for the work.

        +  <div class="modal" id="perm-info" tabindex="-1" role="dialog" aria-hidden="true">
        

        It makes sense to give the id a prefix (e.g. explorer) to avoid confusion. The same comment applies to things like perm-heading, etc.

        +            <td><span class="explorer-perm-links editable-click">
        +                  {type|helper_to_directory}{permission|helper_to_permission}
        +                  {aclBit|helper_to_acl_bit}
        +                </span></td>
        
        -            <td><a style="cursor:pointer" inode-type="{type}" class="explorer-browse-links" inode-path="{pathSuffix}">{pathSuffix}</a></td>
        +            <td><a style="cursor:pointer" inode-type="{type}" class="explorer-browse-links">{pathSuffix}</a></td>

        The change seems unnecessary.

        +  function view_perm_details(filename, abs_path, perms) {
        

        There is no need to parse the permission from the string as the original data is available in the LISTSTATUS call. What you can do is to expose it through a data field. e.g.,

        +          <tr inode-path="{pathSuffix}" data-permission="{permission}">
        
        +  function convertCheckboxesToOctalPermissions() {
        

        It is easier to calculate the permission by expose the location of the bit using an attribute. e.g.,

        var p = 0;
        $.each('perm inputbox:checked').function() {
          p += 1 << (+$(this).attr('data-bit'));
        }
        return p.toString(8);
        
        Show
        wheat9 Haohui Mai added a comment - Sorry for the late reply. Thanks for the work. + <div class= "modal" id= "perm-info" tabindex= "-1" role= "dialog" aria-hidden= " true " > It makes sense to give the id a prefix (e.g. explorer ) to avoid confusion. The same comment applies to things like perm-heading , etc. + <td><span class= "explorer-perm-links editable-click" > + {type|helper_to_directory}{permission|helper_to_permission} + {aclBit|helper_to_acl_bit} + </span></td> - <td><a style= "cursor:pointer" inode-type= "{type}" class= "explorer-browse-links" inode-path= "{pathSuffix}" >{pathSuffix}</a></td> + <td><a style= "cursor:pointer" inode-type= "{type}" class= "explorer-browse-links" >{pathSuffix}</a></td> The change seems unnecessary. + function view_perm_details(filename, abs_path, perms) { There is no need to parse the permission from the string as the original data is available in the LISTSTATUS call. What you can do is to expose it through a data field. e.g., + <tr inode-path= "{pathSuffix}" data-permission= "{permission}" > + function convertCheckboxesToOctalPermissions() { It is easier to calculate the permission by expose the location of the bit using an attribute. e.g., var p = 0; $.each('perm inputbox:checked').function() { p += 1 << (+$( this ).attr('data-bit')); } return p.toString(8);
        Hide
        wheat9 Haohui Mai added a comment -

        I think the code requires some more clean up on unused ids in the HTML. Maybe we should replace the closest() call with something more performant.

        Show
        wheat9 Haohui Mai added a comment - I think the code requires some more clean up on unused ids in the HTML. Maybe we should replace the closest() call with something more performant.
        Hide
        hadoopqa Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 0m 0s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 release audit 0m 14s The applied patch does not increase the total number of release audit warnings.
            0m 20s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12728954/HDFS-7995.02.patch
        Optional Tests  
        git revision trunk / 5190923
        Java 1.7.0_55
        uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10442/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 0m 0s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 release audit 0m 14s The applied patch does not increase the total number of release audit warnings.     0m 20s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12728954/HDFS-7995.02.patch Optional Tests   git revision trunk / 5190923 Java 1.7.0_55 uname Linux asf906.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/10442/console This message was automatically generated.
        Hide
        aw Allen Wittenauer added a comment -

        +1 lgtm

        Show
        aw Allen Wittenauer added a comment - +1 lgtm
        Hide
        raviprak Ravi Prakash added a comment -

        Thanks for the review Allen. I've fixed the alignment.
        For regexp / recursive I'll have to implement FsShell functionality. I would like to do that as a separate JIRA

        Show
        raviprak Ravi Prakash added a comment - Thanks for the review Allen. I've fixed the alignment. For regexp / recursive I'll have to implement FsShell functionality. I would like to do that as a separate JIRA
        Hide
        aw Allen Wittenauer added a comment -

        I talked to Ravi about this in person. Putting a comment here so it is open and official:

        • Modal is fine.
        • Alignment is off.
        • Really need regex and recursive for multiple changes at once (single directory with lots of items as well as large hierarchical dirs.)
        Show
        aw Allen Wittenauer added a comment - I talked to Ravi about this in person. Putting a comment here so it is open and official: Modal is fine. Alignment is off. Really need regex and recursive for multiple changes at once (single directory with lots of items as well as large hierarchical dirs.)
        Hide
        wheat9 Haohui Mai added a comment -

        I'm not against a modal dialog. My feeling that this is an overuse in terms of UX in the use case of changing permissions.

        Show
        wheat9 Haohui Mai added a comment - I'm not against a modal dialog. My feeling that this is an overuse in terms of UX in the use case of changing permissions.
        Hide
        raviprak Ravi Prakash added a comment -

        Hi Haohui! This kind of dialog has been selected by several GUIs. We will also need to extend it for supporting ACLs too. I feel a modal is unavoidable.

        Show
        raviprak Ravi Prakash added a comment - Hi Haohui! This kind of dialog has been selected by several GUIs. We will also need to extend it for supporting ACLs too. I feel a modal is unavoidable.
        Hide
        wheat9 Haohui Mai added a comment -

        Sorry for the delay.

        It looks to me that it is somewhat noisy from the UX prospective as it needs a big modal dialog to update the permission. What about letting the user to update the permission string in place through X-editable?

        Show
        wheat9 Haohui Mai added a comment - Sorry for the delay. It looks to me that it is somewhat noisy from the UX prospective as it needs a big modal dialog to update the permission. What about letting the user to update the permission string in place through X-editable?
        Hide
        raviprak Ravi Prakash added a comment -

        Hi Haohui Mai Would you be able to review this patch?

        Show
        raviprak Ravi Prakash added a comment - Hi Haohui Mai Would you be able to review this patch?
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12707606/HDFS-7995.01.patch
        against trunk revision 61df1b2.

        +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 applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. There were no new javadoc warning messages.

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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-hdfs-project/hadoop-hdfs.

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

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12707606/HDFS-7995.01.patch against trunk revision 61df1b2. +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 applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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-hdfs-project/hadoop-hdfs. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/10081//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/10081//console This message is automatically generated.
        Hide
        raviprak Ravi Prakash added a comment -

        A couple of notes:

        1. I tried passing &recursive=true in the WebHDFS request but that doesn't seem to produce any effect . Perhaps we can extend WebHDFS in another JIRA and then its just a few lines of adding that checkbox too (which I have ready).
        2. To indicate that the permission field is editable I would like to use the same dashed underline line as Chown . This would become active after HDFS-7796 goes in
        3. I am sneaking in a small change to change text (instead of html) of the createDirectory tab to prevent XSS.
        Show
        raviprak Ravi Prakash added a comment - A couple of notes: I tried passing &recursive=true in the WebHDFS request but that doesn't seem to produce any effect . Perhaps we can extend WebHDFS in another JIRA and then its just a few lines of adding that checkbox too (which I have ready). To indicate that the permission field is editable I would like to use the same dashed underline line as Chown . This would become active after HDFS-7796 goes in I am sneaking in a small change to change text (instead of html) of the createDirectory tab to prevent XSS.
        Hide
        raviprak Ravi Prakash added a comment -

        Here's a patch which adds only the functionality for changing permissions. Haohui Mai I went through your earlier comments and tried my best to follow all your guidelines for this patch too so hopefully it should be less painful for you. Could you please review it?

        Show
        raviprak Ravi Prakash added a comment - Here's a patch which adds only the functionality for changing permissions. Haohui Mai I went through your earlier comments and tried my best to follow all your guidelines for this patch too so hopefully it should be less painful for you. Could you please review it?

          People

          • Assignee:
            raviprak Ravi Prakash
            Reporter:
            raviprak Ravi Prakash
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development