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!
---