Skip to content

Conversation

@pravin-dsilva
Copy link
Contributor

What is this PR for?

The stacktrace for this issue gives the error:

06:31:15,263 ERROR org.apache.zeppelin.notebook.repo.VFSNotebookRepo:151 - Can't read note file:///tmp/ZeppelinLTest_1495261875233/notebook/2BQA35CJZ
com.google.gson.JsonSyntaxException: 2016-03-29T16:21:09-0700

This issue is related to notebooks failing to load with date ParseException and was fixed in ZEPPELIN-1129. However, in ZEPPELIN-2395 the fix for old format is excluded and hence the issue occurs again. Added the same code to fix the old date format issue in the fromJSON method of Note.java

What type of PR is it?

Bug Fix

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-2471

How should this be tested?

Tests should pass on CI

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

public static Note fromJson(String json) {
GsonBuilder gsonBuilder = new GsonBuilder();
gsonBuilder.setPrettyPrinting();
Gson gson = gsonBuilder.registerTypeAdapter(Date.class, new NotebookImportDeserializer())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation is off, could you please fix that.
also this seems to be the style in use in this file

InterpreterResult intpResult =
           new InterpreterResult(InterpreterResult.Code.ERROR, intpException.getMessage());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made a few changes, could you please review and let me know if anything else needs to be changed? Making any other changes seems to give me checkstyle errors.

@felixcheung
Copy link
Member

There's one test failure - could you take a look https://travis-ci.org/pravin-dsilva/zeppelin/jobs/235945509

@pravin-dsilva
Copy link
Contributor Author

The failure that i see is

test(org.apache.zeppelin.interpreter.InterpreterOutputChangeWatcherTest)  Time elapsed: 2.034 sec  <<< FAILURE!
java.lang.AssertionError: expected:<1> but was:<2>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:645)
	at org.junit.Assert.assertEquals(Assert.java:631)
	at org.apache.zeppelin.interpreter.InterpreterOutputChangeWatcherTest.test(InterpreterOutputChangeWatcherTest.java:95)

This doesnt seem related to my change, but could anyone confirm this?

@Leemoonsoo
Copy link
Member

Thanks @pravin-dsilva for the fix.

LGTM and merge to master if no further comment.

@asfgit asfgit closed this in e49baf7 Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants