Skip to content

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Jun 21, 2016

What is this PR for?

This bug seems to be produced by #862. Currently a exported notebook is not imported with below error message.

ERROR [2016-06-20 17:19:21,797] ({qtp559670971-14} NotebookServer.java[onMessage]:231) - Can't handle message
com.google.gson.JsonSyntaxException: 2016-06-20T14:33:31-0700
    at com.google.gson.internal.bind.DateTypeAdapter.deserializeToDate(DateTypeAdapter.java:81)
    at com.google.gson.internal.bind.DateTypeAdapter.read(DateTypeAdapter.java:66)
    at com.google.gson.internal.bind.DateTypeAdapter.read(DateTypeAdapter.java:41)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:93)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:172)
    at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.read(TypeAdapterRuntimeTypeWrapper.java:40)
    at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.read(CollectionTypeAdapterFactory.java:81)
    at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.read(CollectionTypeAdapterFactory.java:60)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:93)
    at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:172)
    at com.google.gson.Gson.fromJson(Gson.java:791)
    at org.apache.zeppelin.notebook.Notebook.importNote(Notebook.java:199)
    at org.apache.zeppelin.socket.NotebookServer.importNote(NotebookServer.java:656)
    at org.apache.zeppelin.socket.NotebookServer.onMessage(NotebookServer.java:175)
    at org.apache.zeppelin.socket.NotebookSocket.onWebSocketText(NotebookSocket.java:56)
    at org.eclipse.jetty.websocket.common.events.JettyListenerEventDriver.onTextMessage(JettyListenerEventDriver.java:128)
    at org.eclipse.jetty.websocket.common.message.SimpleTextMessage.messageComplete(SimpleTextMessage.java:69)
    at org.eclipse.jetty.websocket.common.events.AbstractEventDriver.appendMessage(AbstractEventDriver.java:65)
    at org.eclipse.jetty.websocket.common.events.JettyListenerEventDriver.onTextFrame(JettyListenerEventDriver.java:122)
    at org.eclipse.jetty.websocket.common.events.AbstractEventDriver.incomingFrame(AbstractEventDriver.java:161)
    at org.eclipse.jetty.websocket.common.WebSocketSession.incomingFrame(WebSocketSession.java:309)
    at org.eclipse.jetty.websocket.common.extensions.ExtensionStack.incomingFrame(ExtensionStack.java:214)
    at org.eclipse.jetty.websocket.common.Parser.notifyFrame(Parser.java:220)
    at org.eclipse.jetty.websocket.common.Parser.parse(Parser.java:258)
    at org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.readParse(AbstractWebSocketConnection.java:632)
    at org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.onFillable(AbstractWebSocketConnection.java:480)
    at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
    at java.lang.Thread.run(Thread.java:745)
Caused by: java.text.ParseException: Unparseable date: "2016-06-20T14:33:31-0700"
    at java.text.DateFormat.parse(DateFormat.java:366)
    at com.google.gson.internal.bind.DateTypeAdapter.deserializeToDate(DateTypeAdapter.java:79)

What type of PR is it?

Bug Fix

Todos

What is the Jira issue?

ZEPPELIN-1028

How should this be tested?

  1. Apply this patch (Build the source and restart Zeppelin)
  2. Export a notebook and try to import it again
  3. It should be imported as before

Screenshots (if appropriate)

With this patch, we can import the below two types of date format notebooks.

screen shot 2016-06-22 at 12 18 01 pm

- Exported before #862 merged : `MMM dd, yyyy HH:mm:ss`

screen shot 2016-06-22 at 12 17 31 pm

### Questions: - Does the licenses files need update? No - Is there breaking changes for older versions? No - Does this needs documentation? No

@sixmen
Copy link
Contributor

sixmen commented Jun 21, 2016

Times in the web interface will show UTC times instead of local times if this patch is applied.

Before applying this patch,
2016-06-21 12 06 33

After applying this patch, (I'm at Seoul(+09:00))
2016-06-21 12 01 31

I think there are two ways to solve this problem.

First, export a date as natural format (like Jun 21, 2016 2:28:24 AM), and convert UTC to local in web interface.

( modify moment(pdata.dateUpdated).format('MMMM DD YYYY, h:mm:ss A') to moment.utc(pdata.dateUpdated).local().format('MMMM DD YYYY, h:mm:ss A') )

Second, export a date as ISO 8601, and modify importing code.

( modify zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java#importNode 's GsonBuilder to recognize ISO 8601 & natural format )

The second must be support two format (ISO 8601 for exported notebooks from new app and natural format for exported notebooks from old app), so I think first solution is simpler.
But I think standard format(ISO 8601) is preferable for interchange format.

@bzz
Copy link
Member

bzz commented Jun 21, 2016

Looks good to me, let's test and merge right after #1043

@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jun 21, 2016

@sixmen Really appreciate for sharing your feedback. I'm in PDT timezone, but I can't reproduce your situation. Here is my case.

  • Before applying this patch(latest master branch)

screen shot 2016-06-20 at 9 41 30 pm

In this case, we can't import the exported notebook because of the date format as we said.

  • After applying this patch

screen shot 2016-06-20 at 9 51 09 pm

I can see my local time in both cases. Am I missing sth ?

@AhyoungRyu
Copy link
Contributor Author

@bzz Actually I look into #1043, I found that this bug is not related with #1043. So I updated the description of this PR :)

@sixmen
Copy link
Contributor

sixmen commented Jun 21, 2016

I'm running the zeppelin server on my server and my server's timezone is UTC.

I think the problem is happened if PC's timezone is different from server's timezone.

@AhyoungRyu
Copy link
Contributor Author

@sixmen I see, thanks for sharing your problem. Let me check more then :)

@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jun 22, 2016

@sixmen I updated this patch as your second suggestion.
screen shot 2016-06-21 at 7 45 52 pm

My test server is in KST timezone. As you can see in the above screenshot, displayed elapse time show local time(PDT) as before. And we can also import the exported notebook :)

@sixmen
Copy link
Contributor

sixmen commented Jun 22, 2016

This patch is what I thought.
But it may fail to import an exported notebook from old version(e.x 0.5.6).
If you don't mind backward-compatibility, I think this patch is OK.
Otherwise, it may support two date formats.

@AhyoungRyu
Copy link
Contributor Author

@sixmen Good to hear that :)
IMO it would be better we support only unified date format from now on. And regarding

But it may fail to import an exported notebook from old version(e.x 0.5.6).

I can write down about this to https://zeppelin.apache.org/docs/0.6.0-SNAPSHOT/quickstart/explorezeppelinui.html#home-page so that ppl know. What do you think ?

@corneadoug
Copy link
Contributor

Having backward compatibility and transforming old notebooks into new format would be better.
Adding it to the doc would be a plus, however being sure that nobody end up with issues for now is better.

@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jun 22, 2016

@corneadoug Thank you for sharing your opinion and I agree with you. So I made it to support below two date formats for the backward compatibility.

screen shot 2016-06-22 at 12 18 01 pm

- Exported before #862 merged : `MMM dd, yyyy HH:mm:ss`

screen shot 2016-06-22 at 12 17 31 pm

Now we can import both exported .json notebooks which have the above date format.

@sixmen If possible, could you check the updated patch ? :)

@sixmen
Copy link
Contributor

sixmen commented Jun 23, 2016

It works well in my test.

@AhyoungRyu
Copy link
Contributor Author

@sixmen Yeah thanks for clarifying it.

@Leemoonsoo
Copy link
Member

Leemoonsoo commented Jun 23, 2016

LGTM and merge it into master and branch-0.6 if there're no more discussions.

asfgit pushed a commit that referenced this pull request Jun 24, 2016
### What is this PR for?

This bug seems to be produced by #862. Currently a exported notebook is not imported with below error message.

```
ERROR [2016-06-20 17:19:21,797] ({qtp559670971-14} NotebookServer.java[onMessage]:231) - Can't handle message
com.google.gson.JsonSyntaxException: 2016-06-20T14:33:31-0700
	at com.google.gson.internal.bind.DateTypeAdapter.deserializeToDate(DateTypeAdapter.java:81)
	at com.google.gson.internal.bind.DateTypeAdapter.read(DateTypeAdapter.java:66)
	at com.google.gson.internal.bind.DateTypeAdapter.read(DateTypeAdapter.java:41)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:93)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:172)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.read(TypeAdapterRuntimeTypeWrapper.java:40)
	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.read(CollectionTypeAdapterFactory.java:81)
	at com.google.gson.internal.bind.CollectionTypeAdapterFactory$Adapter.read(CollectionTypeAdapterFactory.java:60)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$1.read(ReflectiveTypeAdapterFactory.java:93)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:172)
	at com.google.gson.Gson.fromJson(Gson.java:791)
	at org.apache.zeppelin.notebook.Notebook.importNote(Notebook.java:199)
	at org.apache.zeppelin.socket.NotebookServer.importNote(NotebookServer.java:656)
	at org.apache.zeppelin.socket.NotebookServer.onMessage(NotebookServer.java:175)
	at org.apache.zeppelin.socket.NotebookSocket.onWebSocketText(NotebookSocket.java:56)
	at org.eclipse.jetty.websocket.common.events.JettyListenerEventDriver.onTextMessage(JettyListenerEventDriver.java:128)
	at org.eclipse.jetty.websocket.common.message.SimpleTextMessage.messageComplete(SimpleTextMessage.java:69)
	at org.eclipse.jetty.websocket.common.events.AbstractEventDriver.appendMessage(AbstractEventDriver.java:65)
	at org.eclipse.jetty.websocket.common.events.JettyListenerEventDriver.onTextFrame(JettyListenerEventDriver.java:122)
	at org.eclipse.jetty.websocket.common.events.AbstractEventDriver.incomingFrame(AbstractEventDriver.java:161)
	at org.eclipse.jetty.websocket.common.WebSocketSession.incomingFrame(WebSocketSession.java:309)
	at org.eclipse.jetty.websocket.common.extensions.ExtensionStack.incomingFrame(ExtensionStack.java:214)
	at org.eclipse.jetty.websocket.common.Parser.notifyFrame(Parser.java:220)
	at org.eclipse.jetty.websocket.common.Parser.parse(Parser.java:258)
	at org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.readParse(AbstractWebSocketConnection.java:632)
	at org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.onFillable(AbstractWebSocketConnection.java:480)
	at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
	at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.text.ParseException: Unparseable date: "2016-06-20T14:33:31-0700"
	at java.text.DateFormat.parse(DateFormat.java:366)
	at com.google.gson.internal.bind.DateTypeAdapter.deserializeToDate(DateTypeAdapter.java:79)
```

### What type of PR is it?
Bug Fix

### Todos

### What is the Jira issue?
[ZEPPELIN-1028](https://issues.apache.org/jira/browse/ZEPPELIN-1028)

### How should this be tested?
1. Apply this patch (Build the source and restart Zeppelin)
2. Export a notebook and try to import it again
3. It should be imported as before

### Screenshots (if appropriate)
With this patch, we can import the below two types of date format notebooks.
 -  Exported after #862 merged : `yyyy-MM-dd'T'HH:mm:ssZ`
<img width="289" alt="screen shot 2016-06-22 at 12 18 01 pm" src="https://cloud.githubusercontent.com/assets/10060731/16280468/6ebad710-3874-11e6-9ce6-b38e239649a2.png">

 - Exported before #862 merged : `MMM dd, yyyy HH:mm:ss`
<img width="288" alt="screen shot 2016-06-22 at 12 17 31 pm" src="https://cloud.githubusercontent.com/assets/10060731/16280459/6a4f256e-3874-11e6-848e-ec1ff6e9b441.png">

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: AhyoungRyu <fbdkdud93@hanmail.net>

Closes #1055 from AhyoungRyu/ZEPPELIN-1028 and squashes the following commits:

8f91d2d [AhyoungRyu] Fix class description
829bcae [AhyoungRyu] Rename NotebookImportSerializer -> NotebookImportDeserializer
2dff9bb [AhyoungRyu] Support two date format for backward compatibility
2d8fc66 [AhyoungRyu] Remove new line
7c493bf [AhyoungRyu] Change date format in importNote
479c0d0 [AhyoungRyu] Fix exported notebook importing error

(cherry picked from commit 57e0dc8)
Signed-off-by: Lee moon soo <moon@apache.org>
@asfgit asfgit closed this in 57e0dc8 Jun 24, 2016
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.

5 participants