Skip to content

Conversation

@prabhjyotsingh
Copy link
Contributor

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?

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

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 prabhjyotsingh changed the title ZEPPELIN-1483 [ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission Sep 23, 2016
@prabhjyotsingh
Copy link
Contributor Author

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] ------------------------------------------------------------------------

import java.io.IOException;
import java.io.StringReader;
import java.util.*;
import java.util.concurrent.TimeUnit;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed if we include java.util.*

import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.user.Credentials;
import org.quartz.*;
import org.quartz.impl.StdSchedulerFactory;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed if we include org.quartz.*

import org.slf4j.LoggerFactory;

import javax.ws.rs.*;
import javax.ws.rs.core.Response;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@prabhjyotsingh
Copy link
Contributor Author

Thank you @corneadoug, have fixed it.

@khalidhuseynov
Copy link
Member

khalidhuseynov commented Sep 28, 2016

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
Copy link
Contributor Author

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
@asfgit asfgit closed this in 9e9ea3a Oct 20, 2016
darionyaphet pushed a commit to darionyaphet/zeppelin that referenced this pull request Oct 27, 2016
…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
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
…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 prabhjyotsingh deleted the ZEPPELIN-1483 branch February 25, 2018 03:47
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