Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ZEPPELIN-2758]. NumberFormatException on importing notebook #2485

Closed
wants to merge 2 commits into from

Conversation

zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Jul 12, 2017

What is this PR for?

This is due to the custom TypeAdapter for Paragraph. I don't know why we introduce it. To me it is not necessary. So I remove this in this PR. And the spark UI still can be accessed in frontend.

What type of PR is it?

[Bug Fix]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Unit test is updated 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

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 12, 2017

@karuppayya I remove NotebookTypeAdapterFactory.java which I think is not necessary. Let me know your concern.

@necosta
Copy link
Contributor

necosta commented Jul 12, 2017

LGTM

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 14, 2017

Will merge it if no more comments.

@karuppayya
Copy link
Contributor

@zjffdu the NotebookTypeAdapterFactory was added to avoid runtimeInfos being persisted in json. (These infos are valid only during the runtime of the Zeppelin server, interpreter)
How are we handling this now?

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 14, 2017

Thanks @karuppayya So do you mean Note will be serialized to different formats for runtime serialized to frontend and serialized to file ? What is the side effect if I store it in file ?

@karuppayya
Copy link
Contributor

Currently only the frontend receives this info.
If we remove the AdapterFactory, The runtimeInfos will be serialised to json file.
Say runtimeinfos contain spark UI link.
if it is persisted in json, when Zeppelin server restarts, we will see a invalid spark UI link(we can clean the runtimeinfos during zep server initialization though), which we might not want

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 14, 2017

Thanks @karuppayya for explaining. How about reset the runtimeInfos to null when deserialized from json (it happens when it is read from file). Serializing to different formats in different scenarios seems a little complicated to me

@karuppayya
Copy link
Contributor

Yeah , we can reset the infos when the json is deserialized.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 15, 2017

Thanks @karuppayya push another commit to reset the runtimeInfo when deseriliazed from json.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 18, 2017

Will merge it if no more comments.

@prabhjyotsingh
Copy link
Contributor

LGTM!

@asfgit asfgit closed this in 3dd25c2 Jul 19, 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
4 participants