[ZEPPELIN-697] Replace dynamic form with angular object from registry if exists#745
[ZEPPELIN-697] Replace dynamic form with angular object from registry if exists#745doanduyhai wants to merge 1 commit intoapache:masterfrom
Conversation
doanduyhai
force-pushed
the
ZEPPELIN-697
branch
2 times, most recently
from
938c70f to
fb5a531
Compare
|
ping @Leemoonsoo for code review |
|
ping @Leemoonsoo |
|
The implementation itself looks good to me. While this PR links 'dynamic form' and 'angular display system', I'd like to discuss little bit about the direction. Zeppelin has 'dynamic form' and 'angular display system' for frontend - backend interaction.
'Dynamic Form' and 'Angular Display System' are both does frontend-backend interaction. While they're sharing similar characteristics, do you see any possibilities of consolidate these 3 into 1? |
|
Hello @Leemoonsoo Thank for raising question about the 3 types of variables. My though is that Angular Variables should be merged/replaced by ResourcePool. Indeed the Angular Object scope is not very intuitive. It is first scoped by interpreter group then by note/paragraph. Removing Angular Object to use Resource Pool is a good idea but there is still one pending issue: angular value watcher. As you know people can assign a watcher function in the back-end on Angular value. If we want to replace Angular objects by Resource Pool, we need to provide similar feature, unless you think we can completely drop this watcher feature ... Another point to take into account is the migration path. For all people that have saved their notes as JSON with saved Angular objects, we need to handle the migration to Resource pool when the re-import those notes. About the Dynamic Form system I don't think we should merge it with Angular Object/Resource Pool. Dynamic Form values are just an alias to automatically generate a form input. And the fact that its value change triggers the execution of the current paragraph can be implemented using All in all I think we should ultimately have enhanced Resource Pool and Dynamic Form only. But those changes are very big and deserve another JIRA epic. For now, we can just implement this overriding system for this PR and then when we refactor the Angular JS to Resource Pool, we will merge the behavior together WDYT ? |
|
I agree, consolidate Angular Object / Resource Pool into Resource Pool really make sense. I think watcher also can be handled. I think the way how this links PR Resource Pool and Dynamic Form make sense and aligned with the future plan. Thanks for the patch and sharing the idea. LGTM |
|
@Leemoonsoo Merge if ok ? Next 2 PR will concentrate on keeping same paragraph Id when re-importing node from JSON and add comprehensive documentation for the new AngularJS |
|
Merge into master if there're no more discussions. |
… if exists
### What is this PR for?
Replace dynamic form with angular object from registry if exists
I updated the `Paragraph.jobRun()` method to look for existing variable from the Angular Object Registry first before displaying the dynamic form.
We look for Angular object having same name:
* first at paragraph scope (note id + paragraph id)
* then at note scope (note id only)
I did not look at **global** scope because Leemoonsoo was mentioning somewhere that we may likely remove the global scope some day
_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-697]**
### How should this be tested?
* `git fetch origin pull/745/head:AngularObjectReplaceDynamicFormVar`
* `git checkout AngularObjectReplaceDynamicFormVar`
* `mvn clean package -DskipTests`
* `bin/zeppelin-daemon.sh restart`
* Create a new note
* In the first paragraph, put the following code
```html
%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'); z.runParagraph('PUT_HERE_PARAGRAPH_ID')"> Bind</button>
<button type="submit" class="btn btn-primary" ng-click="z.angularUnbind('superhero','PUT_HERE_PARAGRAPH_ID'); z.runParagraph('PUT_HERE_PARAGRAPH_ID')"> Unbind</button>
</form>
</form>
```
* Create a second paragraph with the following code:
```scala
%md
### The superhero is : **${superhero}**
```
* In the first paragraph, replace the text PUT_HERE_PARAGRAPH_ID by the paragraph id of the second paragraph
* Execute first the second paragraph to see that the dynamic form system is working as usual
* Now put **Batman** in the input text of the first paragraph and click alternatively on **Bind** to see that dynamic form in the second paragraph is removed
* Click on **Unbind** to see that the dynamic form system is back
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? --> **No**
* Is there breaking changes for older versions? --> **Yes**
* Does this needs documentation? --> **Yes**
[ZEPPELIN-635]: https://issues.apache.org/jira/browse/ZEPPELIN-635
[ZEPPELIN-697]: https://issues.apache.org/jira/browse/ZEPPELIN-697
Author: DuyHai DOAN <[email protected]>
Closes apache#745 from doanduyhai/ZEPPELIN-697 and squashes the following commits:
be4dbd1 [DuyHai DOAN] [ZEPPELIN-697] Replace dynamic form with angular object from registry if exists
2 participants
What is this PR for?
Replace dynamic form with angular object from registry if exists
I updated the
Paragraph.jobRun()method to look for existing variable from the Angular Object Registry first before displaying the dynamic form.We look for Angular object having same name:
I did not look at global scope because @Leemoonsoo was mentioning somewhere that we may likely remove the global scope some day
This is a sub-task of epic ZEPPELIN-635
What type of PR is it?
[Improvement]
Todos
Is there a relevant Jira issue?
ZEPPELIN-697
How should this be tested?
git fetch origin pull/745/head:AngularObjectReplaceDynamicFormVargit checkout AngularObjectReplaceDynamicFormVarmvn clean package -DskipTestsbin/zeppelin-daemon.sh restartScreenshots (if appropriate)
Questions: