Skip to content

Docker README Fixes #554

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Docker README Fixes #554

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ If you have a [Docker daemon running](https://docs.docker.com/installation/), e.
$ docker run --rm -it -p 8888:8888 -v "$(pwd):/notebooks" jupyter/notebook

In your browser open the URL `http://localhost:8888/`.
All notebooks from your session will be saved in the current directory.
All notebooks from your session will be saved in the current directory. In reality, they are save inside the container at `/notebook`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be rephrased to make clear that the data volume will be created internally and persist(!) if not overridden like in the example.

Copy link
Member Author

Choose a reason for hiding this comment

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

How do you mean persist here? Using --rm means the container will be cleaned up on exit.

Copy link
Contributor

Choose a reason for hiding this comment

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

your example may be well for demonstration purposes. but it's hardly useful for daily usage.

my point is: this is a documentation of the Docker image. that image includes a volume declaration. for non-demonstration-usages it is essential to mention this volume explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

All notebooks from your session will be saved in the current directory. In reality, they are save inside the container at /notebook.

in the case of the example this is not true and a contradiction.

Copy link
Member Author

Choose a reason for hiding this comment

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

your example may be well for demonstration purposes. but it's hardly useful for daily usage.

I respectfully disagree. There are quite a few dependencies required by jupyter. Getting a nice working copy can sometimes be tricky. Being able to startup a docker container instead of debugging the build/install process is a huge asset. Not having to understand all of the ends and outs of docker in order to get jupyter up and running makes this worthwhile for the average end user.

If your point here is that docker is normally used on cluster deployments and the like, then I think that user will know enough about docker that they will have no problem changing these options to fit their needs.

my point is: this is a documentation of the Docker image. that image includes a volume declaration. for non-demonstration-usages it is essential to mention this volume explicitly.

Which it now is.

All notebooks from your session will be saved in the current directory. In reality, they are save inside the container at /notebook.
in the case of the example this is not true and a contradiction.

Not true. They are saved in that directory. While the container is up and running one could still find these using docker exec. The container is merely blown away on exit instead of merely being stopped.


I have tried to add a line to address the concern you have raised. If you think I have not done so, please be explicit about what you would like to see instead. I would like this to be resolved in a manner that we can both find satisfactory soon.

These are then synced to current working directory.

On other platforms without `docker`, this can be started using `docker-machine`
by replacing `localhost` with an IP from [`docker-machine ip <MACHINE>`](https://docs.docker.com/machine/reference/ip/).
Expand Down