Skip to content
This repository has been archived by the owner on Feb 19, 2021. It is now read-only.

Docker #28

Closed
wants to merge 4 commits into from
Closed

Docker #28

wants to merge 4 commits into from

Conversation

TheConnMan
Copy link

NOTE: Currently a work in progress, the document_consumer is not running and the appropriate Docker volumes have not been added.

Ref #2.

@danielquinn: I'm getting the following error from the PIL module when trying to run python manage.py document_consumer:

  ...
  File "/usr/local/lib/python3.4/dist-packages/PIL/Image.py", line 66, in <module>
    from PIL import _imaging as core
ImportError: cannot import name _imaging

Does that look familiar or do you have anything else I could try? I had to mess around with the PYTHONPATH because Docker wasn't picking up the default folders.

@danielquinn
Copy link
Collaborator

That error looks to me like Pillow was compiled on a system that doesn't have ghostscript or the -dev libraries required to handle images. If you're running Ubuntu or Debian, try making sure that you apt-get install ghostscript libpdf-dev (or something like that) as well as libjpeg-dev, etc. Then uninstall Pillow and re-install it, at which point it should compile against the right libraries and support what you need.

@danielquinn
Copy link
Collaborator

You should probably collaborate with @pitkley, since he's got some skin in this game as well: #2 (comment)

@TheConnMan
Copy link
Author

Yeah, I saw @pitkley's Docker repo right after I added the pull request. I'm fine deferring to his if he wants to add it to this repo instead of keeping it separate.

@danielquinn
Copy link
Collaborator

Alright then. I'm afraid I don't know much about Docker, so adoption of this PR will take some time and experimentation on my part as I want to understand whatever it is I'm merging :-)

Like you, I'd prefer that the Dockerfile (and any required files) be part of this repo (once we're settled on how it should look/work). I just think it's kind of silly to see the both of you doing much the same thing, so I wanted to point you both toward each other. I'm going to mention this PR in the relevant issue to see if the folks there have any input.

Unfortunately, I can't merge it into master until we know it works as expected though.

@pitkley
Copy link
Member

pitkley commented Feb 13, 2016

Great to see another Dockerfile for Paperless. A few points, which might be biased:

  1. Starting from ubuntu and needing to fiddle around with PYTHONPATH is not worth it in my eyes, given that there are official python images.

    The one I used -- the default one -- is based on buildpack-deps which conveniently already contains most needed dev-packages to be able to build and run Paperless and its requirements.

  2. I wouldn't use the vagrant-provision script. It creates a dependency on something that might be for a completely -- or worse, subtly different environment which might cause issues that could be hard to track.

  3. I am torn on the idea of creating the superuser as part of the build-process.

    On the one hand, it is super handy to just get it going, no worries about executing anything. On the other hand though, the user will most likely not be building this image himself but rather pulling it prebuilt from the Docker Hub. This seems like a security issue when every image has the same superuser built in.

  4. I see that the consumer-part is still WIP. In my case, I have generalized the image to the point that I can decide on startup if I want it to act as a consumer or as a webserver.

    I don't know if this is the right way to go, but if you try to run both the consumer and the webserver in the same container, make sure to think about the reaping problem. One might have to use something like s6-overlay.

Generally I think that supplying an official Dockerfile with Paperless is a good thing we should work towards. Right now though, I am not comfortable enough to create a PR for my image. I have not done enough testing and I would like some responses from other people using it first.

Additionally, given how fast Paperless probably will be evolving, given e.g. the discussion in issue #3 about moving to Flask, an official image might be to early and people might expect to be able to easily migrate, which might not be the case.

@TheConnMan: When you have completed your Dockerfile -- if you intend to go further with it -- let me know, I'll gladly test it and see if we can "extract" the best of both.

For the time being, adding a notice to the README directing people towards issue #2, this PR and existing Docker images might also be something to think about!

@TheConnMan
Copy link
Author

Your points on implementation of the Dockerfile are all solid, I agree that the approach you took in using the Python image and splitting up into multiple containers is the right way to go. I would have had to background one of the processes in a single container which wouldn't have been good practice. Not using the Vagrant script is for the best as well. As for the default user, I agree it's a security concern. I add the default user at runtime of the container (which would be runtime of your webapp container) so the username and password could be set with environment variables.

Below are a few other points:

  • I understand your hesitation to add a Dockerfile to the repo (especially if the project will move to Flask), but I wanted to add a Dockerfile as soon as possible because it will allow people to get up and running very quickly. I've already seen multiple comments about screenshots or demos so I wouldn't want the project to miss out on users because of start-up hurdles. That being said, I'll defer to @danielquinn's opinion because it is his repo.
  • I personally like having a COPY or ADD command to add the working directory into the build instead of cloning the repo. I want to be able to make changes locally and run them in a container because honestly use Docker so I don't have to download a ton of Python stuff onto my machine. That's not possible with your files in another repo so I didn't know if you were planning on doing that anyways.

I don't have any plans of updating my Dockerfile and am happy to help you with yours as long as we can merge it into this repo so I can use it!

@pitkley
Copy link
Member

pitkley commented Feb 14, 2016

Oh, I'm sorry, I missed that you create the user within the CMD and not as a RUN. While that indeed makes the user configurable at runtime, it will create an additional user every time the container is started (see models.py) if the database is kept across container creations -- which is probably not what the enduser would expect to happen.

I agree with you on using COPY or ADD, but it just didn't make sense for my external Dockerfile. If we include the Dockerfile with the repository, adding the local source is definitely the way to go -- no need in cloning the repository again if it is already there and up-to-date!

Thanks for offering your help, one big help would be if you could try out my Docker image and see if you can make it work. If any problems come up or things are unclear, maybe create an issue over at my repository for the time being.

@TheConnMan
Copy link
Author

Sounds good. I actually ran into the duplicate user issue when restarting a container yesterday. I bet we could make an external "Create User" Python script which checks if the user already exists before creating it.

I'll take another look at your repo today and add any issues I find. For now I'll keep this pull request open so people can find the discussion. I'll close it when you make yours.

@pitkley
Copy link
Member

pitkley commented Feb 17, 2016

Just as an FYI, I have opened and "finished" my PR #39 which (probably) replaces this one.

@danielquinn
Copy link
Collaborator

Hi guys, as you may have noticed I just merged #39 so this PR is no longer required. I just wanted to say thanks to @TheConnMan for pioneering the Docker issue on this project, and even though we didn't end up using this PR, it was a good starting point.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants