You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by prabhjyotsingh <gi...@git.apache.org> on 2016/02/09 09:20:34 UTC
[GitHub] incubator-zeppelin pull request: User in interpreter context
GitHub user prabhjyotsingh opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/705
User in interpreter context
### What is this PR for?
The goal of the PR is to pass userName/userInfo from front-end to interpreters.
The PR uses Shiro authentication(https://github.com/apache/incubator-zeppelin/pull/586).
### What type of PR is it?
Improvement
### Todos
* [x] - Change RemoteInterpreterService.thrift signature
* [x] - Change InterpreterContext/RemoteInterpreterContext signature
* [x] - Modify all existing interpreters to above signature
### Is there a relevant Jira issue?
N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/prabhjyotsingh/incubator-zeppelin UserInInterpreterContext
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/705.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 #705
----
commit fadc6d92ffabc0dc4cb2079702ad292df3fd070a
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2016-02-08T16:29:13Z
userName to be present in InterpreterContext/RemoteInterpreterContext
commit d928203cf6d69b3c6427aa35f047b25a399b3198
Author: Prabhjyot Singh <pr...@gmail.com>
Date: 2016-02-08T16:48:37Z
revert shiri.ini
if fromMessage.principal.equals("anonymous") then set user as null
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-183166585
This is ready for review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-186915557
Let's rebase and merge this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-183499201
@rconline how would you want to discuss? Should we open a JIRA - or a JIRA parent with subtests for this multi-user auth use cases?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-186065829
Thanks you @Leemoonsoo, that does make a lot of sense.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by rconline <gi...@git.apache.org>.
Github user rconline commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-183897757
@felixcheung @prabhjyotsingh The purpose of this discussion should be to get to an approach that allows us zeppelin to do a couple of things - pass user identity to the underlying spark cluster, and thereby support multi-tenancy. There is JIRA open - https://issues.apache.org/jira/browse/ZEPPELIN-645 which we could use to discuss this further. Does that make sense?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by rconline <gi...@git.apache.org>.
Github user rconline commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-181871112
This is a requisite first step for getting to multi-user scenarios. I believe this should get discussed a little bit in terms of approach.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-185582355
I have tried and it works well.
One thing is, `org.apache.zeppelin.display` package supposed to have Display System related codes.
I think `AuthenticationInfo` is less related to Display System. Could you suggest other package name?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/705
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-186360054
Thanks @prabhjyotsingh.
It looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-186989791
Thank you for reviewing this. Have merged this with master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-186631173
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-183990823
I think it make sense to let access authentication info through InterpreterContext.
But i think authentication info will be more than a user name in the future. It can have some credentials/token stuff from Authentication system (shiro), so interpreter process can reuse it without asking authentication again, when they're required.
So my suggestion is, create such class `AuthenticationInfo`, and pass it into InterpreterContext instead of `String userName`, for further extension in the future.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-183823300
@prabhjyotsingh could you explain more on the reason for this change?
Also this seems like a somewhat big change - could you open a JIRA on this so it could be tracked?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: User in interpreter context
Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/705#issuecomment-184762793
Thank you for the suggestion @Leemoonsoo. Have added AuthenticationInfo class, and made relevant changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---