You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by doanduyhai <gi...@git.apache.org> on 2016/02/24 21:33:40 UTC

[GitHub] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

GitHub user doanduyhai opened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/746

    [ZEPPELIN-699] Return Paragraph execution result for the REST API

    ### What is this PR for?
    Right now, when calling the REST API `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` Zeppelin always returns **OK** as shown by this source code: https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java#L477
    
    This ticket will update the behavior so that Zeppelin also return the result of the paragraph execution
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Code Review
    * [ ] - Simple Test
    
    ### Is there a relevant Jira issue?
    **[ZEPPELIN-699]**
    
    ### How should this be tested?
    * `git fetch origin pull/746/head:ParagraphExecutionRESTAPI`
    * `git checkout ParagraphExecutionRESTAPI`
    * `mvn clean package -DskipTests`
    * `bin/zeppelin-daemon.sh restart`
    * Create a new note
    * In the first paragraph, put the following code
    
    ```scala
    z.getInterpreterContext().getParagraphId()
    println("Current time = "+new java.util.Date().toString)
    ```
    
    * Retrieve the current note id in the URL
    * Execute the paragraph once to retrieve the paragraph id
    * Use a REST Client like **[POSTman]** to create a HTTP POST query `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` 
    * You should receive something similar as follow for answer
    
    ```
    {
        "status": "OK",
        "message": "res6: String = 20160224-210856_166847643\nCurrent time = Wed Feb 24 21:17:43 CET 2016\n"
    }
    ```
    
    ### Screenshots (if appropriate)
    ![paragraphexecutionrestapi](https://cloud.githubusercontent.com/assets/1532977/13299992/2735a6e2-db3e-11e5-831e-33e927fc4c55.gif)
    
    ### Questions:
    * Does the licenses files need update? --> **No**
    * Is there breaking changes for older versions? --> **No**
    * Does this needs documentation? --> **No**
    
    [ZEPPELIN-699]: https://issues.apache.org/jira/browse/ZEPPELIN-699
    [POSTman]: https://www.getpostman.com/

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

    $ git pull https://github.com/doanduyhai/incubator-zeppelin ZEPPELIN-699

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

    https://github.com/apache/incubator-zeppelin/pull/746.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 #746
    
----
commit 28f20848c4c3f8ba84069d20b937af349f24b37e
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-02-24T20:16:03Z

    [ZEPPELIN-699] Return Paragraph execution result for the REST API

----


---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

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

    https://github.com/apache/incubator-zeppelin/pull/746


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

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

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188989671
  
    I don't have strong opinion here for adding parameter vs new api.
    But i want to mention that adding parameter make the api return two different things.
    
    When sync is false, return success or fail for the job submission. 
    When sync is true,  return success or fail for the job execution, with result data.
    
    That makes the api difficult to understand. And later, there could be some additional parameter introduced for only for async api or only for sync api. in that case, combination of different parameters need to be explained for users.
    
    So i'm little bit biased to new api for this reasons. what do you guys think?


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by doanduyhai <gi...@git.apache.org>.
Github user doanduyhai commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188828325
  
    @Leemoonsoo so you're suggesting to create another API for pagraph execution ? Fine for me, what's about `http://<ip>:<port>/api/notebook/synchronous_job/<note_id>/<paragraph_id>` ?


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-189009246
  
    that we would need a job run status API...


---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    Hello @bzz, I've rebased from the latest master and my PR always fail with this issue:
    
    ```
    12:28:28,504  INFO org.apache.zeppelin.AbstractZeppelinIT:59 - Finished.
    Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 83.01 sec - in org.apache.zeppelin.integration.ParagraphActionsIT
    
    Results :
    
    Tests in error: 
      ZeppelinIT.testSparkInterpreterDependencyLoading:243 » NoSuchElement Unable to...
    ```
    
    You have committed recently a fix for `ZeppelinIT`, do you know what can trigger this issue ?
    
    I have the exact same error on my other PR for Cassandra interpreter update: https://github.com/apache/incubator-zeppelin/pull/950



---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

Posted by doanduyhai <gi...@git.apache.org>.
GitHub user doanduyhai reopened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/746

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

    ### What is this PR for?
    Right now, when calling the REST API `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` Zeppelin always returns **OK** as shown by this source code: https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java#L477
    
    This ticket will update the behavior so that Zeppelin also return the result of the paragraph execution
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Code Review
    * [ ] - Simple Test
    
    ### Is there a relevant Jira issue?
    **[ZEPPELIN-699]**
    
    ### How should this be tested?
    * `git fetch origin pull/746/head:ParagraphExecutionRESTAPI`
    * `git checkout ParagraphExecutionRESTAPI`
    * `mvn clean package -DskipTests`
    * `bin/zeppelin-daemon.sh restart`
    * Create a new note
    * In the first paragraph, put the following code
    
    ```scala
    %sh
    
    echo "Current time = "`date +"%T"
    ```
    
    * Retrieve the current note id in the URL
    * Retrieve the current paragraph id
    * Use a REST Client like **[POSTman]** to create a HTTP POST query `http://<ip>:<port>/api/notebook/run/<note_id>/<paragraph_id>` 
    * You should receive something similar as follow for answer
    
    ```
    {
        "status": "OK",
        "body": {
            "code": "SUCCESS",
            "type": "TEXT",
            "msg": "Current time = 16:14:18\n"
        }
    }
    ```
    
    ### Screenshots (if appropriate)
    ![zeppelin_synchronous_rest_api](https://cloud.githubusercontent.com/assets/1532977/15748069/b4a26a46-28dd-11e6-8f51-aa13ddba3f1c.gif)
    
    API Documentation update
    
    **Existing asynchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773274/5b508cae-2976-11e6-9e52-14d8b7e7828e.png)
    
    **New synchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773309/84965a94-2976-11e6-9719-81d8b555c3c4.png)
    
    
    ### Questions:
    * Does the licenses files need update? --> **No**
    * Is there breaking changes for older versions? --> **No**
    * Does this needs documentation? --> **Yes**
    
    [ZEPPELIN-699]: https://issues.apache.org/jira/browse/ZEPPELIN-699
    [POSTman]: https://www.getpostman.com/

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

    $ git pull https://github.com/doanduyhai/incubator-zeppelin ZEPPELIN-699

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

    https://github.com/apache/incubator-zeppelin/pull/746.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 #746
    
----
commit c8825f7ff639f554862a6933c4a259cd14389f51
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:15:46Z

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

commit 695c13e762bad5a31bf47131ccae196062aa5889
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:24:38Z

    [ZEPPELIN-699] Update Notebook REST API documentation

commit 46a2f3662aab87edf522e92bfa7700170230263b
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-03T08:20:17Z

    [ZEPPELIN-699] Fix doc after code review

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by vgromakowski <gi...@git.apache.org>.
Github user vgromakowski commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188844963
  
    I would suggest to use the same method as for the spark job server, asynchronous is the default, synchronous is obtain with an additional parameter.
    
    De : Lee moon soo [mailto:notifications@github.com]
    Envoyé : jeudi 25 février 2016 15:48
    À : apache/incubator-zeppelin
    Cc : GROMAKOWSKI Vincent IST/ISAD
    Objet : Re: [incubator-zeppelin] [ZEPPELIN-699] Return Paragraph execution result for the REST API (#746)
    
    
    This PR change behavior of REST api job/{notebookId}/{paragraphId} from asynchronous to synchronous.
    
    To me it's better we keep asynchronous behavior for backward compatibility. Some user might depends on asynchronous behavior.
    
    Synchronous behavior can be implemented in a different rest api.
    
    —
    Reply to this email directly or view it on GitHub<https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188816674>.
    
    _________________________________________________________________________________________________________________________
    
    Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
    pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
    a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
    Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
    
    This message and its attachments may contain confidential or privileged information that may be protected by law;
    they should not be distributed, used or copied without authorisation.
    If you have received this email in error, please notify the sender and delete this message and its attachments.
    As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
    Thank you.
    



---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

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

    https://github.com/apache/incubator-zeppelin/pull/746


---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    finished my pass on this.
    +1 on other's comment.



---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

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

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188816674
  
    This PR change behavior of REST api `job/{notebookId}/{paragraphId}` from asynchronous to synchronous.
    
    To me it's better we keep asynchronous behavior for backward compatibility. Some user might depends on asynchronous behavior.
    
    Synchronous behavior can be implemented in a different rest api.


---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    @felixcheung Done, I've added 2 new screen captures of the API documentation update


---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    @felixcheung CI green, you can resume code review


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by doanduyhai <gi...@git.apache.org>.
Github user doanduyhai commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-196040202
  
    @felixcheung I'm still waiting for someone to reply me on my question:
    
    > By the way, for the existing asynchronous API, I don't think returning OK in all cases is correct. Since the execution of the paragraph is asynchronous and we don't wait for it to return the result to the client, how can we know if the execution has been successful or not ? How can we decide if we should return OK or INTERNAL_SERVER_ERROR ?
    
    Or do you think we can just let it as it is and create the new API ?


---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

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

    https://github.com/apache/incubator-zeppelin/pull/746


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-196036497
  
    hello - where are we on this?


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by doanduyhai <gi...@git.apache.org>.
Github user doanduyhai commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188977035
  
    @vgromakowski 
    
    Something like `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>?sync=true` ?


---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

Posted by doanduyhai <gi...@git.apache.org>.
GitHub user doanduyhai reopened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/746

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

    ### What is this PR for?
    Right now, when calling the REST API `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` Zeppelin always returns **OK** as shown by this source code: https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java#L477
    
    This ticket will update the behavior so that Zeppelin also return the result of the paragraph execution
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Code Review
    * [ ] - Simple Test
    
    ### Is there a relevant Jira issue?
    **[ZEPPELIN-699]**
    
    ### How should this be tested?
    * `git fetch origin pull/746/head:ParagraphExecutionRESTAPI`
    * `git checkout ParagraphExecutionRESTAPI`
    * `mvn clean package -DskipTests`
    * `bin/zeppelin-daemon.sh restart`
    * Create a new note
    * In the first paragraph, put the following code
    
    ```scala
    %sh
    
    echo "Current time = "`date +"%T"
    ```
    
    * Retrieve the current note id in the URL
    * Retrieve the current paragraph id
    * Use a REST Client like **[POSTman]** to create a HTTP POST query `http://<ip>:<port>/api/notebook/run/<note_id>/<paragraph_id>` 
    * You should receive something similar as follow for answer
    
    ```
    {
        "status": "OK",
        "body": {
            "code": "SUCCESS",
            "type": "TEXT",
            "msg": "Current time = 16:14:18\n"
        }
    }
    ```
    
    ### Screenshots (if appropriate)
    ![zeppelin_synchronous_rest_api](https://cloud.githubusercontent.com/assets/1532977/15748069/b4a26a46-28dd-11e6-8f51-aa13ddba3f1c.gif)
    
    API Documentation update
    
    **Existing asynchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773274/5b508cae-2976-11e6-9e52-14d8b7e7828e.png)
    
    **New synchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773309/84965a94-2976-11e6-9719-81d8b555c3c4.png)
    
    
    ### Questions:
    * Does the licenses files need update? --> **No**
    * Is there breaking changes for older versions? --> **No**
    * Does this needs documentation? --> **Yes**
    
    [ZEPPELIN-699]: https://issues.apache.org/jira/browse/ZEPPELIN-699
    [POSTman]: https://www.getpostman.com/

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

    $ git pull https://github.com/doanduyhai/incubator-zeppelin ZEPPELIN-699

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

    https://github.com/apache/incubator-zeppelin/pull/746.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 #746
    
----
commit c8825f7ff639f554862a6933c4a259cd14389f51
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:15:46Z

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

commit 695c13e762bad5a31bf47131ccae196062aa5889
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:24:38Z

    [ZEPPELIN-699] Update Notebook REST API documentation

commit 46a2f3662aab87edf522e92bfa7700170230263b
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-03T08:20:17Z

    [ZEPPELIN-699] Fix doc after code review

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by doanduyhai <gi...@git.apache.org>.
Github user doanduyhai commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188791929
  
    Fixed


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by doanduyhai <gi...@git.apache.org>.
Github user doanduyhai commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188999787
  
    Humm, your remark about future extension of the existing API with new parameter makes sense. We need to design API carefully to make evolution easier.
    
    This plays in favor for a separate API
    
    By the way, for the existing **asynchronous** API, I don't think returning OK in all cases is correct. But then, since the execution of the paragraph is asynchronous and we don't wait for it to return the result to the client, how can we know if the execution has been successful or not ? How can we decide if we should return **OK** or **INTERNAL_SERVER_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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-219175679
  
    hello - how are we on this?



---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

Posted by doanduyhai <gi...@git.apache.org>.
GitHub user doanduyhai reopened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/746

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

    ### What is this PR for?
    Right now, when calling the REST API `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` Zeppelin always returns **OK** as shown by this source code: https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java#L477
    
    This ticket will update the behavior so that Zeppelin also return the result of the paragraph execution
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Code Review
    * [ ] - Simple Test
    
    ### Is there a relevant Jira issue?
    **[ZEPPELIN-699]**
    
    ### How should this be tested?
    * `git fetch origin pull/746/head:ParagraphExecutionRESTAPI`
    * `git checkout ParagraphExecutionRESTAPI`
    * `mvn clean package -DskipTests`
    * `bin/zeppelin-daemon.sh restart`
    * Create a new note
    * In the first paragraph, put the following code
    
    ```scala
    %sh
    
    echo "Current time = "`date +"%T"
    ```
    
    * Retrieve the current note id in the URL
    * Retrieve the current paragraph id
    * Use a REST Client like **[POSTman]** to create a HTTP POST query `http://<ip>:<port>/api/notebook/run/<note_id>/<paragraph_id>` 
    * You should receive something similar as follow for answer
    
    ```
    {
        "status": "OK",
        "body": {
            "code": "SUCCESS",
            "type": "TEXT",
            "msg": "Current time = 16:14:18\n"
        }
    }
    ```
    
    ### Screenshots (if appropriate)
    ![zeppelin_synchronous_rest_api](https://cloud.githubusercontent.com/assets/1532977/15748069/b4a26a46-28dd-11e6-8f51-aa13ddba3f1c.gif)
    
    API Documentation update
    
    **Existing asynchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773274/5b508cae-2976-11e6-9e52-14d8b7e7828e.png)
    
    **New synchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773309/84965a94-2976-11e6-9719-81d8b555c3c4.png)
    
    
    ### Questions:
    * Does the licenses files need update? --> **No**
    * Is there breaking changes for older versions? --> **No**
    * Does this needs documentation? --> **Yes**
    
    [ZEPPELIN-699]: https://issues.apache.org/jira/browse/ZEPPELIN-699
    [POSTman]: https://www.getpostman.com/

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

    $ git pull https://github.com/doanduyhai/incubator-zeppelin ZEPPELIN-699

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

    https://github.com/apache/incubator-zeppelin/pull/746.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 #746
    
