You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by echarles <gi...@git.apache.org> on 2016/08/31 15:50:41 UTC
[GitHub] zeppelin pull request #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and ...
GitHub user echarles opened a pull request:
https://github.com/apache/zeppelin/pull/1390
[WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and services per user
### What is this PR for?
This implements "notes per user" and "Multiple-users on a single WEB server" - For implementation reasons (the static fields present on the zeppelin server, and the need for each users to bind/configure their own note to the interpereters managed by the InterpreterFactory), this PR implements both - In other words, it would have been too difficult to decouple both).
### What type of PR is it?
[Feature]
### Todos
* [ ] - More test
* [ ] - Documentation
* [ ] - Migration (notebook folder structure changes)
* [ ] - Discuss the S3, Azure... repo cases
* [ ] - Check unit tests
* [ ] - Forsee a "shared" notebook workspace to mimick current behavior
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-1339
* https://issues.apache.org/jira/browse/ZEPPELIN-1000
### How should this be tested?
Logon / Logout with different users and check that the notes are different.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? N
* Is there breaking changes for older versions? Y
* Does this needs documentation? Y
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/datalayer/zeppelin-datalayer multiuser
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1390.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 #1390
----
commit 60c9380c3545e294ae8e776275b628899738cfc1
Author: Eric Charles <er...@datalayer.io>
Date: 2016-08-31T15:43:09Z
WIP Initial working implementation
----
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
this approach is a bit more breaking in terms of changes as well as compatibility with older versions (at least on notebook and storage level). i was working more on a filtering approach on a notes based on permissions which is backward compatible and currently WIP under #1392 . possibly we could discuss each approach's pros and cons and come up with something better.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles you can also push gradually to facilitate review process
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@khalidhuseynov
- have fixed the format issues
- I didn't change Notebook#getNotebookAuthorization, it was already present. However, a single global NotebookAuthorization as you like will change this. Will push something that moves the NotebookAuthorization out of the Notebook.
- The ZEPPELIN_AUTH_USER_KEY is used to remove the object from the user WEB session when user session expires... (to free memory and avoid OOM) however it is not yet used to put the objects... Will also push something.
@corneadoug I will write asap a small document explaining the logic behing this PR (interpreter factory by user, runAs...).
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/1390
I have introduced a parameter in the URL `?runAs=...` that allows to impersonate from an authenticated user another user. The needed R/W authorization are respected.
An alternative would be to intercalate the user in the URL: http://localhost/#/userName/noteId
Regarding multiple owners, I would simply drop this feature (just like a file, a note belongs to a unique user).
Apart minor navigation glitches, it works pretty well here.
I continue to follow #1265 and try to keep this PR orthogonal to what is discussed/implemented in there.
I will add a note regarding ZEPPELIN-1144 Zeppelin home page should only list notebooks with read or write permission #1330 - If listing all notes with read access is mandatory, I would have to fallback to the single `notebook-authorization.json`, or making the NotebookAuthorization service independent from the Notebook object.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles I've looked around your PR. I think your PR makes Zeppelin support separate users within a same instance. I have two questions regarding current behaviour. The first is about collaborative mode. If some users want to share their notebook to others which are able to login in same Zeppelin instance, and enable them to run it, how can user do that? The second one is about running interpreter instance. For now, Zeppelin supports three mode for different notes to run an interpreter, 'shared', 'scoped', 'isolated'. But in your PR, because you make interpreterFactory divided by users, all interpreters run a 'isolated' mode for all users. This means that if hundred users run simple markdown interpreter, Zeppelin launches same number of JVM on a same host. My PR considers that situation but your PR divide interpreterFactory by user, then will break my consideration. What do you think of it?
And complexity of configuration, @corneadoug will help to make intuitive menu.
---
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] zeppelin pull request #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1390
---
[GitHub] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles
Could you elaborate little bit about what will happen on the file system when notebook has multiple owners?
Concept of per user interpreter-setting.json seems conflict with "Run interpreter per user" https://github.com/apache/zeppelin/pull/1265. Could you explain little bit about this, too?
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@zjffdu Correct, this PR also implements ZEPPELIN-1338 User level interpreter setting (I have updated the title and description on this PR).
@Leemoonsoo Will check and adapt if needed the behavior for multiple owners and will further digg into #1265 to assess any conflit/divergence. I'd like to second the comment made by @zjffdu in #1265 on the interpreter options complexity. Btw, at the time of the introduction of the current interpreter modes, I already raised my worries on the complexity of those and honestly I am not convinced that I understand them fully (at least at first sight without digging into the code) and I fear that #1265 will make the situation still more complex. Taking back @zjffdu statement: *Besides, I think we should think more deeply on the relationship between note, user and interpreter.*, and I would add also the relation with the SearchService, Helium... and any other services that Zeppelin uses (cc/ @jongyoul) - All these questions also binds to ZEPPELIN-1236 "Multi-user notebook with user controls support" created by @frosiere talking about shared dashboard... Having this discussion on the ma
iling list independently of any PR would better define the efforst to be taken in each PR.
@khalidhuseynov I have the bad habit to build with `-Dcheckstyle.skip=true` - I commit a fix in the next hour.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
so build failed at least because of checkstyle violations. here are some to fix:
```
[ERROR] ZeppelinConfiguration.java[327] (sizes) LineLength: Line is longer than 100 characters (found 106).
[ERROR] ZeppelinConfiguration.java[363] (sizes) LineLength: Line is longer than 100 characters (found 110).
[ERROR] ZeppelinConfiguration.java[375] (sizes) LineLength: Line is longer than 100 characters (found 111).
[ERROR] Notebook.java[152] (sizes) LineLength: Line is longer than 100 characters (found 105).
[ERROR] Notebook.java[638] (sizes) LineLength: Line is longer than 100 characters (found 104).
[ERROR] GitNotebookRepo.java[60] (sizes) LineLength: Line is longer than 100 characters (found 112).
[ERROR] S3NotebookRepo.java[85] (sizes) LineLength: Line is longer than 100 characters (found 111).
[ERROR] VFSNotebookRepo.java[63] (sizes) LineLength: Line is longer than 100 characters (found 112).
```
Another question is related to what @Leemoonsoo mentioned regarding multiple owners. Actually that's broader question, say user1 added user2 as the reader, or writer for his noteA. then we still need to control each user's view based on note permissions.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@khalidhuseynov Good to discuss with you about what a multiuser note management would be.
For the file system repo, this PR will give you:
```
\u251c\u2500\u2500 anonynmous
\u2502�� \u251c\u2500\u2500 2A94M5J1Z
\u2502�� \u2502�� \u2514\u2500\u2500 note.json
\u2502�� \u251c\u2500\u2500 2BQA35CJZ
\u2502�� \u2502�� \u2514\u2500\u2500 note.json
\u2502�� \u251c\u2500\u2500 r
\u2502�� \u2502 \u2514\u2500\u2500 note.json
\u2502�� \u2514\u2500\u2500 interpreter-setting.json
\u251c\u2500\u2500 user1
\u2502�� \u251c\u2500\u2500 2BWADFP17
\u2502�� \u2502�� \u2514\u2500\u2500 note.json
\u2502�� \u2514\u2500\u2500 interpreter-setting.json
\u2514\u2500\u2500 user2
\u251c\u2500\u2500 2BU5DAHBJ
\u2502�� \u2514\u2500\u2500 note.json
\u2514\u2500\u2500 interpreter-setting.json
```
The note folders are moved one level-down, each user having its workspace.
Each user also has its own `interpreter-setting.json` (was before in the `conf` folder) - Just like before, each interpreter contribute to the settings via classpath or file system.
Interpreter settings and bindings assignment is made possible by keeping separate interpreterfactory, searchservice... per web user session.
To ensure backwards compatibility, I propose (not yet implemented) a `zeppelin.notes.per-user` configuration being `false` by default.
The other repository implements (azure, github...) need more love with the credential service (each user can have their own credentials).
PS: I had a quick look at #1392 - Let's continue this discussion - Depending on the outcomes, we can decide `what`, `who` and `where` - About the `when`, the soone the better.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1390
I didn't look through the PR, but according @jongyoul 's last comments. If only `isolated` mode is supported, then I don't think is a good idea. We should not only consider isolatation of each user, but also need to take considerion of collobration. The most important thing is about how to share data between users. I think that is why we introduce `scoped` mode.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by echarles <gi...@git.apache.org>.
Github user echarles commented on the issue:
https://github.com/apache/zeppelin/pull/1390
I have a prototype that supports the 3 modes. Give me a few days to polish and commit.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles I also believe having global/single NotebookAuthorization module would be more relevant
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles Did you also implement interpreter setting per user in this PR as described in ZEPPELIN-1338 ?
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
since this also related to #1265 as well as [ZEPPELIN-1338](https://issues.apache.org/jira/browse/ZEPPELIN-1338) for interpreter multi user environment, maybe @jongyoul could take more detailed look here as well to see possible design, performance, resource utilisation issues
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles also would you mind rebasing from 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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] [ZEPPELIN-1338] ...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1390
@echarles can you explain more about the `?runAs=`? I don't really see what this is used for.
---
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] zeppelin issue #1390: [WIP] [ZEPPELIN-1339] [ZEPPELIN-1000] Note and service...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/1390
AFAIK, this issue is already assigned @khalidhuseynov, and he is in progress. @khalidhuseynov Can you share your status?
---
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.
---