[ZEPPELIN-4981]. Zeppelin Client API#3887
Conversation
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
9 times, most recently
from
9d366ad to
8f9e129
Compare
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
7 times, most recently
from
e8d3eb7 to
8391a94
Compare
Reamer
left a comment
Reamer
left a comment
There was a problem hiding this comment.
First, I think this is a very big PR, and I hope that some others will also review this PR. Overall it seems to be really good. I see that the library unirest-java did the communication, this library seems useful.
I have seen that you catch all/most API Exceptions (example)
I think it is better to use an ExceptionMapper. If not, then you should remove the `throws' declaration.
| * @throws Exception | ||
| */ | ||
| private void checkResponse(HttpResponse<JsonNode> response) throws Exception { | ||
| if (response.getStatus() == 302) { |
There was a problem hiding this comment.
302 is the answer to a redirection. 401 is the status code for Unauthorized. Are we responding with an incorrect status code from the Zeppelin server?
There was a problem hiding this comment.
Zeppelin would redirect user to login page when user is unauthorized. Here I just assume redirect means unauthorized because I think this is the only place to have redirect response in zeppelin. But it is true that we should find a better way to detect unauthorized
| * @return | ||
| * @throws Exception | ||
| */ | ||
| public NoteResult waitUntilNoteFinished(String noteId) throws Exception { |
There was a problem hiding this comment.
This function can lead to to an infinity loop. I would call waitUntilNoteFinished(String noteId, long timeoutInMills) with a high default timeout.
There was a problem hiding this comment.
There's another method waitUntilNoteFinished(String noteId, long timeoutInMills) which allow user to set timeout. The reason I didn't add a default large timeout is because of streaming scenario that may run forever, such as flink streaming job.
Thanks for the review @Reamer , I have update the PR to use ExceptionMapper. |
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
2 times, most recently
from
4741643 to
bf5788d
Compare
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
from
bf5788d to
c2383b5
Compare
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
2 times, most recently
from
6ada563 to
ed58f59
Compare
zjffdu
force-pushed
the
ZEPPELIN-4981
branch
from
ed58f59 to
8eddf3e
Compare
|
will merge it soon if no more comment |
### What is this PR for? This is the initial PR fo introducing zeppelin client api. You can check this google doc for the overall design. https://docs.google.com/document/d/1bLLKKxleZlZpP9EFJlLLkJKwDBps-RNvzNwh3LFZWZ4/edit?usp=sharing In this PR, I add 2 modules: zeppelin-client, zeppelin-client-examples. You can take a look at the module zeppelin-client-examples to see how we can use zeppelin client api. ### What type of PR is it? [ Feature ] ### Todos * [ ] - Task ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-4981 ### How should this be tested? * Manually tested and Unit test is added. ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Jeff Zhang <[email protected]> Closes #3887 from zjffdu/ZEPPELIN-4981 and squashes the following commits: 9b8ff42 [Jeff Zhang] update travis 8eddf3e [Jeff Zhang] use ExceptionMapper bb05371 [Jeff Zhang] update 4205c1c [Jeff Zhang] code refactoring 53fe1c1 [Jeff Zhang] update examples 487cb8c [Jeff Zhang] minor update d7d11f7 [Jeff Zhang] minor update f5e1e2e [Jeff Zhang] add session reconnect bc723e7 [Jeff Zhang] minor update 7fbd481 [Jeff Zhang] minor update fe22986 [Jeff Zhang] add sessionId to ZSession c770176 [Jeff Zhang] add session status api 1dbc414 [Jeff Zhang] move classes to websocket folder 0e43ce1 [Jeff Zhang] add more examples 39d230b [Jeff Zhang] fix ut d4bbe95 [Jeff Zhang] add zeppelin client examples 5ca4995 [Jeff Zhang] update 5611b7b [Jeff Zhang] resolve gson dependency issue 1aa1d68 [Jeff Zhang] [ZEPPELIN-4981]. Zeppelin Client API (Zeppelin SDK) (cherry picked from commit f11e6f3) Signed-off-by: Jeff Zhang <[email protected]>
2 participants
What is this PR for?
This is the initial PR fo introducing zeppelin client api. You can check this google doc for the overall design. https://docs.google.com/document/d/1bLLKKxleZlZpP9EFJlLLkJKwDBps-RNvzNwh3LFZWZ4/edit?usp=sharing
In this PR, I add 2 modules: zeppelin-client, zeppelin-client-examples.
You can take a look at the module zeppelin-client-examples to see how we can use zeppelin client api.
What type of PR is it?
[ Feature ]
Todos
What is the Jira issue?
How should this be tested?
Screenshots (if appropriate)
Questions: