Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ checkstyle.txt
.envrc
.tool-versions
.elasticbeanstalk/
.local/

# These get baked into the docker container on build
src/CI_COMMIT_SHA
Expand Down
43 changes: 43 additions & 0 deletions api/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# NOTE: Why not Alpine? Because Alpine demands installation of OS software
# prior to running app code, which leads to maintenance work and a larger image
# size compared to Debian — considering Debian layers are shared across images.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand how having to maintain a new Dockerfile is a viable trade-off here. I'll prefer a unified build toolchain to a neater dev container image; I'd rather this file be a new target stage in the existing root Dockerfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed! For now this Dockerfile is only experimental. It aligns nicely with the proposal but it would have a long way to go before becoming an actual thing.

FROM python:3.12

# Prevent Python from buffering stdout and stderr
# https://docs.python.org/3/using/cmdline.html#envvar-PYTHONUNBUFFERED
ENV PYTHONUNBUFFERED=1

# Prevent Python from writing .pyc files to disk
# https://docs.python.org/3/using/cmdline.html#envvar-PYTHONDONTWRITEBYTECODE
ENV PYTHONDONTWRITEBYTECODE=1

# Install Python packages
WORKDIR /tmp
ARG POETRY_VERSION_CONSTRAINT
ARG INSTALL_DEV_DEPENDENCIES
Copy link
Member

Choose a reason for hiding this comment

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

Why would we want to not install dev dependencies in a development container?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! The ability to not install dev dependencies allows us to reuse the image setup in other contexts, e.g. production. But then again, this Dockerfile is only experimental.

COPY api/pyproject.toml api/poetry.lock ./
RUN \
pip install -q poetry${POETRY_VERSION_CONSTRAINT:-} &&\
poetry config virtualenvs.create false &&\
poetry config cache-dir /var/cache/pypoetry &&\
###
# NOTE: Forcefully delete optional private dependencies for now since
# Poetry needs to fetch them in order to calculate the dependency tree. See
# https://github.com/python-poetry/poetry/issues/4562
# TODO: Improve management of enterprise features
Copy link
Member

Choose a reason for hiding this comment

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

This is, at least in part, being tracked in #4764 (comment).

sed -ie '/tool.poetry.group.auth-controller/,+1d' pyproject.toml &&\
sed -ie '/tool.poetry.group.saml/,+1d' pyproject.toml &&\
sed -ie '/tool.poetry.group.ldap/,+1d' pyproject.toml &&\
sed -ie '/tool.poetry.group.workflows/,+1d' pyproject.toml &&\
sed -ie '/tool.poetry.group.licensing/,+1d' pyproject.toml &&\
poetry lock &&\
###
poetry install --no-root $([ "$INSTALL_DEV_DEPENDENCIES" != "true" ] && echo "--without dev") &&\
rm -rf /var/cache/pypoetry &&\
rm -rf /tmp/*
Copy link
Member

Choose a reason for hiding this comment

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

Private dependency problem aside (which, IMO, should not be solved here), why isn't this make install with POETRY_CACHE_DIR set to a mounted path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'd rather use #4764 to improve private dependency management. This sed trickery only exists to favor this PoC.

why isn't this make install with POETRY_CACHE_DIR set to a mounted path?

Another good question. I can list a few reasons why not to use make install within the Docker build:

  • IMO make is better suited for persons interacting with the shell directly. Especially, assuming make commands are going to run Docker underneath [if we eventually decide in favor of it], I'd rather not introduce inconsistency by having some commands that are intended to run from within a container instead of from the outside.
  • Currently make install will: 1. install pip, 2. install Poetry in a venv, then finally 3. install Python packages.
    • pip is already available in the Python Docker image, there's no need to reinstall.
    • The Poetry install script will setup a venv and install Poetry. A Docker env has no benefit out of Python virtualenvs, or really anything else this script does besides finally pip install poetry.
    • Though currently a necessary step to setup the local env, make install could be disregarded entirely thanks to ./run — or Docker Compose, really.
  • Finally, RUN --mount to improve build efficiency sounds like a great idea! We'd have to explore that.


# Prepare the app
WORKDIR /opt/app
COPY api/ .
EXPOSE 8000
CMD ["flagsmith", "start", "api"]
3 changes: 2 additions & 1 deletion api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions api/run
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/bin/sh

# Runs any command from within an `api` container.
# If no command is given, it runs the application's default command.

# NOTE: This script is intended as a one-command option to run Flagsmith code
# with no previous setup except from installing Docker.

# Requirements:
# - Docker

# Examples:
# - `./run` — runs pending migrations and starts the API HTTP server in dev mode.
# - `./run bash` — starts a bash shell in the API container.
# - `./run python manage.py makemigrations` — runs the command in the API container.
# - `./run flagsmith createsuperuser` — the Flagsmith CLI should be there too!
# - `./run pytest --sw --pdb tests/unit/test_my_feature.py` — you get it.

SCRIPT=$(readlink -f "$0")
SCRIPT_DIR=$(dirname "$SCRIPT")
ROOT_DIR=$(cd "$SCRIPT_DIR/.."; pwd -P)
PWD=$(pwd -P)
WORKING_DIR=$(echo "$PWD" | sed "s|$ROOT_DIR||")

docker compose -f "${ROOT_DIR}/docker/new/docker-compose.yml" run --rm \
$([ $# -eq 0 ] && echo --service-ports --use-aliases) \
$([ -n "${WORKING_DIR}" ] && echo --workdir /opt/app${WORKING_DIR}) \
api "$@"
42 changes: 42 additions & 0 deletions docker/new/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Reusable app definition
x-app: &app
image: ${DOCKER_IMAGE:-local/flagsmith/flagsmith}:${DOCKER_TAG:-latest}
build:
context: ../../
dockerfile: api/Dockerfile
args:
POETRY_VERSION_CONSTRAINT: ==2.1.2
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather pin to a specific version (see Makefile).

INSTALL_DEV_DEPENDENCIES: true
depends_on:
database:
condition: service_healthy
environment:
DEBUG: true
DATABASE_URL: postgres://postgres@database:5432/postgres
DJANGO_SETTINGS_MODULE: app.settings.local
volumes:
- ../../.local/pypoetry:/root/.config/poetry:rw # Poetry local cache
- ../../:/opt/app:rw,cached # Application directory

services:
api:
<<: *app
command: sh -c 'flagsmith migrate && flagsmith start --reload api'
ports:
- 8000:8000

# TODO
# task-processor:
# <<: *app
# command: ...

database:
image: postgres:15.5-alpine
volumes:
- ../../.local/database:/var/lib/postgresql/data:rw,cached
environment:
POSTGRES_HOST_AUTH_METHOD: trust
healthcheck:
test: pg_isready -Upostgres
interval: 1s
timeout: 30s
Loading