Skip to content

[ZEPPELIN-923] Apply new mechanism to ShellInterpreter - #956

Closed
AhyoungRyu wants to merge 3 commits into
apache:masterfrom
AhyoungRyu:ZEPPELIN-923
Closed

[ZEPPELIN-923] Apply new mechanism to ShellInterpreter#956
AhyoungRyu wants to merge 3 commits into
apache:masterfrom
AhyoungRyu:ZEPPELIN-923

Conversation

@AhyoungRyu

Copy link
Copy Markdown
Contributor

What is this PR for?

This PR applies the new interpreter registration mechanism to Shell interpreter.

What type of PR is it?

Improvement

Todos

  • - Remove static property definition code lines in shellInterpreter.java and add interpreter-setting.json under the shell/src/main/resources

What is the Jira issue?

ZEPPELIN-923

How should this be tested?

  1. apply patch
  2. rm -r interpreter/sh and conf/interpreter.json
  3. build source
  4. bin/zeppelin-daemon.sh start
  5. Run some shell command

Screenshots (if appropriate)

Questions:

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

@jongyoul

Copy link
Copy Markdown
Member

😄

@AhyoungRyu

Copy link
Copy Markdown
ContributorAuthor

@jongyoul But CI is failed :( Is this relevant with this PR ?

@jongyoul

Copy link
Copy Markdown
Member

@AhyoungRyu One looks irrelevant and another is about rat plugin. you need to exclude interpreter-setting.json in a rat plugin.

@AhyoungRyu

Copy link
Copy Markdown
ContributorAuthor

@jongyoul I see. Okay Thanks for letting me know :)

@prabhjyotsingh

Copy link
Copy Markdown
Contributor

@AhyoungRyu have made a fix #4492.7 failed #958. Can you rebase with master, and re-trigger ?

And yes other is [INFO] Rat check: Summary of files. Unapproved: 1 unknown: 1 generated: 0 approved: 2 licence.

@AhyoungRyu

Copy link
Copy Markdown
ContributorAuthor

@jongyoul@prabhjyotsingh I excluded interpreter-setting.json and also rebased from master.

@prabhjyotsingh

Copy link
Copy Markdown
Contributor

LGTM.

@prabhjyotsingh

Copy link
Copy Markdown
Contributor

Merging this if no more discussion.

@asfgitasfgit closed this in 486ed23Jun 7, 2016
@bzzbzz mentioned this pull request Jun 30, 2016
1 task
Sign up for freeto 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

@AhyoungRyu@jongyoul@prabhjyotsingh