You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by liuxunorg <gi...@git.apache.org> on 2018/12/02 02:44:49 UTC
[GitHub] zeppelin pull request #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs w...
GitHub user liuxunorg opened a pull request:
https://github.com/apache/zeppelin/pull/3245
[ZEPPELIN-3893] Bug Fix that clear paragraphs when executing the Paragraph API asynchronously
### What is this PR for?
When calling the asynchronous execution of the paragraph API,
```
http://[zeppelin-server]:[zeppelin-port]/api/notebook/run/[noteId]/[paragraphId]
```
The title and text of the paragraph will be cleared.
### What type of PR is it?
[Bug Fix]
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3893
### How should this be tested?
[CI pass](https://travis-ci.org/liuxunorg/zeppelin/builds/462203315)
### Screenshots (if appropriate)
No
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liuxunorg/zeppelin ZEPPELIN-3893
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/3245.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 #3245
----
commit 5a98ced52d0e7cf0c18a8380ca06d9ce692249f3
Author: liuxunorg <33...@...>
Date: 2018-12-01T16:09:57Z
[ZEPPELIN-3893] Bug Fix that clear paragraphs when executing the Paragraph API asynchronously
### What is this PR for?
When calling the asynchronous execution of the paragraph API,
```
http://[zeppelin-server]:[zeppelin-port]/api/notebook/run/[noteId]/[paragraphId]
```
The title and text of the paragraph will be cleared.
### What type of PR is it?
[Bug Fix]
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-3893
### How should this be tested?
[CI pass](https://travis-ci.org/liuxunorg/zeppelin/builds/418742522)
### Screenshots (if appropriate)
No
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
----
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/zeppelin/pull/3245
merging if no more comment
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by liuxunorg <gi...@git.apache.org>.
Github user liuxunorg commented on the issue:
https://github.com/apache/zeppelin/pull/3245
@jongyoul , pass of [CI](https://travis-ci.org/liuxunorg/zeppelin/builds/463120804)
---
[GitHub] zeppelin pull request #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs w...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/3245
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by liuxunorg <gi...@git.apache.org>.
Github user liuxunorg commented on the issue:
https://github.com/apache/zeppelin/pull/3245
Hi @zjffdu , This bug fix contains test cases that are asynchronous to the runParagraph API. Should it be helpful for the zeppelin, or should the PR be merged first?
When you submit a new `runParapraph()` function, the `runParapraph()` function is called in many places in the zeppelin. Do you modify all the calls to `runParapraph()` together?
After you submit the PR of the new `runParapraph()` function, Look again if you still need to merge my PR, you decide.
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by liuxunorg <gi...@git.apache.org>.
Github user liuxunorg commented on the issue:
https://github.com/apache/zeppelin/pull/3245
Hi @felixcheung , I added a test case. Please help me review the code, thank you!
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by liuxunorg <gi...@git.apache.org>.
Github user liuxunorg commented on the issue:
https://github.com/apache/zeppelin/pull/3245
@zjffdu ,Thank you a for the suggestion, I wrote a comment on your issue.
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by liuxunorg <gi...@git.apache.org>.
Github user liuxunorg commented on the issue:
https://github.com/apache/zeppelin/pull/3245
@zjffdu , @felixcheung , Please help me review the code, thank you!
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/3245
LGTM. Waiting for a pass of CI
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3245
@liuxunorg This is the change what I suggest
https://github.com/zjffdu/zeppelin/commit/29593d74bac17f124e97fc43a4942e3365a12ed7
Could you verify whether it solve this issue ?
---
[GitHub] zeppelin issue #3245: [ZEPPELIN-3893] Bug Fix that clear paragraphs when exe...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3245
Hi @liuxunorg I would prefer to add another method
```
public boolean runParagraph(String noteId,
String paragraphId,
Map<String, Object> params,
Map<String, Object> config,
boolean failIfDisabled,
boolean blocking,
ServiceContext context,
ServiceCallback<Paragraph> callback) throws IOException {
```
which would delegate to existing runParagraph method.
This is more clean for me.
---