[ZEPPELIN-1313] NullPointerException when using Clone notebook REST API #1348
Conversation
|
Reopening to trigger the CI build. |
|
Reopening to trigger the CI build. |
|
The CI build is green refer to https://travis-ci.org/apache/zeppelin/builds/154093445 Could someone please review this? |
| if (newNoteName != null) { | ||
| newNote.setName(newNoteName); | ||
| } else { | ||
| newNote.setName("Note " + newNote.getId()); |
There was a problem hiding this comment.
Those methods returns the same. I think we need to remove note.id().
There was a problem hiding this comment.
@felixcheung yes we use only note.getId() even in other place like create notebook.
@jongyoul I will remove note.id() with this.
There was a problem hiding this comment.
@kavinkumarks note.id() is used widely and it's better to handle with another PR.
There was a problem hiding this comment.
@jongyoul I did the changes already with this ticket and running CI build now.Please let me know if I need to revert them.
|
LGTM |
1 similar comment
|
LGTM |
set the default name for the case.Added test cases too.
kavinkumarks
force-pushed
the
zeppelin-1313-fix-NPE-clone-notebook
branch
from
3509f1e to
941e13b
Compare
|
@kavinkumarks Thanks for the removing the method, but personally, I think we should handle it with another PR. @felixcheung What do you think of it? It looks irrelevant. |
|
I agree, that should be a separate PR/JIRA. I'm ok with including it now since changes are already made? Is CI passing? |
|
@felixcheung Yeah, @kavinkumarks already did it, thus I also am OK to include it. CI failure is not related to this PR. LGTM |
|
@felixcheung / @jongyoul could you please take care of merging this? Thanks, |
|
@felixcheung / @jongyoul any updates on this? Thanks, |
|
@AhyoungRyu Could you please merge this PR? This will help you |
|
I could also merge this tomorrow if you are having problems.
Sorry about the delay.
|
|
@felixcheung I don't have any problem. I'd like to give @AhyoungRyu a chance to merge PR for the first time. :-) |
|
Right, when I say "you" I mean Ahyoung :) |
|
@jongyoul @felixcheung Sure. Let me try :D |
|
Thanks everyone! |
4 participants
What is this PR for?
This fixes when the input json is empty for the clone notebook REST API and for this case the default name will be populated.
What type of PR is it?
Bug Fix
Todos
NA
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1313
How should this be tested?
When the input json is empty for the clone notebook REST API, the response should be 201 with the default name populated.
Screenshots (if appropriate)
Questions: