allo- / ffprofile

A tool to create firefox profiles with personalized defaults.
GNU Affero General Public License v3.0
769 stars 56 forks source link

Add a Dockerfile for the build and run #211

Closed Starli0n closed 3 years ago

Starli0n commented 3 years ago

Hi,

I added the possibility to build and run a docker image.

For that I put a part of the automatic procedure into a script file so that both the local install and the dockerfile could profit from this and to avoid code duplication.

Both automatic procedure and docker image work fine, including the addons download.

Also, I added a link to the INSTALL.md file from the README.md. I did not see it at first glance and I was glad to discover it.

allo- commented 3 years ago

You don't want to run this in production, do you?

For deployment read the Django docs. You should use something like gunicorn instead of the builtin testserver behind a reverse proxy, e.g., nginx.

allo- commented 3 years ago

Can you rename setup.sh? It looks like an install script for the app but creates a project with default settings. Especially it looks like a setup.py file which is useed to build python packages. We do not have one yet, but people could think it does something similar.

Somethink like create_testproject or exampleproject would probably be better, or moving these scripts in a subfolder that clearly separates them from the standard python files,

Starli0n commented 3 years ago

You don't want to run this in production, do you?

Yes and no, I would like to have a personal instance available everywhere I go. So it does not require production grade.

For deployment read the Django docs. You should use something like gunicorn instead of the builtin testserver behind a reverse proxy, e.g., nginx.

I do not know much about Django and even less about gunicorn, even thought I heard about it. I will have to give a try...

Starli0n commented 3 years ago

Can you rename setup.sh? It looks like an install script for the app but creates a project with default settings. Especially it looks like a setup.py file which is useed to build python packages. We do not have one yet, but people could think it does something similar.

Finally, I move back the content of setup.sh to the Makefile into a dedicated task called create-project and I call it from the Dockerfile as well. I think it is better than to keep creating another folder or file for that. What do you think ?

allo- commented 3 years ago

Have a look here: https://docs.djangoproject.com/en/3.1/howto/deployment/ I currently use gunicorn, but it's not the only option. And Django changes the recommended way from time to time. First it was fastcgi, then wsgi and now they seem to prefer asgi.

For personal use and development runserver will work fine, but it is very basic and can't handle larger loads.

allo- commented 3 years ago

Finally, I move back the content of setup.sh to the Makefile into a dedicated task called create-project and I call it from the Dockerfile as well. I think it is better than to keep creating another folder or file for that. What do you think ?

Sounds okay. I am not so sure if the Makefile is self-explanatory anyway, but on the other hand it is mostly used by developers and the target name is clear enough.

Starli0n commented 3 years ago

Have a look here: https://docs.djangoproject.com/en/3.1/howto/deployment/ I currently use gunicorn, but it's not the only option. And Django changes the recommended way from time to time. First it was fastcgi, then wsgi and now they seem to prefer asgi.

Thank you for the link or for the information, I mostly use python for tooling and when I need a server I use Flask which seems to be lighter than Django.

Starli0n commented 3 years ago

Finally, I move back the content of setup.sh to the Makefile into a dedicated task called create-project and I call it from the Dockerfile as well. I think it is better than to keep creating another folder or file for that. What do you think ?

Sounds okay. I am not so sure if the Makefile is self-explanatory anyway, but on the other hand it is mostly used by developers and the target name is clear enough.

I rebased with your last commit

Starli0n commented 3 years ago

Actually, create-project is used by the Dockerfile as well and it does not use a venv. So it will not work for that case. That's why I use make -s create-project

allo- commented 3 years ago

What about

install: create-project
    # ... (empty?)
create-project: create-venv
    # ...
create-project-with-venv: create-venv create-project
    # (empty)
create-venv:
    # Your code from install

I am just not 100% if this works with make -j in the right order.

I just think that a Makefile without recursion may reduce headache later. When it gets too complicated we can use the recursive solution, as the Makefile is not that complicated anyway.

Starli0n commented 3 years ago

I do not get in your solution, how to create a project without creating venv ?

I do not get what is the issue to use make -s create-project if it is to add more complexity in the Makefile ?

allo- commented 3 years ago

I need a closer look to be sure about the suggested solution, especially since it is pseudo code.

The problem with calling make is that you throw away the dependency management, because make does not know the state (which targets were reached) of the parent/child process.

On the other hand is the current Makefile something that could have been a shellscript, so this does not need to be overoptimzed. I just thought maybe it can be improved.

Starli0n commented 3 years ago

Indeed, here the Makefile is not used to compile but to run some tasks in a more piratical way than a simple shell script. Actually, it enables to run several scripts in one file that should be done using functions and parameters if it would have to done in a shell script.

I already used this method and I never had any issue with it so far...

Starli0n commented 3 years ago

I have thought about something like this:

install: remove create-venv create-project
    (empty)

create-venv:
    # create venv here
    ...

create-project:
    # create project here
    ...

But when testing, I realize that create-project need the activation of the venv which is disable after splitting into two tasks. I do not see how to optimize this without using the make -s

Starli0n commented 3 years ago

Perhaps, you can merge this working solution until finding a more suitable one for you.

Like that I can keep on working, what do you think ?

allo- commented 3 years ago

Yes, I wanted to merge it as is, if your next (this) post doesn't suggest further changes.

Starli0n commented 3 years ago

Thank you very much 👍