You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by AhyoungRyu <gi...@git.apache.org> on 2017/03/18 15:54:44 UTC

[GitHub] zeppelin pull request #2154: [HOT FIX][MASTER] Fix multi dynamic select form...

GitHub user AhyoungRyu opened a pull request:

    https://github.com/apache/zeppelin/pull/2154

    [HOT FIX][MASTER] Fix multi dynamic select forms behaviour

    ### What is this PR for?
    After #2100 merged, we can control the behaviour of running select form using `Run on selection change` under each paragraph control menu. But currently if user creates multiple dynamic forms in one paragraph, the select form box itself[1] and `Run on selection` menu[2] don't appear  as reported in https://github.com/apache/zeppelin/pull/2141#issuecomment-287537706. 
    
     - [1]
    ![image](https://cloud.githubusercontent.com/assets/10060731/24073544/4b477ec2-0c3c-11e7-95ae-d651c0180903.png)
    
     - [2]
    ![image](https://cloud.githubusercontent.com/assets/10060731/24073550/5b91998e-0c3c-11e7-9418-797a5d26aa67.png)
     
    Regardless the number of select forms and the types of dynamic form, `Run on selection change` menu should be shown up if the paragraph has at least 1 select form. 
    
    ### What type of PR is it?
    Bug Fix & Hot Fix
    
    ### What is the Jira issue?
    N/A
    
    ### How should this be tested?
    1. Create multiple select forms 
    ```
    %md 
    My first selection is ${my selection1=1,1|2|3}
    My second selection is ${my selection2=4,4|5|6}
    ```
    
    2. Create different types of dynamic form (e.g. 1 select form + 1 checkbox)
    ```
    %md
    
    My selection is ${my selection=1,1|2|3}
    My check list is ${checkbox:checkboxTest=list1|list2, list1|list2|list3|list4}
    
    ```
    
    There should be `Run on selection change` menu under the paragraph control menu in the above cases. And the select form should appear!
    
    ### Screenshots (if appropriate)
     - When multiple select forms are created
    ![double-selectforms](https://cloud.githubusercontent.com/assets/10060731/24073573/af12ae2c-0c3c-11e7-80fa-18abe98f2dfd.gif)
    
     - When different dynamic forms are created (e.g. 1 checkbox + 1 select form)
    ![checkbox_selectform](https://cloud.githubusercontent.com/assets/10060731/24073578/bce7af52-0c3c-11e7-85df-c858342f7e2e.gif)
    
    ### Questions:
    * Does the licenses files need update? no
    * Is there breaking changes for older versions? no
    * Does this needs documentation? no


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/AhyoungRyu/zeppelin fix/multiDynamicFormBehaviour

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/2154.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2154
    
----
commit a9a8d3d672405448ea41bd69e9026034caf37b82
Author: RyuAhyoung <ah...@macbook-pro-5.local>
Date:   2017-03-18T15:07:27Z

    Fix multi dynamicforms behaviour

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro Not sure how your local git repo status is now, but if you think you can't make it back to normal status, I would recommend you to remove local Zeppelin dir and reclone it. When the PR that you want to review is based on another branch (in this case `branch-0.7`) not master, you can do it like this. 
    
    1. check your remote  repo list and its name first
    ```bash
    $ git remote -v 
    origin	https://github.com/guicaro/zeppelin (fetch)
    origin	https://github.com/guicaro/zeppelin(push)
    upstream	https://github.com/apache/zeppelin.git (push)
    upstream	https://github.com/apache/zeppelin.git (fetch)
    
    # in this case, 
    ```
    
    2. sync your local repo with `upstream`
     - if you already set `upstream` with https://github.com/apache/zeppelin.git, then sync your local git repo with `upstream`.
    ```bash
    $ git fetch upstream
    ```
     - But if you didn't set `upstream` yet, you can add `upstream` to your remote repo list like below
    ```bash
    $ git remote add upstream https://github.com/apache/zeppelin.git
    
    # to make sure, print your remote repo list again
    $ git remote -v 
    
    # fetch upstream's latest commit history
    $ git fetch upstream
    ```
    
    3. then create new branch based on remote's `branch-0.7`
    ```bash
    $ git checkout -t branch-0.7
    ```
    
    4. Then run 
    ```
    $ ./dev/test_zeppelin_pr.py 2156
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    2nd and 3rd build jobs keep failed even after restarting them several times. And the reasons are as below 
    ```
    1. Restart Zeppelin
    2. Create interpreter setting in 'Interpreter' menu on Zeppelin GUI
    3. Then you can bind the interpreter on your note
    Install intp2(org.apache.commons:commons-math3:3.6.1) to /tmp/ZeppelinLTest_1489855375119/interpreter/intp2 ... 
    Interpreter intp2 installed under /tmp/ZeppelinLTest_1489855375119/interpreter/intp2.
    
    1. Restart Zeppelin
    2. Create interpreter setting in 'Interpreter' menu on Zeppelin GUI
    3. Then you can bind the interpreter on your note
    Install intp1(org.apache.commons:commons-csv:1.1) to /tmp/ZeppelinLTest_1489855376594/interpreter/intp1 ... 
    Interpreter intp1 installed under /tmp/ZeppelinLTest_1489855376594/interpreter/intp1.
    
    1. Restart Zeppelin
    2. Create interpreter setting in 'Interpreter' menu on Zeppelin GUI
    3. Then you can bind the interpreter on your note
    Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.869 sec - in org.apache.zeppelin.interpreter.install.InstallInterpreterTest
    Running org.apache.zeppelin.interpreter.InterpreterSettingTest
    SLF4J: Class path contains multiple SLF4J bindings.
    SLF4J: Found binding in [jar:file:/home/travis/build/AhyoungRyu/zeppelin/zeppelin-interpreter/target/lib/slf4j-log4j12-1.7.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
    SLF4J: Found binding in [jar:file:/home/travis/.m2/repository/org/slf4j/slf4j-log4j12/1.7.10/slf4j-log4j12-1.7.10.jar!/org/slf4j/impl/StaticLoggerBinder.class]
    SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
    SLF4J: Actual binding is of type [org.slf4j.impl.Log4jLoggerFactory]
    Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.506 sec - in org.apache.zeppelin.interpreter.InterpreterSettingTest
    
    Results :
    
    Failed tests: 
      NotebookTest.testAngularObjectRemovalOnInterpreterRestart:699 expected null, but was:<AngularObject{noteId='2CB425J2J', paragraphId='null', object=object1, name='o1'}>
    
    Tests run: 172, Failures: 1, Errors: 0, Skipped: 0
    
    [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on project zeppelin-zengine: There are test failures.
    [ERROR] 
    [ERROR] Please refer to /home/travis/build/AhyoungRyu/zeppelin/zeppelin-zengine/target/surefire-reports for the individual test results.
    [ERROR] -> [Help 1]
    [ERROR] 
    [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
    [ERROR] Re-run Maven using the -X switch to enable full debug logging.
    [ERROR] 
    [ERROR] For more information about the errors and possible solutions, please read the following articles:
    [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
    [ERROR] 
    [ERROR] After correcting the problems, you can resume the build with the command
    [ERROR]   mvn <goals> -rf :zeppelin-zengine
    ```
    
    Is this related with my change? If so, please point me. It'll be really appreciated :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    The error is not related to this change. I'm working on the CI error.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    FYI, quick show for `edit on double click` with markdown interpreter
    ![edit on double click](https://cloud.githubusercontent.com/assets/10060731/24077344/853c25be-0c8d-11e7-9df1-822051cbbff7.gif)
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by guicaro <gi...@git.apache.org>.
Github user guicaro commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @AhyoungRyu Thanks so much for your help. Even after starting my local repo from scratch and fetching from upstream I can't seem to get to #2154 using `test_zeppelin_pr.py`. Here are some relevant lines from output, and then I end up in the pr2154 branch with several files staged
    
    ```
    sh: Run: command not found
    sh: Run: command not found
    sh: Run: command not found
    : command not found
    sh: line 1: fg: no job control
    : bad substitutionelection1=1,1|2|3}
    : command not found
    sh: line 1: fg: no job control
    : command not found
    : bad substitutionelection=1,1|2|3}
    sh: Run: command not found
    fatal: Paths with -a does not make sense.
    Commit failed
    ```
    
    Inspecting the python file seems like all commands issued are `git` 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by guicaro <gi...@git.apache.org>.
Github user guicaro commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @AhyoungRyu I've been trying to get #2156 working without any luck, completely my fault in not following the correct flow. I get into all kinds of merge conflicts even after I switch to the `branch-0.7` and then run the `test_zeppelin_pr.py` script you guys have. Some relevant messages:
    
    <img width="820" alt="screen shot 2017-03-19 at 9 54 36 am" src="https://cloud.githubusercontent.com/assets/1750596/24078848/2878a04e-0c8a-11e7-8831-521257a7428f.png">
    
    What is the workflow you follow to review when not from master?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    After rebasing from master, I restarted 2nd build job several times but it keeps failed because of timeout. 
    https://api.travis-ci.org/jobs/212613496/log.txt?deansi=true
    ```
    The job exceeded the maximum time limit for jobs, and has been terminated.
    ```
    
    So I think it would be better to merge this into master as a hotfix.
    
    @guicaro Feel free to create new issue or ping me if you find another issue regarding this feature. And also, if you(or maybe other users) feel "Edit on double click" feature is a bit awkward in checkbox form, surely we can try to improve it :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro Just noticed that the behaviour that you mentioned was intended one. 
    In Zeppelin, we support `editOnDoubleClick` \w Markdown interpreter. With this feature, you can hide/show the code block. Since you clicked the paragraph several times while you're clicking the checkbox, the code section was hidden because of this feature of `%md` interpreter. 
    So it's not neither dynamic form behaviour itself's issue nor this patch's. 
    
    You don't need to test this in #2156(don't wanna make you spend your time in the weekend), you can simply check this with `%spark` interpreter in this branch :)
    ```
    %spark
    println("Hello "+z.select("day", Seq(("1","mon"),
                                        ("2","tue"),
                                        ("3","wed"),
                                        ("4","thurs"),
                                        ("5","fri"),
                                        ("6","sat"),
                                        ("7","sun"))))
    
    val options = Seq(("apple","Apple"), ("banana","Banana"), ("orange","Orange"))
    println("Hello "+z.checkbox("fruit", options).mkString(" and "))
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro Really appreciated your time & effort in the weekend. Sadly i have no idea what could be the reason for your problem with limited information. But i think we need only to focus on this hotfix patch in this PR. If you're still in trouble with your git env and you think you can't resolve it, then please [email me](mailto:ahyoungryu@apache.org). I'll happy to help you :)
    
    As I said in [this comment](https://github.com/apache/zeppelin/pull/2154#issuecomment-287598706), would be better to merge this PR as a hotfix.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by guicaro <gi...@git.apache.org>.
Github user guicaro commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @AhyoungRyu just glanced through comments in #2156 I also wondered why changes in check boxes did not trigger a run when placed next to a select form and with the `Run on selection change` glad to see we are all on the same page :)
    
    I'm going to have to get back to you on verifying #2156 my local git is displaying all kinds of colors and merge conflicts :( after I tried using `dev/test_zeppelin_pr.py`, I'll take a look at it 5 or 6 hours it that's ok.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin pull request #2154: [HOT FIX][MASTER] Fix multi dynamic select form...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/2154


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro Thank you for the quick checking!
    I made some change in #2156(it's base branch is `branch-0.7` while this PR was created on `master`) for the checkbox. The every checking the checkbox will trigger running if `Run on selection change` is on. To keep the consistency, this PR will be updated like it does. If possible, could you check you can reproduce the issue or not in #2156?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro BTW you don't need to test #2156(base branch: `branch-0.7`) anymore since I updated this PR(base branch: `master`)  as like #2156.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zeppelin issue #2154: [HOT FIX][MASTER] Fix multi dynamic select forms behav...

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/2154
  
    @guicaro Seems sth screwed up in your git. It's okay I can try to reproduce it now :) 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---