SebKuzminsky / pycam

Other
342 stars 101 forks source link

Docker image #62

Closed laconc closed 7 years ago

laconc commented 7 years ago

The docker image itself lives here: https://hub.docker.com/r/pycam/pycam. I manually built the version currently up there but we could set it to auto-build new versions as releases go out. That's work for a future PR.

Let me know who I need to hand the keys over to for the Docker Hub organization. I believe all I need is a Docker ID to do so.

I updated the INSTALL document with what I believe to be the current install methods. I did test what's on there but I would love it if someone else would double-check. I stripped the Windows section since it doesn't currently work. I have limited experience with Windows so I'm passing the buck to the next guy.

sumpfralle commented 7 years ago

Thank you!

I am not used to docker, thus I cannot really comment on this technically. But it looks quite concise - thus I assume, there is probably not so much to discuss about technical details.

Would you mind switching to Debian as a base image? Personally I would prefer the base distribution instead of picking one of its derivatives.

laconc commented 7 years ago

That was a great suggestion @sumpfralle, I've gone ahead and changed the base image to Debian.

I'll work on providing some documentation early next week.

sumpfralle commented 7 years ago

Thank you!

Should I merge your above current commits right now or should I wait for your documentation (which is very welcome)?

laconc commented 7 years ago

I would prefer if you waited - I don't think many people would get a lot of use out of it as is. I plan to have it by Tuesday.

matelakat commented 7 years ago

What about moving Dockerfile to a docker directory, and using volumes instead of copying in the full source? (see also the last section of my comment).

I couldn't start pycam with this. In the Issue above, you were using the command:

docker run -it \
    -v /tmp/.X11-unix:/tmp/.X11-unix \
    -e DISPLAY=<some-ip>:0 \
    pycam

To start the container. You also did some socat magic. What was the source of <some-ip> (I guess that was referring to the host system?) I'm not fully getting the purpose of those. Reading some topics we might be able to run X applications by just forwarding the X sockets (which is the volume part) or using --host net. Maybe the socat magic was required because you were on Mac? (I am on Linux)

Could you shed some light on those?

It's also not clear what we want to achieve with the change:

laconc commented 7 years ago

Thanks for giving it a try @matelakat! You're correct that the socat stuff is a Mac thing. I just added some documentation and for Linux (at least for Ubuntu) it should be:

sudo docker run -it \
    -v ~/Documents:/root/Documents \
    -v ~/.Xauthority:/root/.Xauthority \
    -e DISPLAY \
    --net=host \
    pycam

Let me know if you still experience issues.

From the research that I did, Mac support has always been an issue and I thought this could be a neat way for people to get access when they're struggling otherwise. As for setting up an interactive development environment: that sounds awesome. I have personally not done that but I'm willing to spend some time on it for a future PR.

@sumpfralle I'm comfortable with a merge now if you feel it's good to go. Also, take a look at my update in the first comment.

sumpfralle commented 7 years ago

Thank you both for discussing the details!

Regarding the IP: at least on a linux system you can use DISPLAY=:0 without an IP. Wouldn't this be sufficient for MacOS, as well? Or would at least DISPLAY=127.0.0.1:0 work?

Regarding docker pull pycam/pycam: does this mean that someone needs to keep an external image up-to-date? How does that work?

@matelakat: since you are using (and probably understanding) docker a bit: do you have any more comments? Otherwise I will merge it now.

laconc commented 7 years ago

I had tried using localhost but it failed. The best explanation that I could find as to why was:

docker -e DISPLAY=$IP:0 should instruct X clients in docker to look for an X server on $IP:6000, which socat will then forward to the XQuartz socket.

An external image will need to be kept up-to-date, although we can have it built automatically. There's a couple of ways of doing that, including through Travis or Docker Hub itself. Even doing it manually only requires a quick docker build, then setting the tags, and a docker push. Luckily, that's all very simple.

I would rather not be the only one with access to the repository for the docker image. I can add anyone as an owner who should have that access.

sumpfralle commented 7 years ago

Please excuse my lack of knowledge regarding docker. Maybe you can help:

Or did I misunderstand your wording?

laconc commented 7 years ago

You're right, the Dockerfile is the recipe, and a user can absolutely just build it themselves with something like docker build -t pycam .. I wouldn't consider a binary necessary for most purposes. The advantages are about the same as having versioned releases; a user would be able to pull a previous version by changing a tag: docker pull pycam/pycam:0.6.1. For it to be worth it, it really needs to be automated, more info on that. The ideal solution would be to auto build whenever there's a release.

If the plan is to eschew from docker binaries altogether, than that's a minor change from the user's perspective; they would just need to build the image instead of pulling one. They will still be downloading the base image (Debian, in this case) during the build process, though.

sumpfralle commented 7 years ago

Ah - ok. Now I understand. Thank you!

Personally I would prefer to distribute pycam as source only, if possible. I do not want to carry the burden of being responsible for potentially outdated binaries. If you want to take care for this (or automate it), this is fine. Otherwise I would suggest to switch to the source image model. Would this need documentation changes?

laconc commented 7 years ago

Since you've given me the option, I would love an opportunity to automate it. This PR can be merged as is and I can perform the automation work within the week.

sumpfralle commented 7 years ago

Great!