added Dockerfile; URL changed from `localhost` to `0.0.0.0`

[ozzzzz]

Hi there! Thank you for the great tool! Main motivation in changing URL from localhost to 0.0.0.0 is to make it possible to run in docker-container and receive requests not only from localhost.

If you want, I can help with publishing on Docker hub. After that it will be possible to run your tool as a service with only one command (as example):

docker run -it -p 8000:8000 pandegroup/pdbfixer

[peastman]

Thanks! Can you explain the logic behind this change? localhost is a synonym for 127.0.0.1. The address 0.0.0.0 means something very different (see https://en.wikipedia.org/wiki/0.0.0.0) and is used to indicate an unknown or invalid target. Isn't that going to break it?

I'm not too familiar with packaging apps as docker containers. Can anyone else comment on that part? Is this a useful feature, and does the implementation look ok?

[ozzzzz]

Host 0.0.0.0 is used for the servers to listen requests not only from the localhost, by from any host. Here is quote from the page that you mentioned:

A way to specify "any IPv4 address at all". It is used in this way when configuring servers (i.e. when binding listening sockets). This is known to TCP programmers as INADDR_ANY. (bind(2) binds to addresses, not interfaces.)

This edit is needed for the docker-container. Inside docker container request is going not from the localhost, but from docker host. But if you running your application on one server and request it from another, this edit is needed too.

I tried to run server in docker and without docker, it works.

[peastman]

I think I partly understand. So in the line

server = _ThreadingHTTPServer(("0.0.0.0", 8000), _Handler)

you're telling it to accept connections from any address? The documentation at https://docs.python.org/3/library/http.server.html shows it passing '', which perhaps is another way of doing the same thing.

But what about these lines:

url = 'http://0.0.0.0:'+str(uiserver.server.server_address[1])
print("PDBFixer running: %s " % url)
webbrowser.open(url)

If you direct the web browser to 0.0.0.0, how does it know where to actually go?

[ozzzzz]

I see your doubt and it's reasonable. Some browsers perceive 0.0.0.0 as localhost (for example, Google Chrome do that).

I replaced from 0.0.0.0 to localhost back in ui.py.

[peastman]

Ok, that makes more sense.

Does anyone have comments on the Docker packaging?

[ozzzzz]

Anybody here?)

[jchodera]

@lnaden may have some input on the docker packaging.

[peastman]

Sorry about that! This fell off my radar over the holidays. I don't know enough to evaluate the Docker packaging, so I'll see if @Lnaden has any comments about it. If not I'll go ahead and merge this.

[Lnaden]

The Dockerfile you have will work okay, just a bit bloated. If you wanted to just make a docker image available instead of having the user have to run the docker build command themselves, you could have Travis do the build on after_success and/or in the after_sucess.sh script, and then push the image/tag to Docker Hub.

I can give you some ideas on how to reduce the size of the image if you want.

[Lnaden]

The other recommendation I would have is to change the source of the git clone since in its current form, it will always get the master branch of the Git, which may not be what users want. And, until this is merged, trying to port bind through docker will fail since the HTTP server inside the Docker image will never bind to anything other than communications from inside itself, so the Docker port pass throughs will not work.

[ozzzzz]

I can give you some ideas on how to reduce the size of the image if you want.

Yes, please!

[Lnaden]

To reduce image sizes in docker in general, you basically have to overload every directive to do as much as you can in each step because of how Docker builds its layers. Here is a sample Dockerfile you can use (with edits):

FROM continuumio/miniconda3

RUN conda update -n base -c defaults conda && \
    conda install -n base -c omnia -c conda-forge openmm -y && \
    conda clean --yes --all

RUN git clone https://github.com/ozzzzz/pdbfixer.git && \
    cd /pdbfixer && \
    python setup.py install && \
    cd / && \
    rm -rf /pdbfixer

ENTRYPOINT pdbfixer

In this case, we use the first RUN directive to do all the conda stuff in one layer, then we use the second RUN directive to do all the cloning and installing. In each one of the steps, I cleanup what was installed to minimize how much is sticking around. This particular one reduces the image size from 774 MB down to 631 MB which will compress down on Dockerhub if you upload it there.

I do want to note that the Dockerfile you specify with the PR points to the PDBFixer GitHub HEAD, which may not be what users want. The example I have here points to your fork/branch so I can test it properly. It may be better to point to specific release tarballs so people get the releases rather than developmental heads.

[ozzzzz]

To reduce image sizes in docker in general, you basically have to overload every directive to do as much as you can in each step because of how Docker builds its layers. Here is a sample Dockerfile you can use (with edits):

FROM continuumio/miniconda3

RUN conda update -n base -c defaults conda && \
    conda install -n base -c omnia -c conda-forge openmm -y && \
    conda clean --yes --all

RUN git clone https://github.com/ozzzzz/pdbfixer.git && \
    cd /pdbfixer && \
    python setup.py install && \
    cd / && \
    rm -rf /pdbfixer

ENTRYPOINT pdbfixer

In this case, we use the first RUN directive to do all the conda stuff in one layer, then we use the second RUN directive to do all the cloning and installing. In each one of the steps, I cleanup what was installed to minimize how much is sticking around. This particular one reduces the image size from 774 MB down to 631 MB which will compress down on Dockerhub if you upload it there.

I do want to note that the Dockerfile you specify with the PR points to the PDBFixer GitHub HEAD, which may not be what users want. The example I have here points to your fork/branch so I can test it properly. It may be better to point to specific release tarballs so people get the releases rather than developmental heads.

I update version :).