[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission by prabhjyotsingh · Pull Request #1454 · apache/zeppelin

prabhjyotsingh · 2016-09-23T14:25:45Z

What is this PR for?

Zeppelin home page list notebooks doesn't show notebook with group permission

What type of PR is it?

[Bug Fix]

Todos

- consume userAndRole instead of AuthenticationInfo

What is the Jira issue?

ZEPPELIN-1483

How should this be tested?

In current scenario only those notebook lists that have direct user permission, those with group does not list up, but if user have link to those notebook, it can still be accessed.
IMO the notebook with group permission should also be listed in the home screen.

Screenshots (if appropriate)

Questions:

Does the licenses files need update? n/a
Is there breaking changes for older versions? n/a
Does this needs documentation? n/a

prabhjyotsingh · 2016-09-26T06:14:04Z

Ready for review.
CI fails for #7346.9 with; which is unrelated

�[31m- should provide onclick method *** FAILED ***�[0m
�[31m  The code passed to eventually never returned normally. Attempted 1 times over 310.82543 milliseconds. Last failure message: 0 was not equal to 1. (AbstractAngularElemTest.scala:72)�[0m
�[32mAngularElem�[0m
�[32m- should print angular display directive only once in a paragraph�[0m
�[32mAngularElem�[0m
�[32m- should bind angularObject to ng-model directive�[0m
�[32mAngularElem�[0m
�[32m- should able to disassociate AngularObjects�[0m
�[32mAngularElem�[0m
�[32m- should allow access to InterpreterContext inside of callback function�[0m
�[32mAngularElem�[0m
�[32m- should able to be created from implicit conversion�[0m
�[32mAngularElemTest:�[0m
�[32mAngularElem�[0m
�[32m- should provide onclick method�[0m
�[32mAngularElem�[0m
�[32m- should print angular display directive only once in a paragraph�[0m
�[32mAngularElem�[0m
�[32m- should bind angularObject to ng-model directive�[0m
�[32mAngularElem�[0m
�[32m- should able to disassociate AngularObjects�[0m
�[32mAngularElem�[0m
�[32m- should allow access to InterpreterContext inside of callback function�[0m
�[32mAngularElem�[0m
�[32m- should able to be created from implicit conversion�[0m
�[36mRun completed in 1 second, 221 milliseconds.�[0m
�[36mTotal number of tests run: 18�[0m
�[36mSuites: completed 5, aborted 0�[0m
�[36mTests: succeeded 17, failed 1, canceled 0, ignored 0, pending 0�[0m
�[31m*** 1 TEST FAILED ***�[0m
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Zeppelin: Interpreter .............................. SUCCESS [ 12.937 s]
[INFO] Zeppelin: Zengine .................................. SUCCESS [  5.803 s]
[INFO] Zeppelin: Display system apis ...................... FAILURE [  2.906 s]
[INFO] Zeppelin: Spark dependencies ....................... SKIPPED
[INFO] Zeppelin: Spark .................................... SKIPPED
[INFO] Zeppelin: Server ................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 22.748 s
[INFO] Finished at: 2016-09-25T05:00:13+00:00
[INFO] Final Memory: 30M/463M
[INFO] ------------------------------------------------------------------------

corneadoug · 2016-09-26T06:35:43Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java

+import java.io.IOException;
+import java.io.StringReader;
+import java.util.*;
+import java.util.concurrent.TimeUnit;


Not needed if we include java.util.*

corneadoug · 2016-09-26T06:36:03Z

zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java

 import org.apache.zeppelin.user.AuthenticationInfo;
 import org.apache.zeppelin.user.Credentials;
+import org.quartz.*;
+import org.quartz.impl.StdSchedulerFactory;


Not needed if we include org.quartz.*

corneadoug · 2016-09-26T06:36:41Z

zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java

+import org.slf4j.LoggerFactory;
+
+import javax.ws.rs.*;
+import javax.ws.rs.core.Response;


Not needed if we include javax.ws.rs.*

prabhjyotsingh · 2016-09-26T18:19:06Z

Thank you @corneadoug, have fixed it.

khalidhuseynov · 2016-09-28T14:06:14Z

adding group permissions to listed notes makes sense. tested and works as expected. maybe would makes sense to add integration tests to differentiate the cases of user and groups, maybe in different PR as improvement. CI looks irrelevant, LGTM

prabhjyotsingh · 2016-09-29T02:50:45Z

Merging this if no more discussion.

# Conflicts: # zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java # zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java

…ok with group permission ### What is this PR for? Zeppelin home page list notebooks doesn't show notebook with group permission ### What type of PR is it? [Bug Fix] ### Todos * [x] - consume userAndRole instead of AuthenticationInfo ### What is the Jira issue? * [ZEPPELIN-1483](https://issues.apache.org/jira/browse/ZEPPELIN-1483) ### How should this be tested? In current scenario only those notebook lists that have direct user permission, those with group does not list up, but if user have link to those notebook, it can still be accessed. IMO the notebook with group permission should also be listed in the home screen. ### Screenshots (if appropriate) ![testgroup](https://cloud.githubusercontent.com/assets/674497/18789097/47c5a558-81c7-11e6-80e1-1d0bc42d0b17.gif) ### Questions: * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Prabhjyot Singh <[email protected]> Author: Prabhjyot Singh <[email protected]> Closes apache#1454 from prabhjyotsingh/ZEPPELIN-1483 and squashes the following commits: 2484833 [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1483 c8d810e [Prabhjyot Singh] organise imports d3261c4 [Prabhjyot Singh] consume userAndRole instead of AuthenticationInfo

prabhjyotsingh changed the title ~~ZEPPELIN-1483~~ [ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission Sep 23, 2016

consume userAndRole instead of AuthenticationInfo

d3261c4

prabhjyotsingh force-pushed the ZEPPELIN-1483 branch from f6a5aea to d3261c4 Compare September 25, 2016 04:46

corneadoug reviewed Sep 26, 2016

View reviewed changes

organise imports

c8d810e

Merge remote-tracking branch 'origin/master' into ZEPPELIN-1483

2484833

# Conflicts: # zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java # zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java

asfgit closed this in 9e9ea3a Oct 20, 2016

prabhjyotsingh deleted the ZEPPELIN-1483 branch February 25, 2018 03:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission#1454

[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission#1454
prabhjyotsingh wants to merge 3 commits intoapache:masterfrom
prabhjyotsingh:ZEPPELIN-1483

prabhjyotsingh commented Sep 23, 2016

Uh oh!

prabhjyotsingh commented Sep 26, 2016

Uh oh!

corneadoug Sep 26, 2016

Uh oh!

corneadoug Sep 26, 2016

Uh oh!

corneadoug Sep 26, 2016

Uh oh!

prabhjyotsingh commented Sep 26, 2016

Uh oh!

khalidhuseynov commented Sep 28, 2016 •

edited

Loading

Uh oh!

prabhjyotsingh commented Sep 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

prabhjyotsingh commented Sep 23, 2016

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

prabhjyotsingh commented Sep 26, 2016

Uh oh!

corneadoug Sep 26, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug Sep 26, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug Sep 26, 2016

Choose a reason for hiding this comment

Uh oh!

prabhjyotsingh commented Sep 26, 2016

Uh oh!

khalidhuseynov commented Sep 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prabhjyotsingh commented Sep 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

khalidhuseynov commented Sep 28, 2016 •

edited

Loading