You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@zeppelin.apache.org by meniluca <gi...@git.apache.org> on 2017/03/27 22:14:02 UTC

[GitHub] zeppelin pull request #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

GitHub user meniluca opened a pull request:

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

    [ZEPPELIN-2319] new methods for ZeppelinContext

    ### What is this PR for?
    Adds new methods to ZeppelinContext to run paragraphs in a more user-friendly mode.
    
    ```java
    z.runAllBefore()
    z.runAllAfter()
    z.run(int... ids)
    z.run(String... ids)
    z.runNext(int i)
    ```
    more details in the Jira tickets.
    
    ### What type of PR is it?
    Feature
    
    ### What is the Jira issue?
    [ZEPPELIN-2319](https://issues.apache.org/jira/browse/ZEPPELIN-2319)
    
    ### How should this be tested?
    They all wrap the ``run(List<Object> ids)`` method, hence can be tested in the same test suite.
    
    ### Screenshots (if appropriate)
    Coming soon
    
    ### 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/meniluca/zeppelin master

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

    https://github.com/apache/zeppelin/pull/2195.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 #2195
    
----
commit 0eca6ca19f55efa814e9ffa915ab785e2f54e963
Author: Luca Menichetti <lm...@cern.ch>
Date:   2017-03-27T21:42:38Z

    [ZEPPELIN-2319] new methods for ZeppelinContext

----


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by zjffdu <gi...@git.apache.org>.

Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    Sorry for late response. @meniluca I am very interested in your scenario. It make sense to me for adding these api, another concern is whether it is more proper to expose it via ui or expose this api directly ? Any advantage for exposing it via api directly for your scenario ? And it would be better to add test case for it as well.


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by meniluca <gi...@git.apache.org>.

Github user meniluca commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    Guys, do you need something else from my side to merge?


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by meniluca <gi...@git.apache.org>.

Github user meniluca commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    @zjffdu I agree in principle, but if you look at the code they are few rather trivial new methods, all of them wrapping up the already existing ``run()``. I have these function running in my notebooks since 1.5 versions, I don't think you have to worry about backwards compatibility and maintenance too much (they are really simple!). I'll add screenshots later, but it's easy to imagine what they do.
    
    Say that I am in the middle of the notebook and I want to run all paragraphs before a certain point, I will run ``z.runAllBefore`` and that's it. In the same way if I need to run the rest of the notebook I will use ``z.runAllAfter``. Those functionalities are really useful if you are building a notebook with complex dynamic forms usage and they will avoid the user to click repeatedly the run button.
    
    In Jupyter buttons and shortcuts that are doing this are already present, hence I think that sooner or later you will need those functions.
    
    To describe quickly the other 3 functions... Say that I have one block where I setup variables and I want other paragraphs to be updated, instead of issuing:
    ```scala
    val maxAge = z.input("maxAge", 30) // select age and run paragraphs
    z.run("parID1")
    z.run("parID2")
    z.run("parD3")
    ```
    I will have
    ```scala
    z.run("parID1","parID2","parID3")
    ```
    or, even more useful, to avoid repetitions
    ```scala
    val arrayIDs = Array("parID1","parID2","parID3")
    z.run(arrayIDs:_*)
    // in another paragraph
    z.run(arrayIDs:_*)
    ```
    
    Similarly the function ``z.run(int... idx)`` is doing the same thing, meanwhile ``z.runNext(int N)``, runs the following N paragraphs. Imagine I have the same groups of three blocks in sequence and every time I need to update a variable:
    
    ```scala
    var maxAge = z.input("maxAge", 30)
    z.runNext(3)
    // following 3 paragraphs are executed
    // ... later, in another paragraph below, $maxAge is updated
    maxAge = df.where("balance > 2000").max("age").collect()(0)
    z.runNext(3) 
    // and so on...
    ```
    
    Anyway, most of them are meant to be more user-friendly wrappers of the ``run()`` functions, syntactic sugar. The most important are the runAllBefore/After in my opinion. Let me know what you think and how I can be of any help.
    
    Cheers,
    Luca


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by zjffdu <gi...@git.apache.org>.

Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    @meniluca Does it only work with one type of interpreter ? Say does it work with multiple different interpreters, such as spark + sh ? And please also check #2151 


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by zjffdu <gi...@git.apache.org>.

Github user zjffdu commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    @meniluca I am a little conservative for adding new public api in ZeppelinContext. As new api means more overhead to maintain it and keep backward compatibility. Could you describe the scenario of why you need these api ? Maybe we could build more generate api or solution for 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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by Leemoonsoo <gi...@git.apache.org>.

Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    There can be simpler way to make a test for wrapper function using mockito.
    
    Other way is adding like [zRunTest](https://github.com/apache/zeppelin/blob/v0.7.1/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java#L319)


---
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 #2195: [ZEPPELIN-2319] new methods for ZeppelinContext

Posted by meniluca <gi...@git.apache.org>.

Github user meniluca commented on the issue:

    https://github.com/apache/zeppelin/pull/2195
  
    @jongyoul, everybody, would you suggest where to put the test? Consider that all these methods are wrapping the ``ZeppelinContext.run("parID")`` method that is NOT tested in this module (simply because interpreters don't know anything about paragraph IDs). I need to have a list of paragraphs and their IDs, but I cannot ``import org.apache.zeppelin.notebook.*`` in a test of this module.
    
    Can I create a test elsewhere? Maybe in ``zengine``? Or can you suggest a solution?
    
    I want to stress again that this PR is simply adding more user-friendly wrappers of already exposed public methods in the ZeppelinContext. They are necessary to give user more flexibility in building more dynamic/interactive notebooks, not only to add new buttons and whatnot.
    
    Cheers,
    Luca


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