Bind/Unbind interpreters to note #235

Leemoonsoo · 2014-12-07T05:30:32Z

Currently all interpreters are shared to all notes by default.
It'll be good If we can selectively bind/unbind available interpreters to note.

Ability to instantiate new interpreter and bind/unbind to note
API for bind/unbind interpreter
GUI Impelementation for bind/unbind interpreter
Save/load interpreter bindings
default property display
restore z.run() feature

…ure bind/unbind interpreters to note

…ess another one in the same group

Conflicts: zeppelin-server/src/main/java/com/nflabs/zeppelin/server/ZeppelinServer.java

Leemoonsoo · 2014-12-29T05:22:45Z

This PR implements a) ability to create interpreter instance with different configurations, b) bind/unbind created interpreters to note.

Interpreter settings

Interpreters can be instantiated with different configurations.
Here's screenshot of listing interpreter settings

Here's how screenshot of creating new interpreter setting

Interpreter bindings
Each note can configure which interpreter is going to be used.
Each note now have setting menu for interpreter bind/unbind. User can click to bind/unbind interpreter and reorder interpreters by drag&drop. (first interpreter becomes default one)

Screenshot.

Configuration
Zeppelin now creates configuration file 'conf/interpreter-settings.json'. The file persist/load information of interpreter settings and bindings. The file is automatically generated when it's not exists.

Any feedback is welcome.
Ready to merge!

…ontext

swkimme · 2014-12-29T07:02:00Z

Super Cool, works great!
I'll test more and post some feedback for UI improvements.

swkimme · 2015-01-12T02:34:17Z

I guess a.cons: 'using multiple instance of interpreter derived from the
same type in a single notebook' is quite rare case.

I prefer current implementation for simplicity. (also it's more intuitive.)
supplying name as option seems to be quite good alternative like moon
sugested, %spark(name='spark-local')

On Sun Jan 11 2015 at 10:45:27 PM Lee moon soo notifications@github.com
wrote:

@haebin https://github.com/haebin Thanks for nice suggestion!

I think there're pros and cons.

a. Current implementaion

Cons - In a single notebook, can not use multiple instance of interpreter
derived from the same type.
Pros - All zeppelin user will use the same %magic keyword for the same
interpreter type. (therefore notebook can be run in any user's Zeppelin
without %magic keyword modification)

b. Haebin proposed way
Pros - Can use multiple instance of interpreter derived from the same
type. in a single note.
Cons - In case notebook is copied from other, to run, need to modify
%magic keyword for local Zeppelin's setting name.

Guys what do you think? @haebin https://github.com/haebin, @swkimme
https://github.com/swkimme any good idea?

One compromise is using always %spark but giving some arguments like
%spark(name='spark-local')

—
Reply to this email directly or view it on GitHub
#235 (comment).

… InterpreterContext)

…ogress()

Leemoonsoo · 2015-01-12T13:17:51Z

Now, i changed behavior to minimize the UX changes according to @swkimme 's suggestion.
Newly created note now have automatically selected interpreter sets.
(Existing note that created before this feature will still be asked)

Also it'll be asked before discard any changes on note setting menu.

Right buttons are now aligned

About description in interpreter menu, i was trying to make it look alike table that explain application's properties. Those kind of table we can see all around, hadoop, spark, etc. (eg. https://spark.apache.org/docs/1.1.0/configuration.html#application-properties)

Leemoonsoo · 2015-01-13T04:57:34Z

Summary of this PR

New features

Creating / removing interpreter settings (interpreter instances)
Binding/unbinding interpreter to notes

Improvements

Align upper right icons in note page.

Bug fix

Resolve netty dependency confliction
Resolve NPE after Zeppelin restart when trying to to autocomplete
Cancel paragraph now cancels only specific paragraph, not all other pending paragraphs.

API Changes

Interpreter.interpret(String) -> Interpreter.interpret(String, InterpreterContext)
Interpreter.cancel() -> Interpreter.cancel(InterpreterContext)
Interpreter.getProgress() -> Interpreter.getProgress(InterpreterContext)

There're thing to be improved, like editing interpreter setting, use radio box instead of drag and drop to make default interpreter when binding interpreter, etc.
But all proposed features are working well and we can address improvements with separate issues.

Ready to merge

Leemoonsoo · 2015-01-14T03:00:17Z

About @haebin 's suggestion, i also agree on @swkimme that using the alternatives. %spark(name='spark-local')
But i want to handle that in separate issue, we can discuss more about it on mailing list.

Leemoonsoo · 2015-01-14T07:57:25Z

If there're no further issues to discuss, i'm merging it.

corneadoug · 2015-01-14T08:05:54Z

Go for it, but we should also create issues for the suggestions so that we don't forget about it

anthonycorbacho · 2015-01-14T08:14:17Z

Just wait, There is some part in the rest api i want to talk about.

anthonycorbacho · 2015-01-14T08:17:48Z

zeppelin-server/src/main/java/com/nflabs/zeppelin/rest/InterpreterRestApi.java

Please add swagger annotation for the documentation

anthonycorbacho · 2015-01-14T09:03:59Z

@Leemoonsoo where are the tests for the rest api ?

bzz · 2015-01-14T09:06:46Z

Great job @Leemoonsoo !
Looks good to me, except the conventions for REST API noted by @anthonycorbacho

Leemoonsoo · 2015-01-23T04:47:52Z

@anthonycorbacho I've changed REST API convention and added basic rest API test.
Ready to merge.

anthonycorbacho · 2015-01-23T05:21:26Z

@Leemoonsoo Looks better!!

Bind/Unbind interpreters to note

This makes it configurable to specify multiple origins as allowed (default only local origin is allowed). Wildcard origin will not be supported as it is a security vulnerability. It adds a compatibility check in configuration for windows paths. Upgrades servlet config to add httponly and secure which will secure session cookies if used. Author: joelz <djoelz@gmail.com> Author: djoelz <joelz@microsoft.com> Closes ZEPL#235 from djoelz/master and squashes the following commits: 989f1e0 [joelz] Retrying build as it seems ZeppelinIT failed for not reason. 625b54e [joelz] Fixing unit test that reads from a file but initializes to a default value and hence the configuration is present. e9d8384 [joelz] Retrying due to git download issue with build 2887f0d [joelz] Renaming tests to singular name so plugin can detect and run 9260d5d [joelz] Fixing adding the origin header for get and post tests. b7bb7bf [joelz] Fixing Styling b2b418a [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. 4ae9129 [joelz] Fixing null reference 3795de7 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. bcb1ac1 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. 3d6ce2e [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. 1f851c0 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. 7ecf7e9 [joelz] Merge branch 'master' of https://github.com/djoelz/incubator-zeppelin faa6204 [joelz] Merge branch 'apache-master' 52eb1bd [joelz] Merge branch 'master' of https://github.com/apache/incubator-zeppelin into apache-master 5ff1a47 [joelz] Merge branch 'masterOrigin' 47902a6 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. a00adc2 [djoelz] Merge pull request ZEPL#1 from apache/master df324de [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. cecbab8 [joelz] Fixing cross origin bug for rest calls that allow a malicious user to issue requests from a site other than the zeppelin server. Adding unit tests and a dependency to mockito to the server project (please comment if that is ok or if there is another preferred mocking framework). Also upgrading the servelet version from 2.5 to 3.0 as this also fixes a security vulnerability with respect to httonly cookies. 61e857d [joelz] Fixing Rest request lack of Origin validation bug, Added tests that use Mockito (unit test framework) and forces the servlet to use version 3.0 instead of 2.5 08ff369 [djoelz] unecessary file 013f22d [joelz] Fixing issue with ZEPPELIN-173: Zeppelin websocket server is vulnerable to Cross-Site WebSocket Hijacking ea54b55 [joelz] Fixing issue with ZEPPELIN-173: Zeppelin websocket server is vulnerable to Cross-Site WebSocket Hijacking