----
commit dfdf2709e2e0b6f8f2a676e73ecf98e4b0321223
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:15:46Z

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

commit b4e181630e9a2ec5df6a01a61100cd83e1db2224
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:24:38Z

    [ZEPPELIN-699] Update Notebook REST API documentation

commit 39853d30e808b1596dd69657ec40dc89ade3c976
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-03T08:20:17Z

    [ZEPPELIN-699] Fix doc after code review

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-196526150
  
    @doanduyhai typically async pattern in REST, it seems, is either:
    1. server callback
    2. list status
    So IMO if we submit a request o run paragraph asynchronously, and that request is accepted, then we would get back a 202 (preferred) or 200, and through some other ways we would check how the request is going.



---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    @felixcheung I've updated the PR to create a new API as suggested by @Leemoonsoo : `/run/{noteId}/{paragraphId}`.
    
    All my apologies for the long delay, I was lately busy with Cassandra


---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

Posted by doanduyhai <gi...@git.apache.org>.
GitHub user doanduyhai reopened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/746

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

    ### What is this PR for?
    Right now, when calling the REST API `http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>` Zeppelin always returns **OK** as shown by this source code: https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java#L477
    
    This ticket will update the behavior so that Zeppelin also return the result of the paragraph execution
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Code Review
    * [ ] - Simple Test
    
    ### Is there a relevant Jira issue?
    **[ZEPPELIN-699]**
    
    ### How should this be tested?
    * `git fetch origin pull/746/head:ParagraphExecutionRESTAPI`
    * `git checkout ParagraphExecutionRESTAPI`
    * `mvn clean package -DskipTests`
    * `bin/zeppelin-daemon.sh restart`
    * Create a new note
    * In the first paragraph, put the following code
    
    ```scala
    %sh
    
    echo "Current time = "`date +"%T"
    ```
    
    * Retrieve the current note id in the URL
    * Retrieve the current paragraph id
    * Use a REST Client like **[POSTman]** to create a HTTP POST query `http://<ip>:<port>/api/notebook/run/<note_id>/<paragraph_id>` 
    * You should receive something similar as follow for answer
    
    ```
    {
        "status": "OK",
        "body": {
            "code": "SUCCESS",
            "type": "TEXT",
            "msg": "Current time = 16:14:18\n"
        }
    }
    ```
    
    ### Screenshots (if appropriate)
    ![zeppelin_synchronous_rest_api](https://cloud.githubusercontent.com/assets/1532977/15748069/b4a26a46-28dd-11e6-8f51-aa13ddba3f1c.gif)
    
    API Documentation update
    
    **Existing asynchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773274/5b508cae-2976-11e6-9e52-14d8b7e7828e.png)
    
    **New synchronous API**
    ![image](https://cloud.githubusercontent.com/assets/1532977/15773309/84965a94-2976-11e6-9719-81d8b555c3c4.png)
    
    
    ### Questions:
    * Does the licenses files need update? --> **No**
    * Is there breaking changes for older versions? --> **No**
    * Does this needs documentation? --> **Yes**
    
    [ZEPPELIN-699]: https://issues.apache.org/jira/browse/ZEPPELIN-699
    [POSTman]: https://www.getpostman.com/

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

    $ git pull https://github.com/doanduyhai/incubator-zeppelin ZEPPELIN-699

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

    https://github.com/apache/incubator-zeppelin/pull/746.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 #746
    
----
commit c8825f7ff639f554862a6933c4a259cd14389f51
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:15:46Z

    [ZEPPELIN-699] Add new synchronous paragraph run REST API

commit 695c13e762bad5a31bf47131ccae196062aa5889
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-02T14:24:38Z

    [ZEPPELIN-699] Update Notebook REST API documentation

commit 46a2f3662aab87edf522e92bfa7700170230263b
Author: DuyHai DOAN <do...@gmail.com>
Date:   2016-06-03T08:20:17Z

    [ZEPPELIN-699] Fix doc after code review

----


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by fireboy1919 <gi...@git.apache.org>.
Github user fireboy1919 commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-196497526
  
    I have a similar REST endpoint based upon the use of resource pools (i.e., starting from this pull request):
    https://github.com/apache/incubator-zeppelin/pull/763#issuecomment-196085361
    
    I made mine use this form:
    http://<ip>:<port>/api/notebook/job/<note_id>/<paragraph_id>/message
    
    This is a bit more flexible than pulling straight from the paragraph and does not have synchronous API problem.
    
    Once that new pull request has been approved, I'll format and post my pull request.


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

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

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188842837
  
    @doanduyhai Right. 
    
    for asynchronous behavior, multiple API is required to control the job, like submitting job (POST), canceling job (DELETE), get status of job (GET) to `/api/notebook/job/<note_id>/<paragraph_id>`.
    
    but for synchronous behavior, a single API will go through all the lifecycle of the job, from submitting to get result. So how about just say
    
    `/api/notebook/run/<note_id>/<paragraph_id>` ?
    



---
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] incubator-zeppelin pull request #746: [ZEPPELIN-699] Add new synchronous par...

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

    https://github.com/apache/incubator-zeppelin/pull/746


---
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] incubator-zeppelin pull request: [ZEPPELIN-699] Return Paragraph e...

Posted by vgromakowski <gi...@git.apache.org>.
Github user vgromakowski commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/746#issuecomment-188683473
  
    I would prefer to not return OK in case of failure


---
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] incubator-zeppelin issue #746: [ZEPPELIN-699] Add new synchronous paragraph ...

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

    https://github.com/apache/incubator-zeppelin/pull/746
  
    @doanduyhai I think it was due to flaky-test that is tracked under [ZEPPELIN-878](https://issues.apache.org/jira/browse/ZEPPELIN-878).
    
    Now it's fixed in master and reading this branch helped, CI is green!


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