You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by necosta <gi...@git.apache.org> on 2018/01/23 15:25:38 UTC

[GitHub] zeppelin pull request #2742: [ZEPPELIN-3168] Interpreter Settings Authorizat...

GitHub user necosta opened a pull request:

    https://github.com/apache/zeppelin/pull/2742

    [ZEPPELIN-3168] Interpreter Settings Authorization

    ### What is this PR for?
    Attempts to introduce some control/authorization over interpreter settings.
    Added 2 levels of permissions: Owner and Reader
    Also added a new attribute to InterpreterProperty to prevent editing a value (UI only)
    
    
    ### What type of PR is it?
    [Feature]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-3168
    
    ### How should this be tested?
    * Test interpreter setting creation/update/deletion
    * Test interpreter setting property creation/update/deletion
    * Test interpreter settings API
    
    ### Screenshots (if appropriate)
    ![int_sett1](https://user-images.githubusercontent.com/26248959/35284003-95c9a592-0051-11e8-8835-0479449a82ed.png)
    
    
    ### Questions:
    * Does the licenses files need update? N
    * Is there breaking changes for older versions? N?
    * Does this needs documentation? Y


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nokia/zeppelin zeppelin3168

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/2742.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2742
    
----
commit 918c1d7d457a587ef71e4b51909389b333a585b5
Author: Nelson Costa <ne...@...>
Date:   2018-01-23T15:11:53Z

    [ZEPPELIN-3168] Interpreter Settings Authorization

----


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    ping @zjffdu , @Leemoonsoo , @mebelousov , @felixcheung 
    Many thanks


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    I'll close this PR because original repo was deleted


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    Hi @zjffdu , I didn't create a design doc but I hope the interpreter/overview.md file helps. 
    We can always improve the .md file of course.
    
    In a nutshell, I tried to use as much functionality as it exists, leveraging `set permissions` and re-using interpreter.json for persistence.
    
    As you can see in the screenshot above, I slightly changed the position and title of "New Interpreter" button so it's more recognizable for the end-user.
    
    I tested locally with basic Shiro setup and disabling `zeppelin.anonymous.allowed`.
    
    I don't think there are any breaking changes but more tests are needed to be sure.
    
    



---

[GitHub] zeppelin issue #2742: [WIP] [ZEPPELIN-3168] Interpreter Settings Authorizati...

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    Q: If user create new interpreter property, is it readonly by default or not ?
    A: Editable by default


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    FYI, got this one rebased on top of latest master. Happy to wait for 0.8.0 release before this MR gets reviewed


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    I have a very basic question. Does this feature prevent from changing configurations by `ConfInterpreter` dynamically?


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by Tagar <gi...@git.apache.org>.
Github user Tagar commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    @necosta thanks for this great feature. It makes a lot of sense for Zeppelin instance shared with different set of users. Would it be possible to do this interpreter settings authorization for LDAP groups too, not just for users? 


---

[GitHub] zeppelin pull request #2742: [ZEPPELIN-3168] Interpreter Settings Authorizat...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/2742


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    A few more changes:
    - Exposed interpreter property `Description`
    - Exposed interpreter property `Readonly`
    - Added interpreter option `disallowCustomInterpreter` (Prevents users from using interpreter group to create a new interpreter)
    - Validate permissions on selecting interpreters
    - Disallow deletion of property when readonly
    - Check for duplicate properties


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    sorry @necosta I will review it after 0.8.0 release


---

[GitHub] zeppelin issue #2742: [WIP] [ZEPPELIN-3168] Interpreter Settings Authorizati...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    If user create new interpreter property, is it readonly by default or not ?


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    I agree. It should do


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    No problem @zjffdu ! Didn't know we were releasing 0.8.0. Happy we hear that :)


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    This PR seems didn't touch ConfInterpreter, but it should do I believe. 


---

[GitHub] zeppelin issue #2742: [ZEPPELIN-3168] Interpreter Settings Authorization

Posted by necosta <gi...@git.apache.org>.
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2742
  
    ping @zjffdu , @Leemoonsoo , @mebelousov ,
    Feedback appreciated :) Thanks!


---