You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2020/03/01 13:58:58 UTC
[GitHub] [zeppelin] zjffdu opened a new pull request #3672: [ZEPPELIN-4661].
Fail to run whole note
zjffdu opened a new pull request #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672
### What is this PR for?
This is to fix the regression issue caused by ZEPPELIN-4661. The root cause is that in the frontend we use `paragraph` to represent the script text while in the backend we use `text`, so that converting between frontend data to backend class `Paragraph` will cause this kind of error.
### What type of PR is it?
[Bug Fix ]
### Todos
* [ ] - Task
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-4661
### How should this be tested?
* CI pass and manually tested
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? NO
* Is there breaking changes for older versions? No
* Does this needs documentation? NO
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail
to run whole note
Posted by GitBox <gi...@apache.org>.
zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#issuecomment-593864281
Thanks @alexott , I will merge it soon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] Leemoonsoo commented on issue #3672: [ZEPPELIN-4661].
Fail to run whole note
Posted by GitBox <gi...@apache.org>.
Leemoonsoo commented on issue #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#issuecomment-593682350
Thanks @zjffdu for addressing it. It looks good to me!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] asfgit closed pull request #3672: [ZEPPELIN-4661]. Fail
to run whole note
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] alexott commented on a change in pull request #3672:
[ZEPPELIN-4661]. Fail to run whole note
Posted by GitBox <gi...@apache.org>.
alexott commented on a change in pull request #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#discussion_r386113899
##########
File path: zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
##########
@@ -376,25 +375,33 @@ public boolean runAllParagraphs(String noteId,
return false;
}
- if (paragraphs == null) {
- paragraphs = gson.fromJson(gson.toJson(note.getParagraphs()), new TypeToken<List>(){}.getType());
- }
-
note.setRunning(true);
try {
- for (Map<String, Object> raw : paragraphs) {
- String paragraphId = (String) raw.get("id");
- if (paragraphId == null) {
- continue;
+ if (paragraphs != null) {
+ // run note via the data passed from frontend
+ for (Map<String, Object> raw : paragraphs) {
+ String paragraphId = (String) raw.get("id");
+ if (paragraphId == null) {
+ continue;
+ }
+ String text = (String) raw.get("paragraph");
+ String title = (String) raw.get("title");
+ Map<String, Object> params = (Map<String, Object>) raw.get("params");
+ Map<String, Object> config = (Map<String, Object>) raw.get("config");
Review comment:
Should we wrap this into `try`, so if the input data have incorrect type, we handle this gracefully...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] alexott commented on a change in pull request #3672:
[ZEPPELIN-4661]. Fail to run whole note
Posted by GitBox <gi...@apache.org>.
alexott commented on a change in pull request #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#discussion_r386113737
##########
File path: zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
##########
@@ -376,25 +375,33 @@ public boolean runAllParagraphs(String noteId,
return false;
}
- if (paragraphs == null) {
- paragraphs = gson.fromJson(gson.toJson(note.getParagraphs()), new TypeToken<List>(){}.getType());
- }
-
note.setRunning(true);
try {
- for (Map<String, Object> raw : paragraphs) {
- String paragraphId = (String) raw.get("id");
- if (paragraphId == null) {
- continue;
+ if (paragraphs != null) {
+ // run note via the data passed from frontend
+ for (Map<String, Object> raw : paragraphs) {
+ String paragraphId = (String) raw.get("id");
+ if (paragraphId == null) {
+ continue;
+ }
+ String text = (String) raw.get("paragraph");
+ String title = (String) raw.get("title");
+ Map<String, Object> params = (Map<String, Object>) raw.get("params");
+ Map<String, Object> config = (Map<String, Object>) raw.get("config");
+
+ if (!runParagraph(noteId, paragraphId, title, text, params, config, false, true,
+ context, callback)) {
+ // stop execution when one paragraph fails.
Review comment:
I maybe would add more logging around `noteId` and `paragraphId` into the `runParagraph`. WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] alexott commented on a change in pull request #3672:
[ZEPPELIN-4661]. Fail to run whole note
Posted by GitBox <gi...@apache.org>.
alexott commented on a change in pull request #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#discussion_r386113781
##########
File path: zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
##########
@@ -376,25 +375,33 @@ public boolean runAllParagraphs(String noteId,
return false;
}
- if (paragraphs == null) {
- paragraphs = gson.fromJson(gson.toJson(note.getParagraphs()), new TypeToken<List>(){}.getType());
- }
-
note.setRunning(true);
try {
- for (Map<String, Object> raw : paragraphs) {
- String paragraphId = (String) raw.get("id");
- if (paragraphId == null) {
- continue;
+ if (paragraphs != null) {
+ // run note via the data passed from frontend
+ for (Map<String, Object> raw : paragraphs) {
+ String paragraphId = (String) raw.get("id");
+ if (paragraphId == null) {
Review comment:
maybe add a debug message here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail
to run whole note
Posted by GitBox <gi...@apache.org>.
zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#issuecomment-593100490
@Leemoonsoo Could you help review it ? Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [zeppelin] zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail
to run whole note
Posted by GitBox <gi...@apache.org>.
zjffdu commented on issue #3672: [ZEPPELIN-4661]. Fail to run whole note
URL: https://github.com/apache/zeppelin/pull/3672#issuecomment-593758606
Thanks @Leemoonsoo And @alexott I have addressed your comment, do you have any other comment ? Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services