Skip to content

Added docker setup for better compatibility#963

Open
animalite wants to merge 5 commits into
okfde:mainfrom
animalite:dockerized
Open

Added docker setup for better compatibility#963
animalite wants to merge 5 commits into
okfde:mainfrom
animalite:dockerized

Conversation

@animalite
Copy link
Copy Markdown

@animalite animalite commented Feb 8, 2026

— Easy environment management
— Adds compatibility with Intel Macs
— Added setup function to devsetup.sh —> dockerized()
— Updated local_settings.py.example to include actual useful values. (ports)

I wanted to setup the dev environment and was impossible to do in my machine because of Torch incompatibilities. Now I can set it up reliably. However, building the image takes a long time, but it's the only option for me.

@animalite animalite marked this pull request as draft February 8, 2026 19:21
@animalite animalite marked this pull request as ready for review February 8, 2026 19:22
@krmax44
Copy link
Copy Markdown
Member

krmax44 commented Feb 9, 2026

Hey, thanks for creating this PR and figuring out all the details! Seems very useful. A couple of thoughts:

  • It might be nice to split up the Django app and the frontend into separate containers and to add them to the existing compose-dev.yaml, instead of creating a new docker-compose.yml.
  • For installing pnpm, I recommend using corepack, which is built-in to Node.js. TL;DR: Run corepack enable after installing Node.js.
  • For installing uv, the authors recommend using a COPY --from in the Dockerfile (see the docs).
  • There seems to be quite a bit of code duplication between the Dockerfile and devsetup.sh. Maybe we can bring them a little closer together?

Thanks again, looking forward to merging this.

Comment thread devsetup.sh
dockerized() {
pull
docker compose build
docker compose run --rm django python manage.py migrate --skip-checks
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is skipping checks required, i.e. are they failing in Docker?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's not necessary. I was just following the Readme and current code. Will be removed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

After some checks, yes, it is necessary since the db is empty and django checks for tables that don't exist yet. I'll put it back.

Comment thread Dockerfile Outdated
@animalite
Copy link
Copy Markdown
Author

I didn't change the current compose-dev.yaml so not to give a surprise to other devs that use a venv with suddenly a 14GB build... Also, it follows the standard (docker compose up).

@animalite
Copy link
Copy Markdown
Author

Seems to be working fine now. Should the README be updated as well?

@animalite animalite requested a review from krmax44 February 9, 2026 17:56
— Fixes Python incompatibilities
— Compatible with Intel Macs
— Added setup function to devsetup.sh
— Updated local_settings.py.example to include actual useful values.
— Uses pre-installed uv python image
— Uses devsetup.sh functions
— Added checks to devsetup.sh for Docker compatibility
— Doesn't skip checks at migration
— Node 24 & Python 3.13
— Added missing libraries
@krmax44
Copy link
Copy Markdown
Member

krmax44 commented Apr 20, 2026

Sorry for the delay! I'm thinking about how we can integrate this with well froide, i.e. by having froide base images that we can pick up in this project. Your PR is a great starting point for this, so thanks again! Since we're not using Docker outside of running Postgres + ElasticSearch during development, this doesn't have super high priority right now. But it might be handy for use in CI as well, see okfde/froide#1237.

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.

2 participants