[ZEPPELIN-689] Add AngularJS z object and z.angularBind()

[doanduyhai]

What is this PR for?

Add client-side z object with method angularBind()

@Leemoonsoo

Compared to the original implementation, I have simplified a lot.

Now, you can only bind angular variable to one unique scope, which is the paragraph. I just remove the note scope and also remove the interpreter parameter.

Indeed, when passing a paragraphId, on the server-side, we can retrieve the Paragraph object with the noteId + paragraphId so we can now which interpreter is currently being used.

The signature of the angularBind(varName, value, paragraphId) method has also been greatly simplified.

This is a sub-task of epic ZEPPELIN-635

What type of PR is it?

[Improvement]

Todos

• - Code review
• - Simple test

Is there a relevant Jira issue?

ZEPPELIN-689

How should this be tested?

• git fetch origin pull/740/head:AngularJSBind
• git checkout AngularJSBind
• mvn clean package -DskipTests
• bin/zeppelin-daemon.sh restart
• Create a new note
• In the first paragraph, put the following code

%angular

<form class="form-inline">
  <div class="form-group">
    <label for="superheroId">Super Hero: </label>
    <input type="text" class="form-control" id="superheroId" placeholder="Superhero name ..." ng-model="superhero"></input>
  </div>
  <button type="submit" class="btn btn-primary" 
    ng-click="z.angularBind('superhero', superhero, 'PUT_HERE_PARAGRAPH_ID')"> Bind Angular</button>
</form>

• Create a second paragraph with the following code:

z.angular("superhero")

• Retrieve the paragraphId of the second paragraph
• In the first paragraph, replace the text PUT_HERE_PARAGRAPH_ID by the correct paragraph id
• In the input text, put "Superman" and click on the Bind Angular button
• Execute the second paragraph to see that the superhero variable is now set to Superman

Screenshots (if appropriate)

Questions:

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

[doanduyhai]

Anyone can have a look into this PR please ? I continue rebasing it from the master to avoid conflicts when merging and I don't want to do it 10x for nothing

[Leemoonsoo]

Hi @doanduyhai. Sorry for the late response. I've got some feedbacks here.

two different feature in a function.

I think it's more clear to provide z.runParagraph() than z.angularBind() has flag of runParagraph.
Having two different feature in a single function should be avoided.

passing paragraph id.

It's passing paragraph id or ids using paragraph / paragraphs field in Parameters.
I think it can be improved by considering two perspective. The First one is easy of use. User need to repeat paragraph id to series of z.angularBind() again and again. e.g.

z.angularBind('name1', value1, { paragraph : '20160126-153136_1060166247'})
z.angularBind('name2', value2, { paragraph : '20160126-153136_1060166247'})
z.angularBind('name3', value3, { paragraph : '20160126-153136_1060166247'})
...

Second one is notebook portability, the code now depends on paragraph id and it's not portable anymore. If you export notebook and import notebook, the code will not work anymore unless user update all paragraph id in the code.

Considering easy of use and notebook portability, i think following style of API would help.

// we can get paragraph id from any html element in the paragraph. 
// This way, paragraph id is no longer hardcoded so notebook becomes more portable.
var z = getZeppelinContext(htmlElement)     

// user can avoid passing paragraph id repeatedly.
z.angularBind('name1', value1)
z.angularBind('name2', value2)
z.angularBind('name3', value3)

front-end z.angularBind() call creates/updates angularObject into all interpreters binded.

z.angularBind() updates angularObject of all interpreters binded.
I think it should be update one interpreter who serves that particular paragraph.

Advantage of having API in both front-end, back-end side

It's more general question. Basically, this PR brings the API that currently only back-end side provides.
There can be Pros and Cons.
Cons side, if user creates something that mixes both front-end side API and back-end side API at the same time, while front-end side(js in webbrowser) and back-end side (heterogeneous interpreters) runs asynchronously, i can easily imagine understanding behavior of those code will be really difficult.
Not having front-end API will force user manage all angularObject in one side (back-end side), that will help simplicity of use.

Could you explain advantage of having API in both front-end and back-end side ?

[doanduyhai]

Hello @Leemoonsoo

I think it's more clear to provide z.runParagraph() than z.angularBind() has flag of runParagraph.
Having two different feature in a single function should be avoided.

The reason why I add the runParagraph option for z.angularBind() is that it will be used in 80% of the time. Indeed, what's the point to bind an Angular variable if you don't refresh immediately the target paragraph for quick update ? I can't see any use-case where the user bind an Angular variable and do not click on the "Run" button on the target paragraph(s) or on the whole notebook itself.

If we remove the runParagraph option, to achieve the same feature, we'll need:

<input type="text" ng-click="z.angularBind(....); z.runParagraph(...)" >

or worst, the user should manually click on the target paragraph like in the screenshot, it's far from being user-friendly.

I think it can be improved by considering two perspective. The First one is easy of use. User need to repeat paragraph id to series of z.angularBind() again and again. e.g.

I do agree with your on this point to some extent. Yes, it forces the user to do a manual copy and paste but once it's done, you don't need to update it again (we'll see this point below).

But this issue also applies to ZeppelinContext.runParagraph(String paragraphId) if we had chosen to use the ZeppelinContext to run paragraphs in the back-end instead of front-end so I would say this issue is not really a new issue, it has always been there.

Second one is notebook portability, the code now depends on paragraph id and it's not portable anymore. If you export notebook and import notebook, the code will not work anymore unless user update all paragraph id in the code.

You are absolutely right about this point. Indeed it's vey annoying that exporting and importing the same note make you loose the paragraph id. For this I see 2 solutions:

1. first create the Angular z object from an HtmlElement as you suggest var z = getZeppelinContext(htmlElement), the only problem is what is htmlElement ? How can we refer to a paragraph by its htmlElement ? I find it simpler to refer to a paragraph using its id

2. I think we should fix the dynamic paragraph id and persist the paragraph id at the same time as the paragraph content. As far as I can see, the paragraph id is generated using some timestamp plus a random part so there is very few chance of collision. Consequently it is possible to export a note with all the original paragraph ids and import them elsewhere

   WDYT ?

front-end z.angularBind() call creates/updates angularObject into all interpreters binded.

Allow me to disagree with this idea, if there are 10 paragraphs in the note with 10 different interpreters, it will create a lot of pollution. There is no reason that we push the Angular variable to many interpreters that are not used by the target paragraph(s).

Another argument against binding the variable to all interpreters used by the note is because Angular variables are persisted in note.json file so we'll create a lot of data that will be exported/re-imported and not used ....

Could you explain advantage of having API in both front-end and back-end side ?

Yes, this is part of the goal Usability Improvement in the roadmap.

As an user, I want to be able to design a custom (with my own CSS etc ...) HTML form with my own controls (input text, drop down, calendar component, carrousel etc ...) and be able to push Angular variable to back-end.

Without this PR, to do this, I'll need to write Scala code like

import org.apache.zeppelin.display.angular.paragraphscope._
import AngularElem._

<div>
  <div class="alert alert-info">This is an alert panel</div>
  <form class="form-inline">
    <div class="form-group">
      <input type="text" class="form-control"  placeholder="Input Text"></input>
    </div>
    <button type="submit" class="btn btn-primary">Click Me</button>
  </form>
</div>.onEvent("ng-click", () => {
  //my callback routine
}).display

If I were a web developer using Zeppelin, I don't understand why I am forced to use Scala and the Spark interpreter to produce AngularJS and HTML source code...

Isn't it more natural to write plain Javascript and AngularJS code if you want to render HTML and bind AngularJS ?

I have given many talks about Zeppelin since 6 months and everytime I build a beautiful pure HTML form using Bootstrap, people in the audience ask me why I have another paragraph with source code like this:

z.angularBind(...)
z.runParagraph(...)

It seems so un-natural for users to have Scala code in a separate paragraph just to make the HTML form work. If we want to extend the user-base of Zeppelin, we need to provide user natural ways of creating Angular variable and not force them to use the Spark interpreter for that.

There is also a demande on the user-mailing list recently to be able to update Angular variable from the front-end:

So the demand for controlling Angular variable from the front-end is real, not just some fancy idea I have in mind.

Cons side, if user creates something that mixes both front-end side API and back-end side API at the same time, while front-end side(js in webbrowser) and back-end side (heterogeneous interpreters) runs asynchronously, i can easily imagine understanding behavior of those code will be really difficult.

Totally agree that if one user try to bind AngularJS using the back-end API with ZeppelinContext or the new Angular API you introduce and the front-end API I propose, it will make things complicated. But again, it's the user responsibility to make clean code. You can never stop people from doing stupid things right ?

To conclude, what I propose is to make another PR, before we progress on this one, to persist the paragraph Id with the note content so that we don't have anymore export/import problem.

WDYT ?

[doanduyhai]

@Leemoonsoo

I had a look into the note.json file. Indeed we already store the paragraph id there. Is there any reason that the paragraph id change every time we import the note.json file back into Zeppelin ?

[Leemoonsoo]

two different feature in a function.

The reason why I add the runParagraph option for z.angularBind() is that it will be used in 80% of the time. Indeed, what's the point to bind an Angular variable if you don't refresh immediately the target paragraph for quick update ? I can't see any use-case where the user bind an Angular variable and do not click on the "Run" button on the target paragraph(s) or on the whole notebook itself.

If we remove the runParagraph option, to achieve the same feature, we'll need:

<input type="text" ng-click="z.angularBind(....); z.runParagraph(...)" >

or worst, the user should manually click on the target paragraph like in the screenshot, it's far from being user-friendly.

Idea behind AngularDisplaySystem was, give possibility to interpreter (back-end) interact with front-end. By leveraging two way binding and watcher of AngularJS, AngularDisplaySystem could able to work without introducing any Zeppelin API in front-end side.

Current api is designed like

Actions	Front-end API	Back-end API
Initiate binding	N/A	z.angularBind(var, initialValue)
Update value	same to ordinary angularjs scope variable	z.angularBind(var, newValue)
Watching value	same to ordinary angularjs scope variable	z.angularWatch(var, (oldVal, newVal) => )
Destroy binding	N/A	z.angularUnbind(var)

in my understanding, this PR trying to change it, to

Actions	Front-end API	Back-end API
Initiate binding	z.angularbind(var, initialValue)	z.angularBind(var, initialValue)
Update value	same to ordinary angularjs scope variable, or z.angularBind(var, newValue)	z.angularBind(var, newValue)
Watching value	same to ordinary angularjs scope variable	z.angularWatch(var, (oldVal, newVal) => )
Destroy binding	z.angularUnbind(var)	z.angularUnbind(var)

In this perspective, i'm not sure it's good to embed run paragraph inside of z.angularBind() function in front-end side, while corresponding api in backend side does not.

Also not every case uses angular displays system to let user click and run other paragraph.
Angular display system can be used to dynamically update front-end without re-run the paragraph like this example https://gist.github.com/granturing/a09aed4a302a7367be92.

So, to me, it's less ambiguous and more consistent to have two apis. z.angularBind() and z.runParagraph().

passing paragraph id.

But this issue also applies to ZeppelinContext.runParagraph(String paragraphId) if we had chosen to use the ZeppelinContext to run paragraphs in the back-end instead of front-end so I would say this issue is not really a new issue, it has always been there.

Right, if user use just hardcoded paragraph id to call existing ZeppelinContext.runParagraph(String paragraphId) then the problem will be there. However, current API set provides option to write code without hardcoded paragraph id.

z.runParagraph(z.getInterpreterContext.getParagraphId)

In the similar manner,

<div id="myEl"></div>
<script>
var htmlElement = angular.element('#myEl')
var z = getZeppelinContext(htmlElement)
...
</script>

will give an option to avoid use hardcoded paragraph id.
htmlElement is html Element that created by user code in the paragraph.
That's only way i know get current paragraph id from the javascript that defined in particular paragraph.

I think we should fix the dynamic paragraph id and persist the paragraph id at the same time as the paragraph content. As far as I can see, the paragraph id is generated using some timestamp plus a random part so there is very few chance of collision. Consequently it is possible to export a note with all the original paragraph ids and import them elsewhere

paragraph id can not be simply changed, while of some features (rest api to run paragraph, iframe link, ...) and a lot of internal code (angulardisplay system, job scheduler, etc) assumes paragraph id is immutable.

front-end z.angularBind() call creates/updates angularObject into all interpreters binded.

Allow me to disagree with this idea, if there are 10 paragraphs in the note with 10 different interpreters, it will create a lot of pollution. There is no reason that we push the Angular variable to many interpreters that are not used by the target paragraph(s).

Another argument against binding the variable to all interpreters used by the note is because Angular variables are persisted in note.json file so we'll create a lot of data that will be exported/re-imported and not used ....

Right, that's what i wanted to say actually :-)

Advantage of having API in both front-end, back-end side

There is also a demande on the user-mailing list recently to be able to update Angular variable from the front-end:

So the demand for controlling Angular variable from the front-end is real, not just some fancy idea I have in mind.

The question from the mailing list, "pass variable defined in a angular interpreter to another paragraph which is scala interpreter", i can convert it to "how to initiate angularObject binding from the front-end code?" Because passing variable from front end to back end is already possible once they're binded.

[doanduyhai]

In this perspective, i'm not sure it's good to embed run paragraph inside of z.angularBind() function in front-end side, while corresponding api in backend side does not.

Ok, I see your point now. The table showing back-end and front-end API is quite clear. So I'll remove the runParagraph attribute. If user want to trigger paragraph execution they can use z.runParagraph()

htmlElement is html Element that created by user code in the paragraph.
That's only way i know get current paragraph id from the javascript that defined in particular paragraph.

Your idea can work if we have a paragraph defining HTML element. But what if I want to bind Angular value to a SparkSQL paragraph or Cassandra paragraph ?

%sql
SELECT * FROM my_table WHERE id = ${id}

%cassandra
SELECT * FROM table WHERE key=${key}

There is no HTML element so I cannot retrieve the paragraph Id using DOM JS function... The only way is to rely on paragraph id, or maybe paragraph title but we don't have unicity guarantee when using title ...

paragraph id can not be simply changed, while of some features (rest api to run paragraph, iframe link, ...) and a lot of internal code (angulardisplay system, job scheduler, etc) assumes paragraph id is immutable.

No no, I think there is a misunderstanding here. I don't want to change the paragraph id. What I want is that on Import, we keep the original paragraph id and don't generate a new one every time so that z.angularBind('val', val, {paragraphId: 'xxxx'}) is re-usable and does not break when sharing or importing/exporting notes

To enable this behavior, there is very few change to the code base, see the diff here

The result seems working well with import/export and clone

Right, that's what i wanted to say actually :-)

So there is no problem, we both agree that z.angularBind() should bind value to only one paragraph right ?

i can convert it to "how to initiate angularObject binding from the front-end code?" Because passing variable from front end to back end is already possible once they're binded.

Exactly, so z.angularBind() is here to fill in the gap and allow to initiate a new binding to an Angular variable from front-end.

---

So to conclude, we indeed agree on many things. I propose the following next steps:

1. Rework this PR to remove runParagraph so we decouple z.angularBind() and z.runParagrap()
2. Create another PR to keep the original paragraph id every time we import a note, based on the patch I showed

WDYT @Leemoonsoo ?

[doanduyhai]

@Leemoonsoo what do you think about my last proposal ?

[Leemoonsoo]

Your idea can work if we have a paragraph defining HTML element. But what if I want to bind Angular value to a SparkSQL paragraph or Cassandra paragraph ?

%sql
SELECT * FROM my_table WHERE id = ${id}

%cassandra
SELECT * FROM table WHERE key=${key}

There is no HTML element so I cannot retrieve the paragraph Id using DOM JS function... The only way is to rely on paragraph id, or maybe paragraph title but we don't have unicity guarantee when using title

You're right in that case. Then we can just give user a tip to get paragraphId from HTML element in documentation.

No no, I think there is a misunderstanding here. I don't want to change the paragraph id. What I want is that on Import, we keep the original paragraph id and don't generate a new one every time so that z.angularBind('val', val, {paragraphId: 'xxxx'}) is re-usable and does not break when sharing or importing/exporting notes

To enable this behavior, there is very few change to the code base, see the diff here

The result seems working well with import/export and clone

Understood. Sounds nice. (what happen i import the same note again?)

Allow me to disagree with this idea, if there are 10 paragraphs in the note with 10 different interpreters, it will create a lot of pollution. There is no reason that we push the Angular variable to many interpreters that are not used by the target paragraph(s).

Another argument against binding the variable to all interpreters used by the note is because Angular variables are persisted in note.json file so we'll create a lot of data that will be exported/re-imported and not used ....

Right, that's what i wanted to say actually :-)

So there is no problem, we both agree that z.angularBind() should bind value to only one paragraph right ?

And only one interpreter.

So to conclude, we indeed agree on many things. I propose the following next steps:

Rework this PR to remove runParagraph so we decouple z.angularBind() and z.runParagrap()
Create another PR to keep the original paragraph id every time we import a note, based on the patch I showed

Sounds reasonable. Thank you for all the explanation

[corneadoug]

Just a quick question in case I missed something, why are we limiting the angularBind to paragraph level only?

[doanduyhai]

Yes @corneadoug because:

1. binding angular object at note level will require binding this value to all interpreters enabled in this note. This creates a lot of pollution because we save angular objects into the JSON file
2. When the user disable an interpreter for the current note, we need to unbind and remove all angular objects related to the note
3. I see few use-case where you need to bind an angular object for the whole note

[doanduyhai]

@Leemoonsoo

Understood. Sounds nice. (what happen i import the same note again?)

You can re-importe the same note many times, the note id is re-generated each time and is different for each import. But the paragraphId remains the same.

Remark: in the patch I mentioned above, I change the source code to re-use the paragraphId on note import BUT the jobId (the Paragraph class extends the Job class) is regenerated each time to avoid unicity issue for jobId when we re-import twice the same note ...

So in the last update of this PR, z.angularBind('label', 'value', 'paragraphId') only bind an Angular variable to an unique paragraph and the current interpreter used by this paragraph

[doanduyhai]

@Leemoonsoo

Is the PR now ok for merge ? I've modified it to take into account all the remarks so far

[Leemoonsoo]

init() will be invoked multiple times if there're multiple interpreters in the same interpreter group.
I think it's better make it call only once after interpreter process created. Except for this code looks good to me.

[Leemoonsoo]

@doanduyhai Do you mind add someinfo about this new api to https://github.com/apache/incubator-zeppelin/blob/master/docs/displaysystem/angular.md?

[doanduyhai]

@Leemoonsoo

Don't worry about documentation, this will be added.

Indeed if you look at the epic ZEPPELIN-635, I split all changes into isolated tasks. I will add a task for the change to re-use paragraph Id on note import as well as another task for global documentation of those new features ( z.angularBind(), z.angularUnbind(), z.runParagraph(), new paragraphId display)

Today, ASF JIRA is down so I cannot create those extra JIRAs but as soon as the ASF JIRA is back online, I'll do.

I only consider the JIRA ZEPPELIN-635 finished once we merge all sub-tasks

In the meantime, if no one has objection or other remarks, can we merge this PR so that I can progress on the other PRs ?

[doanduyhai]

@Leemoonsoo

Documentation JIRA: https://issues.apache.org/jira/browse/ZEPPELIN-742
Re-use paragraphId on note import JIRA: https://issues.apache.org/jira/browse/ZEPPELIN-741

[Leemoonsoo]

@doanduyhai Could you take a look a comment for pushAngularObjectRegistryToRemote(client); inside of RemoteInterpreter.init()

[doanduyhai]

@Leemoonsoo

Sorry, was pretty busy this week.

So regarding your remark on calling pushAngularObjectRegistryToRemote() only once in RemoteInterpreter.init(), I just add a boolean to the InterpreterGroup class and use it to keep track whether we push angular object registry or not for an interpreter group.

Please review and validate

[Leemoonsoo]

@doanduyhai Thanks for this new api and improvement.
Looks good to me and merge if there're no more discussions.