Leemoonsoo added 27 commits December 7, 2014 14:23

Refactoring InterpreterFactory and NoteInterpreterLoader for the feat…

1ef3b72

…ure bind/unbind interpreters to note

Implement RegisteredInterpreter class

60111f8

Implement InterpreterGroup

356c02f

Pass interpreter to interpreterGroup reference to let interpreter acc…

281418d

…ess another one in the same group

Add interpreter page

56190da

Remove singleton reference from SparkSqlInterpreter to SparkInterpreter

acd79a0

Add CorsFilter

c526683

Serialize interpreter to pass information to client

48d0786

Remove 'share' common object between interpreters

e04fb35

List available interpreters

fa4e154

Make remove/restart functioning

788741c

load/save interpreter settings

b6f8f7e

Merge branch 'master' into interpreter_binding

0767f4e

Conflicts: zeppelin-server/src/main/java/com/nflabs/zeppelin/server/ZeppelinServer.java

Interpreter setting add API and GUI

a9a8a2a

Add NewInterpreterSettingRequest class

570d205

Bind/unbind reorder interpreter to note

6ef8c3b

Fix netty dependency confliction

bccff9e

Display interpreter names

82201ff

Pass classloaderUrls to interpreter using it's own getter/setter

07fde9f

Bumpup angular version

b23b896

Load/save interpreter bindings

7cb12d0

Automatic interpreter binding suggestion when nothing selected

f5d9fad

Ability to display default interpreter settings

0330452

NotebookApi -> NotebookRestApi

88e35cf

Add default property values when creating default interpreter settings

fce6f86

Style note setting panel for simple, report looknfeel

76a86a5

Give some margins between interpreter settings

509eb93

Let SparkInterpreter use interpreter setting property to build SparkC…

6416868

…ontext

Leemoonsoo added 2 commits January 12, 2015 20:33

Change Interpreter.interpret(String) to Interpreter.interpret(String,…

5e94f4a

… InterpreterContext)

Pass InterpreterContext to Interpreter.cancel() and Interpreter.getPr…

0b2c3bb

…ogress()

Fix unittest

6315964

anthonycorbacho reviewed Jan 14, 2015
View reviewed changes

Change REST API

c28a0ee

bzz added the enhancement label Jan 22, 2015

Add rest api test

6c8172c

Leemoonsoo added a commit that referenced this pull request Jan 27, 2015

Merge pull request #235 from NFLabs/interpreter_binding

18ffaae

Bind/Unbind interpreters to note

Leemoonsoo merged commit 18ffaae into master Jan 27, 2015

This was referenced Jan 27, 2015

Add conf/interpreter.json to gitignore #303

Merged

Fix wrong api method call #309

Merged

Improve spark interpreter parameter handling #310

Merged

Bind/Unbind interpreters to note #235

Bind/Unbind interpreters to note #235

Uh oh!

Conversation

Leemoonsoo commented Dec 7, 2014

Uh oh!

Leemoonsoo commented Dec 29, 2014

Uh oh!

swkimme commented Dec 29, 2014

Uh oh!

swkimme commented Jan 12, 2015

Uh oh!

Leemoonsoo commented Jan 12, 2015

Uh oh!

Leemoonsoo commented Jan 13, 2015

New features

Improvements

Bug fix

API Changes

Uh oh!

Leemoonsoo commented Jan 14, 2015

Uh oh!

Leemoonsoo commented Jan 14, 2015

Uh oh!

corneadoug commented Jan 14, 2015

Uh oh!

anthonycorbacho commented Jan 14, 2015

Uh oh!

anthonycorbacho Jan 14, 2015

Choose a reason for hiding this comment

Uh oh!

anthonycorbacho commented Jan 14, 2015

Uh oh!

bzz commented Jan 14, 2015

Uh oh!

Leemoonsoo commented Jan 23, 2015

Uh oh!

anthonycorbacho commented Jan 23, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants