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

Adds shared volume #6

Merged
merged 2 commits into from
Dec 23, 2016
Merged

Adds shared volume #6

merged 2 commits into from
Dec 23, 2016

Conversation

JJ
Copy link
Contributor

@JJ JJ commented Dec 23, 2016

And documentation on how to use it.

These are very minor changes but add to usability.

And documentation on how to use it.
@JJ
Copy link
Contributor Author

JJ commented Dec 23, 2016

(I know I have the privs to merge this, but maybe someone else wants to check it out)

Copy link

@alexm alexm left a comment

Choose a reason for hiding this comment

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

I tried this on an older docker image and it works fine:

docker run -t -v `pwd`:/home/perl6 rakudo-star perl6 /home/perl6/ls.p6

Is the volume really needed?


$ docker run -t -v `pwd`:/home/perl6/app rakudo-star /home/perl6/app/ls.p6

where we mal the local file ls.p6 (in this directory) to the remote
Copy link

Choose a reason for hiding this comment

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

What does mal mean here? It sounds like a typo, but I may be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally typo. Should be map. Will fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Volume is not really needed. It's just a convenient place to put scripts. You can create new volumes with -v all the time.

And adding a bit of more context.
@moritz moritz merged commit 7724855 into Raku:master Dec 23, 2016
@tianon
Copy link
Contributor

tianon commented Jan 30, 2017

As I noted over in docker-library/official-images#2599, I'm having a hard time understanding what's added by having an explicit VOLUME instruction in the Dockerfile here -- if -v is used on the docker run line, a volume or bind mount is created already, and no pre-defined VOLUME is actually necessary.

@JJ
Copy link
Contributor Author

JJ commented Jan 31, 2017

No, not really. It's just a convention,

@tianon
Copy link
Contributor

tianon commented Feb 6, 2017

I think the description in docker-library/official-images#2437 (comment) is decent context on why this VOLUME doesn't seem terribly appropriate (and could actually be actively harmful to users, while the absence of the explicit VOLUME doesn't prevent anything that's newly documented here):

The general rule we follow is that you should have a volume for where the application stores data (so $SONARQUBE_HOME/data is uncontestably appropriate), but configuration is almost never appropriate in a volume (unless it's specifically managed by the application itself at runtime and never supplied/managed manually by users, or there is an easy way to override the default in the image).

The reason for this is that, once defined, Docker has no means of "UNVOLUME" -- thus, once a volume, always a volume.

hoelzro added a commit that referenced this pull request Feb 6, 2017
This reverts commit 7724855, reversing
changes made to a7548f1.

Explanation:
    #7
    #6 (comment)
@JJ
Copy link
Contributor Author

JJ commented Feb 6, 2017 via email

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.

4 participants