Skip to content

Comments

[WIP Zeppelin-945] Interpreter authorization#1223

Closed
astroshim wants to merge 15 commits intoapache:masterfrom
astroshim:ZEPPELIN-945
Closed

[WIP Zeppelin-945] Interpreter authorization#1223
astroshim wants to merge 15 commits intoapache:masterfrom
astroshim:ZEPPELIN-945

Conversation

@astroshim
Copy link
Contributor

What is this PR for?

This PR is for interpreter authorization.

What type of PR is it?

Improvement

Todos

  • - only show the lock button if admin login.

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-945

How should this be tested?

  1. login as admin
  2. then you can see the lock button on the right top as following image.
    image
  3. you can give users permission as check on the checkbox you want.
  4. run paragraph job as user and see results if job has run or not properly.

Screenshots (if appropriate)

image

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? yes

@Leemoonsoo
Copy link
Member

@astroshim Thanks for the contribution.

Regarding UI, why don't we use the same UI with notebook authorization?
That can give the same user experience for all the authorization inside of Zeppelin.

Than it'll look something like

image

What do you think?

@astroshim
Copy link
Contributor Author

@Leemoonsoo Thank you for your review and I think your opinion makes sense.
I'll change the UI.

@prabhjyotsingh
Copy link
Contributor

@Leemoonsoo I would say, since the use case for this is different than that of notebook, and idea for this is to map access to users with interpreters.

While reading the title/description of this PR this is what I had in my mind

user setting

Also think about the case where user is getting auth using LDAP/AD where there are 100s of user, so this will require pagination. So, we need to have a group/profile for users that can have preset for grouping interpreters together.

user profile

And once this is set, IMO, then user who don't have permission to a particular interpreter, should not see that interpreter in the notebook page.

@astroshim
Copy link
Contributor Author

@prabhjyotsingh Thank you for share your great idea.
This PR is for interpreter authorization but your idea seems include user management.
I think it should be separated and I agree with @Leemoonsoo's idea of giving the same user experience for the authorization inside of Zeppelin.
and I also agree with @prabhjyotsingh's idea of the difference with notebook's because interpreter authorization is n:n. (it's hard to show with notebook's way.)
so What do you guys think about following way?

  • when you want to grant permission of interpreters to all users
    image
  • when you want to grant permission of interpreters to specific user
    image

@astroshim
Copy link
Contributor Author

I'll do this at the another PR.

@astroshim astroshim closed this Aug 2, 2016
asfgit pushed a commit that referenced this pull request Aug 16, 2016
### What is this PR for?
This PR is derived from #1223

### What type of PR is it?
Improvement

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-945

### How should this be tested?
You can change user permission of interpreter at interpreter setting page.

### Screenshots (if appropriate)
![result](https://cloud.githubusercontent.com/assets/3348133/17457867/904e2754-5c3e-11e6-948c-b2969c4b89c7.gif)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: astroshim <hsshim@nflabs.com>

Closes #1257 from astroshim/ZEPPELIN-945_1 and squashes the following commits:

83097ab [astroshim] fix naming.
2c48ded [astroshim] code refactor.
7ed8ad6 [astroshim] fix js code style
3e25159 [astroshim] move directive to componets
f07542a [astroshim] remove comment
febce0c [astroshim] fix js
e42cb9e [astroshim] fix initialize user value
e1e7a35 [astroshim] add interpreter create.
e72c097 [astroshim] Merge branch 'master' into ZEPPELIN-945_1
e1679b3 [astroshim] wip
e56e5b1 [astroshim] wip
635d523 [astroshim] Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-945_1
1ae1c6a [astroshim] fix for @AhyoungRyu pointed.
b34fdf7 [astroshim] fix for checking intpname
29d8f43 [astroshim] fix js
407d260 [astroshim] fit UX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants