drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sohami <...@git.apache.org>
Subject [GitHub] drill pull request #1241: DRILL-6364: Handle Cluster Info in WebUI when exis...
Date Wed, 02 May 2018 06:53:34 GMT
Github user sohami commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1241#discussion_r185392204
  
    --- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
    @@ -103,14 +105,14 @@
                     </td>
                     <td id="status" >${drillbit.getState()}</td>
                     <td class="uptime" >Not Available</td>
    -                  <td>
    -                <#if ( model.shouldShowAdminInfo() &&  ( drillbit.isCurrent()
|| ( !model.isAuthEnabled() && location.protocol != "https" ))) >
    -                      <button type="button" id="shutdown" onClick="shutdown($(this),
'${drillbit.getAddress()}:${drillbit.getHttpPort()}');">
    -                <#else>
    +                <td>
    +                  <#if ( model.shouldShowAdminInfo() || !model.isAuthEnabled() ||
drillbit.isCurrent() ) >
    --- End diff --
    
    I think what is required here is to show shutdown button both for local and remote Drillbits
only to admin users. But the button will be hidden and not enabled to click. Then later logic
in [line 317](https://github.com/kkhatua/drill/blob/ab3e8619c6259803eb362be290a3a3605839a194/exec/java-exec/src/main/resources/rest/index.ftl#L317)
will enable the button.
    
    But with current check even if user is non-Admin it will show the button. Check will be
as below:
    
    ```
    <#if (!model.isAuthEnabled() || model.shouldShowAdminInfo())>
        <#if drillbit.isCurrent()>
            <button type="button" id="shutdown" onClick="shutdown($(this));" disabled="true"
style="opacity:0.5;cursor:not-allowed;">
        <#else>
            <button type="button" id="shutdown" title="Drillbit cannot be shutdown remotely"
disabled="true" style="opacity:0.5;cursor:not-allowed;">
        </#if>
    </#if>
    ```
    
    Please test with auth enabled and non-admin user too. Sharing example snapshot below.
    
    <img width="1271" alt="screen shot 2018-05-01 at 9 42 37 pm" src="https://user-images.githubusercontent.com/22159459/39505653-a37c1326-4d88-11e8-99c7-d8ab5495c7cb.png">



---

Mime
View raw message