Skip to content

[HOTFIX] ZEPPELIN-931: fix interpreter listing bug#945

Closed
AhyoungRyu wants to merge 5 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-931
Closed

[HOTFIX] ZEPPELIN-931: fix interpreter listing bug#945
AhyoungRyu wants to merge 5 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-931

Conversation

@AhyoungRyu
Copy link
Copy Markdown
Contributor

@AhyoungRyu AhyoungRyu commented Jun 1, 2016

What is this PR for?

Currently available interpreter list is not shown in Creating New Interpreter section. It seems this bug was generated after #835 was merged. So I temporally deactivated 3 SerializedName code lines.

What type of PR is it?

Bug Fix

Todos

  • - Fix interpreter listing bug when creating new interpreter

What is the Jira issue?

ZEPPELIN-931

How should this be tested?

  1. Build latest master branch and browse Zeppelin home
  2. Create new interpreter -> You can not see the available interpreter list in this step like below attached screenshot
  3. Apply this patch
  4. Build again and browse -> You can see the available interpreter list as normal

Screenshots (if appropriate)

  • Before
    screen shot 2016-06-01 at 12 36 42 pm
  • After
    screen shot 2016-06-01 at 12 33 06 pm

Questions:

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

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

AhyoungRyu commented Jun 1, 2016

@jongyoul I saw you are working on applying new mechanism to all of the existing interpreters in ZEPPELIN-804 as subtasks. I think this bug was caused since interpreterGroup, interpreterName and interpreterClassName that I deactivated were not matched with group in here. So if you have a better way to solve this bug, just let me know :)

@AhyoungRyu AhyoungRyu closed this Jun 2, 2016
@AhyoungRyu AhyoungRyu reopened this Jun 2, 2016
@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

@AhyoungRyu I've tested and reproduced what you mentioned. Your change looks simple but it's not perfect because you cannot see spark interpreter in your interpreter list because interpreter-setting.json read interpreterGroup, interpreterName and interpreterClassName while initialisation. I'll submit a PR into your branch. Thanks for reporting this.

@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

I don't know whether front-end uses RegisteredInterprerter. We need to find a way to indicate or mark which class is used in front and backend.

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul I see. That's why I can't see Spark interpreter in the list. Thanks ! Okay i'll check ASAP.

@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

@AhyoungRyu Could you please add "[HOTFIX]" in a title?

@AhyoungRyu AhyoungRyu changed the title ZEPPELIN-931: fix interpreter listing bug [HOTFIX] ZEPPELIN-931: fix interpreter listing bug Jun 2, 2016
@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul Yeah I just added :)

Fixed fieldName in interpreter-setting.json
@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul I applied your patch and it works :)

@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

@AhyoungRyu I've missed something. I submitted a PR, again.

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul That was also applied.

@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

@AhyoungRyu Thanks.

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul Let's wait the travis then :)

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@Leemoonsoo CI has been passed. Could you check this one ? :)

@jongyoul
Copy link
Copy Markdown
Member

jongyoul commented Jun 2, 2016

LGTM. I'll merge it asap.

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@jongyoul Yeah sounds good.

@asfgit asfgit closed this in 7c50cac Jun 2, 2016
@AhyoungRyu AhyoungRyu deleted the ZEPPELIN-931 branch June 6, 2016 23:21
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.

2 participants