Skip to content

[ZEPPELIN-2841] fix a problem in shell interpreter . Working directory '.' can not be found in docker environment - #2521

Closed
vistep wants to merge 2 commits into
apache:masterfrom
vistep:master
Closed

[ZEPPELIN-2841] fix a problem in shell interpreter . Working directory '.' can not be found in docker environment#2521
vistep wants to merge 2 commits into
apache:masterfrom
vistep:master

Conversation

@vistep

@vistepvistep commented Aug 8, 2017•
edited
Loading

Copy link
Copy Markdown

What is this PR for?

shell interpreter complained that working directory '.' can not be found in docker environment.
I add a line of code to set current working directory to USER`s home, and it works.

What type of PR is it?

Bug Fix

Todos

  • tests

What is the Jira issue?

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

How should this be tested?

run shell interpreter`s test units

Screenshots (if appropriate)

Questions:

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

@vistepvistep changed the title [ZEPPELIN-2841] fix a problem in shell interpreter . Working directory '.' can not be found in docker envi[ZEPPELIN-2841] fix a problem in shell interpreter . Working directory '.' can not be found in docker environmentAug 8, 2017
@vistep

vistep commented Aug 9, 2017

Copy link
Copy Markdown
Author

Jenkins said that "Can't find build for commit e8cbdd1 from vistep".
I have no idea about what happened. How to fix it?

felixcheung
felixcheung reviewed Aug 9, 2017
View reviewed changes

@felixcheungfelixcheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please see
https://zeppelin.apache.org/contribution/contributions.html#continuous-integration

Looks like travis-ci is not configured for your fork.
Please setup by swich on 'zeppelin' repository at https://travis-ci.org/profile and travis-ci.
And then make sure 'Build branch updates' option is enabled in the settings https://travis-ci.org/SachinJanani/zeppelin/settings.
To trigger CI after setup, you will need ammend your last commit with
git commit --amend
git push your-remote HEAD --force
See http://zeppelin.apache.org/contribution/contributions.html#continuous-integration.

Comment threadshell/src/main/java/org/apache/zeppelin/shell/ShellInterpreter.java Outdated
contextInterpreter.out, contextInterpreter.out));
executor.setWatchdog(new ExecuteWatchdog(Long.valueOf(getProperty(TIMEOUT_PROPERTY))));
executors.put(contextInterpreter.getParagraphId(), executor);
executor.setWorkingDirectory(new File(System.getenv("HOME")));

@felixcheungfelixcheungAug 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this won't work in windows though (well)

@vistepvistepAug 9, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks so much, I will try to figure it out.

@vistep

vistep commented Aug 9, 2017

Copy link
Copy Markdown
Author

I replaced System.getenv("HOME") with System.getProperty("user.home") .
This time, Jenkins said "[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.3:yarn (yarn test) on project zeppelin-web: Failed to run task: 'yarn run test' failed. (error code 1) -> [Help 1]"
I don`t understand why my code affects zeppelin-web ?

@Leemoonsoo

Leemoonsoo commented Aug 9, 2017

Copy link
Copy Markdown
Member

build sometimes fail because of network connection problem of flaky tests.
Could you try restart failed one?

Shu Jiaming
ZEPPELIN-2841fix a bug where shell interpreter complained that worki…
34a0049
…ng directory '.' can not be found while zeppelin was running in docker enviroment.
@vistep

vistep commented Aug 10, 2017

Copy link
Copy Markdown
Author

I tried again and it passed.

@Leemoonsoo

Leemoonsoo commented Aug 10, 2017

Copy link
Copy Markdown
Member

LGTM and merge to master and branch-0.7 if no further comment.

asfgit pushed a commit that referenced this pull request Aug 11, 2017
Shu Jiaming@Leemoonsoo
[ZEPPELIN-2841] fix a problem in shell interpreter . Working director…
5f30854
…y '.' can not be found in docker environment
### What is this PR for?
shell interpreter complained that working directory '.' can not be found in docker environment.
I add a line of code to set current working directory to USER`s home, and it works.
### What type of PR is it?
Bug Fix
### Todos
* tests
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-2841
### How should this be tested?
run shell interpreter`s test units
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update?
No
* Is there breaking changes for older versions?
No
* Does this needs documentation?
No
Author: Shu Jiaming <jiaming.shu@transwarp.io>
Author: 束佳明 <shujiaming@hotmail.com>
Closes#2521 from vistep/master and squashes the following commits:
34a0049 [Shu Jiaming] ZEPPELIN-2841 fix a bug where shell interpreter complained that working directory '.' can not be found while zeppelin was running in docker enviroment.
d02104a [束佳明] Merge pull request #1 from apache/master
(cherry picked from commit 71d1305)
Signed-off-by: Lee moon soo <moon@apache.org>
@sanjaydasgupta

sanjaydasgupta commented Sep 5, 2017

Copy link
Copy Markdown
Contributor

This implementation has a harmful side effect that can break existing notebooks.

The problem with this fix is that it changes the meaning of all relative path-names used in any shell commands. For example, after this fix, the command ls -l data would operate on the data subdirectory of the user's home directory, whereas before the fix it would operate on the data subdirectory of zeppelin's working directory.

One immediate change that is suggested is to change the fix to make it apply only to the narrow situation it is needed in (a Docker environment).

@vistep

vistep commented Sep 5, 2017

Copy link
Copy Markdown
Author

Can we use a try-catch block to test if we need to set working directory when zeppelin opens a shell interpreter?

@sanjaydasgupta

sanjaydasgupta commented Sep 5, 2017

Copy link
Copy Markdown
Contributor

I have not looked at the code in detail, so will refrain from making suggestions about the code :-)

@vistep

vistep commented Sep 5, 2017

Copy link
Copy Markdown
Author

@Leemoonsoo How about the idea above?

@prabhjyotsingh

prabhjyotsingh commented Sep 5, 2017

Copy link
Copy Markdown
Contributor

@sanjaydasgupta@vistep have create JIRA for the same https://issues.apache.org/jira/browse/ZEPPELIN-2903.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixcheungfelixcheungfelixcheung left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@vistep@Leemoonsoo@sanjaydasgupta@prabhjyotsingh@felixcheung