ZEPPELIN-511 REST API: Insert / Retrieve / Move / Delete operation for paragraph by HeartSaVioR · Pull Request #550 · apache/zeppelin

HeartSaVioR · 2015-12-18T01:22:05Z

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

Leemoonsoo · 2015-12-21T00:28:45Z

Thanks for adding really useful REST APIs!
Looks good to me!

prabhjyotsingh · 2015-12-21T05:50:38Z

zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java

This variable should be p2

@prabhjyotsingh Good finding! Will fix.

prabhjyotsingh · 2015-12-21T06:11:14Z

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

prabhjyotsingh · 2015-12-21T06:14:07Z

zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java

This should be also post2.

Thanks for finding, will fix.

HeartSaVioR · 2015-12-21T07:19:17Z

@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?

HeartSaVioR · 2015-12-22T00:53:35Z

Upmerged. Please take a look again, thanks!

HeartSaVioR · 2015-12-24T23:02:29Z

Done with another upmerge.

Leemoonsoo · 2015-12-26T17:27:26Z

Looks good to me.
Merge if there're no more discussions.

minahlee · 2015-12-26T21:03:17Z

Would you mind to do rebase than merge? It will be easier to keep track who has credits on merged code.

…r paragraph * implements 4 REST APIs about the paragraph * Added unit test to test this features Conflicts: zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java

* also change http status code to 201 CREATED

Conflicts: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java

HeartSaVioR · 2015-12-26T23:58:42Z

@minahlee No problem. Rebased via creating new branch, cherry-pick, and reset branch's head.

minahlee · 2015-12-27T19:24:20Z

@HeartSaVioR Thanks for quick response. LGTM

Leemoonsoo · 2015-12-27T19:26:15Z

LGTM. Merging it into master

prabhjyotsingh reviewed Dec 21, 2015
View reviewed changes

HeartSaVioR added 4 commits December 27, 2015 08:36

ZEPPELIN-511 REST API: Insert / Retrieve / Move / Delete operation fo…

3658b00

…r paragraph * implements 4 REST APIs about the paragraph * Added unit test to test this features Conflicts: zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java

ZEPPELIN-511 "insert paragraph" api: return created paragraph id

cc71e06

* also change http status code to 201 CREATED

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

01feed4

ZEPPELIN-511 Address @prabhjyotsingh reviews

59f5f61

Conflicts: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java

HeartSaVioR force-pushed the ZEPPELIN-511 branch from d4425fb to 59f5f61 Compare December 26, 2015 23:56

asfgit closed this in 3bfd97e Dec 27, 2015

Conversation

HeartSaVioR commented Dec 18, 2015

What is this PR for?

What type of PR is it?

Todos

Is there a relevant Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

Leemoonsoo commented Dec 21, 2015

Uh oh!

prabhjyotsingh Dec 21, 2015

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Dec 21, 2015

Choose a reason for hiding this comment

Uh oh!

prabhjyotsingh commented Dec 21, 2015

Uh oh!

prabhjyotsingh Dec 21, 2015

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Dec 21, 2015

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR commented Dec 21, 2015

Uh oh!

HeartSaVioR commented Dec 22, 2015

Uh oh!

HeartSaVioR commented Dec 24, 2015

Uh oh!

Leemoonsoo commented Dec 26, 2015

Uh oh!

minahlee commented Dec 26, 2015

Uh oh!

HeartSaVioR commented Dec 26, 2015

Uh oh!

minahlee commented Dec 27, 2015

Uh oh!

Leemoonsoo commented Dec 27, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants