You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by HeartSaVioR <gi...@git.apache.org> on 2015/12/18 02:22:07 UTC

[GitHub] incubator-zeppelin pull request: ZEPPELIN-511 REST API: Insert / R...

GitHub user HeartSaVioR opened a pull request:

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

    ZEPPELIN-511 REST API: Insert / Retrieve / Move / Delete operation for paragraph

    ### What is this PR for?
    
    This issue is intended to fill gap between REST API and WebSocket operations.
    For now we can only access notebook and paragraph to READONLY (not writing) via REST API but after this PR, we can insert / retrieve / move / delete paragraph via REST API.
    
    ### What type of PR is it?
    
    Feature
    
    ### Todos
    * [ ] - 
    
    ### Is there a relevant Jira issue?
    
    https://issues.apache.org/jira/browse/ZEPPELIN-511
    
    ### How should this be tested?
    
    Please follow the explanation of added REST APIs from rest-notebook.md.
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? (No)
    * Is there breaking changes for older versions? (No)
    * Does this needs documentation? (Yes, I've addressed it.)

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

    $ git pull https://github.com/HeartSaVioR/incubator-zeppelin ZEPPELIN-511

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

    https://github.com/apache/incubator-zeppelin/pull/550.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 #550
    
----
commit f6c49ad784c8b9f46f9daf638e81da9b888d405f
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2015-12-17T22:42:27Z

    ZEPPELIN-511 REST API: Insert / Retrieve / Move / Delete operation for paragraph
    
    * implements 4 REST APIs about the paragraph
    * Added unit test to test this features

commit f396df8cca3f6084f6efa46f5df4645376b28699
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2015-12-18T01:03:45Z

    ZEPPELIN-511 "insert paragraph" api: return created paragraph id
    
    * also change http status code to 201 CREATED

commit c305a8912230a67a6f78643d700aeded64ef8102
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2015-12-18T01:17:24Z

    ZEPPELIN-511 explain new APIs to read-notebook.md

----


---
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-511 REST API: Insert / R...

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

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


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-167347226
  
    Looks good to me.
    Merge if there're no more discussions.


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-166167929
  
    Thanks for adding really useful REST APIs!
    Looks good to me!


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-167165446
  
    Done with another upmerge.


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-166212083
  
    @HeartSaVioR few more things
    
    I think you should do a clean up as well after every test case execution
    
        ZeppelinServer.notebook.removeNote(note.getId());
    
    Also when I run this, line 538;
    
        PostMethod post = httpPost("/notebook/" + note.getId() + "/paragraph/" + p.getId() + "/move/" + 10, "");
    
    it returns 
    
        {"status":"OK","message":""}
      
    Which i think is also not appropriate. Since, this was an invalid operation (total nos of paragraph = 2). This should have either returned some error, or should have moved 1st paragraph to the last location.


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-167437094
  
    @HeartSaVioR Thanks for quick response. LGTM


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-167372073
  
    @minahlee No problem. Rebased via creating new branch, cherry-pick, and reset.


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-167437166
  
    LGTM. Merging it into 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] incubator-zeppelin pull request: ZEPPELIN-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-166469145
  
    Upmerged. Please take a look again, thanks!


---
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-511 REST API: Insert / R...

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

    https://github.com/apache/incubator-zeppelin/pull/550#issuecomment-166220799
  
    @prabhjyotsingh 
    Thanks for the review. Addressed your comments.
    Regarding moving paragraph to invalid index, I think it could be treated as ```bad request```, so I applied it. Would it be fine for 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.
---