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

Feature/jupyter docker #1019

Merged
merged 2 commits into from
Jan 5, 2017
Merged

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Dec 27, 2016

Add jupyter notebook to docker image.

@@ -43,5 +43,7 @@ cp -rv /woboq/data $WOBOQ_OUT/../data
-o $WOBOQ_OUT \
-p paddle:/paddle
/woboq/indexgenerator/codebrowser_indexgenerator $WOBOQ_OUT

cd /woboq
make clean
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

make clean will shrink docker image size.

Copy link
Member

Choose a reason for hiding this comment

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

I want to build a docker env to test this commit, this may take some time, will this block you?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope.

@@ -0,0 +1,27 @@
# from https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/docker/jupyter_notebook_config.py
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this file comes from tensorflow

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

赞这个PR。我的建议都是以简化为宜。


RUN mkdir -p /opt/run
COPY ./paddle/scripts/docker/jupyter_notebook_config.py /root/.jupyter/
COPY ./paddle/scripts/docker/00_sshd /opt/run/
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里命名成00,01的脚本,貌似是为了按照名字alphabetical order来执行。但是下面 run_all脚本的写法不能保证每次都按照同样的顺序来执行。如果要保证按照字母序,可以参考 http://stackoverflow.com/questions/7992689/bash-how-to-loop-all-files-in-sorted-order

不过我觉得至少现在不需要这么弄把,可以把 00_sshd 和 01_jupyter 写进 run_all 里,顺便在调用一下 jypyter_notebook_config.py 就好了把。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

from IPython.lib import passwd

c.NotebookApp.ip = '*'
c.NotebookApp.port = int(os.getenv('PORT', 8888))
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里默认port改改,就不需要标注从Tensorflow来了吧。这个文件的内容都是基于ipython lib的,ipython和jupyter用户都会写,不见得非要受tensorflow启发?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Also use git rebase purge this config file in git history.

I'm building an image base on this commit and test it.

@reyoung reyoung force-pushed the feature/jupyter_docker branch from 3e16601 to d9dbfe4 Compare January 3, 2017 05:36
@reyoung reyoung force-pushed the feature/jupyter_docker branch from d9dbfe4 to 68c89bc Compare January 3, 2017 05:37
@wangkuiyi
Copy link
Collaborator

一个建议: docker/run_all ==> docker/entrypoint

另外,我没有验证docker buld,请 @jacquesqiao 验证再merge吧。谢谢啦!

@reyoung
Copy link
Collaborator Author

reyoung commented Jan 4, 2017

docker/run_all ==> docker/entrypoint

有道理,done

@reyoung reyoung merged commit 533652e into PaddlePaddle:develop Jan 5, 2017
@reyoung reyoung deleted the feature/jupyter_docker branch January 5, 2017 07:50
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024
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.

3 participants