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 2022/01/13 23:02:15 UTC
[GitHub] [zeppelin] woowahan-jaehoon opened a new pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
woowahan-jaehoon opened a new pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288
### What is this PR for?
Skip load notebook when it is damaged.
When I upgraded zeppelin 0.7 to 0.9 and I converted notebook, some notebooks are damaged. Caused damaged notebooks, zeppelin cannot start completely. But damaged notebooks can find very very hard.
### What type of PR is it?
[Bug Fix]
### Todos
* Nothing
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-5632
### How should this be tested?
* Maybe create damaged notebook and start zeppelin...?
### 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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816549086
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
I see, could you add one comment to explain that the invalid file name would cause exception 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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816521622
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
Could you paste the stacktrace? It is weird how damaged notebook will happen if no reading is invoked.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Mabin-J commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
Mabin-J commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816537601
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
Unfortunately, I cannot get stacktrace, anymore because I moved company.
I think that maybe filenames or folder names were weird.
When I saw that exception, I tried to upgrade 0.7 to 0.9. So I ran script for upgrade notebooks. Many staff of company used zeppelin (not only data team), many notebook's name could be bad. (even use `*`, `(`, `)`,`[`, `]`, etc)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r793226376
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
I think it is better to catch the exception and log the warning when invoking NotebookRepos#get
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
And please rebase your PR first.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#issuecomment-1066509077
@woowahan-jaehoon Can you add unit test for it?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] woowahan-jaehoon commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
woowahan-jaehoon commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816519912
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
> I think it is better to catch the exception and log the warning when invoking NotebookRepos#get
In my case, Throwed exception that line when I started zeppelin, not open notebook.
I checked `addOrUpdateNoteNode` method, It isn't use `NotebookRepos#get`.
Why do you think that?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#issuecomment-1055107887
Thanks @woowahan-jaehoon , could you take a look at this comment?https://github.com/apache/zeppelin/pull/4288#discussion_r793226376
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#issuecomment-1033328087
ping @woowahan-jaehoon
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] woowahan-jaehoon commented on pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
woowahan-jaehoon commented on pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#issuecomment-1055105277
> @woowahan-jaehoon Could you rebase it?
I did.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] zjffdu commented on pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#issuecomment-1048620007
@woowahan-jaehoon Could you rebase it?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Mabin-J commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
Mabin-J commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816537601
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
Unfortunately, I cannot get stacktrace, anymore because I moved company.
I think that maybe filenames or folder names were weird.
When I saw that exception, I tried to upgrade 0.7 to 0.9. So I ran script for upgrade notebooks. Many staff of company used zeppelin (not only data team), many notebook's name could be bad. (even use `*`, `(`, `)`,`[`, `]`, etc). Many bad notebook name affect filename when upgrade notebook.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Mabin-J commented on a change in pull request #4288: [ZEPPELIN-5632] zeppelin-zengine: Skip load notebook when it is damaged.
Posted by GitBox <gi...@apache.org>.
Mabin-J commented on a change in pull request #4288:
URL: https://github.com/apache/zeppelin/pull/4288#discussion_r816537601
##########
File path: zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NoteManager.java
##########
@@ -78,8 +78,8 @@ private void init() throws IOException {
{
try {
addOrUpdateNoteNode(new Note(new NoteInfo(entry.getKey(), entry.getValue())));
- } catch (IOException e) {
- LOGGER.warn(e.getMessage());
+ } catch (Throwable e) {
+ LOGGER.warn("addOrUpdateNoteNode Fail: {}", entry.getKey() + " - " + e.getMessage(), e);
Review comment:
Unfortunately, I cannot get stacktrace, anymore because I moved company.
I think that maybe filenames or folder names were weird.
When I saw that exception, I tried to upgrade 0.7 to 0.9. So I ran script for upgrade notebooks. Many staff of company used zeppelin (not only data team), many notebook's name could be bad. (even use `*`, `(`, `)`,`[`, `]`, etc). Many bad notebook name maybe affect filename or foldername when upgrade notebook.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org