Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ZEPPELIN-4635]. Save note permission info into notebook-authorization.json #3668

Closed
wants to merge 1 commit into from
Closed

[ZEPPELIN-4635]. Save note permission info into notebook-authorization.json #3668

wants to merge 1 commit into from

Conversation

zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Feb 29, 2020 •
edited

What is this PR for?

This PR is to revert the changes of ZEPPELIN-3985. Because it would cause the issue of ZEPPELIN-4612. In this PR I will make Zeppelin still save note permission info into notebook-authorization.json. But I also do some code refactoring, and we could store the permission info into other storage such as database in future. Because storing them into one file also has potential issue. such as scale issue, now each time we have to write all notes' permission info into file instead of in the note level.

What type of PR is it?

[Bug Fix | Refactoring]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

  • CI pass

Screenshots (if appropriate)

Questions:

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

@zjffdu zjffdu force-pushed the ZEPPELIN-4635 branch 5 times, most recently from d4c9e1d to fc635dd Compare March 4, 2020 09:06
@zjffdu
Copy link
Contributor Author

zjffdu commented Mar 6, 2020

@Leemoonsoo You might want to know this change.

@Leemoonsoo
Copy link
Member

Thanks @zjffdu LGTM

@zjffdu
Copy link
Contributor Author

zjffdu commented Mar 8, 2020

Thanks @Leemoonsoo will merge if no more comments

@asfgit asfgit closed this in b6c6124 Mar 9, 2020
@Reamer Reamer mentioned this pull request Apr 2, 2020
Closed
asfgit pushed a commit that referenced this pull request Apr 4, 2020
@Reamer @zjffdu
[ZEPPELIN-4718] Fix Regression HDFS Notebook Storage
6c10bb2 
### What is this PR for?
Fixed a regression in #3668.
With this fix zeppelin can save the notebook in HDFS.

### What type of PR is it?
Regression Fix

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

### How should this be tested?
* **Travis-Link**: https://travis-ci.org/github/Reamer/zeppelin/builds/670126594

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

Author: Philipp Dallig <philipp.dallig@gmail.com>

Closes #3713 from Reamer/notebook_dir_hdfs and squashes the following commits:

0117609 [Philipp Dallig] Check for filesystem with a scheme
asfgit pushed a commit that referenced this pull request Apr 4, 2020
@Reamer @zjffdu
[ZEPPELIN-4718] Fix Regression HDFS Notebook Storage
abe6a95 
### What is this PR for?
Fixed a regression in #3668.
With this fix zeppelin can save the notebook in HDFS.

### What type of PR is it?
Regression Fix

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

### How should this be tested?
* **Travis-Link**: https://travis-ci.org/github/Reamer/zeppelin/builds/670126594

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

Author: Philipp Dallig <philipp.dallig@gmail.com>

Closes #3713 from Reamer/notebook_dir_hdfs and squashes the following commits:

0117609 [Philipp Dallig] Check for filesystem with a scheme

(cherry picked from commit 6c10bb2)
Signed-off-by: Jeff Zhang <zjffdu@apache.org>
@jolo-dev
Copy link

Hi @zjffdu,
I am aware of the loading issue but it causes troubles on our end.
We have Zeppelin running on ECS and whenever we redeploy, the notebook-authorization.json is gone.
I guess a compromise needs to be handled.

@zjffdu
Copy link
Contributor Author

zjffdu commented Nov 10, 2020

@jolo-dev What version of zeppelin do you use ? It is a bug if the notebook-authorization.json is gone.

@jolo-dev
Copy link

jolo-dev commented Nov 10, 2020 •
edited

@zjffdu We upgraded to zeppelin-preview2.
But that's not a bug. It's just the notebook-authorization.json is not in the notebook storage but in a separate file.
So the best thing for us would be to put the permissions directly to note.

@zjffdu
Copy link
Contributor Author

zjffdu commented Nov 10, 2020

I see, actually ConfigStorage has 2 implementation:

  • LocalConfigStorage (store notebook-authorization.json in local filesystem)
  • FileSystemConfigStorage. (store notebook-authorization.json in hadoop compatible file system)

I think you can use FileSystemConfigStorage to resolve your issue.

@jolo-dev
Copy link

Well, we are not using hadoop. Is it possible to read this file from an S3 bucket?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@zjffdu @Leemoonsoo @jolo-dev