Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.venv
.env
__pycache__/
*.pyc
.idea
media
.pytest_cache/
25 changes: 25 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
FROM python:3.11-slim

LABEL maintainer="rarturus"

ENV PYTHONUNBUFFERED 1

WORKDIR /app

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The management command has a typo: use the attribute name help, not help1. While the command will still run, the help text won't be available in manage.py help and this deviates from the typical implementation required by the task. Consider also adding a small timeout/retry limit or logging if you need it.

COPY requirements.txt .

RUN apt-get update && apt-get install -y build-essential libpq-dev && rm -rf /var/lib/apt/lists/*
RUN pip install --no-cache-dir -r requirements.txt
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Installing Python packages on Alpine often requires system build dependencies (gcc, musl-dev, postgresql-dev, libffi-dev, openssl-dev, build-base, etc.) if your requirements include packages like psycopg2. If you rely on such packages, add the necessary apk add --no-cache ... before pip install, or use prebuilt wheels (for example psycopg2-binary) to avoid build failures on Alpine.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical: you must install OS build dependencies before running pip install so native DB drivers (like psycopg2) can be built. Add a RUN step such as: apt-get update && apt-get install -y build-essential libpq-dev && rm -rf /var/lib/apt/lists/* before the pip install line to avoid build failures on python:3.11-slim.


COPY . .
RUN mkdir -p /files/media

RUN adduser \
--disabled-password \
--no-create-home \
my_user
Comment on lines +17 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The adduser options used here (--disabled-password, --no-create-home) are GNU-style and may not be supported by Alpine's adduser. On Alpine you typically use adduser -D -H my_user or create user/group with addgroup/adduser flags appropriate for the base image. Please adapt this to the Alpine toolset so the user is created reliably.


RUN chown -R my_user:my_user /files/media
RUN chmod -R 755 /files/media
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Permissions: you create /files/media and later run USER my_user, but you never chown the media directory to the non-root user. Add chown -R my_user:my_user /files/media (or include it with the existing chown) before switching to USER my_user so the app can write uploaded files at runtime.


USER my_user
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Setting USER my_user makes the container run as non-root, which is good for security but will often cause permission problems when mounting the host project into /app (the mounted files may be owned by root or a different UID). Either ensure /app is owned by my_user (chown) at runtime, create the user with the same UID as the host developer, or run as root in development to avoid permission errors with migrations, collectstatic, or file writes.

Empty file added cinema/management/__init__.py
Empty file.
Empty file.
19 changes: 19 additions & 0 deletions cinema/management/commands/wait_for_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import time
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor/informational: the migration header says it was generated by Django 6.0.4 while project settings indicate Django 4.x. This version mismatch is unusual and may indicate the migration was generated with a different Django version. Verify migrations and Django version compatibility.

from django.core.management.base import BaseCommand
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical: You are ignoring the Dockerfile in .dockerignore. Docker will not send the Dockerfile into the build context if it's listed here, which prevents building the app image. Remove this line so the Dockerfile is included in the build context.

from django.db import connections
from django.db.utils import OperationalError


class Command(BaseCommand):
help = "Wait for database to be available"

def handle(self, *args, **options):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If your requirements include packages like psycopg2 (common for PostgreSQL), this pip install step will fail because the image currently does not install system build dependencies (libpq headers, gcc, etc.). On Debian-slim you should run apt-get update && apt-get install -y build-essential libpq-dev (and clean apt caches) before pip install (or switch to psycopg2-binary).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Before running pip install -r requirements.txt you must install OS build dependencies required for DB drivers on python:3.11-slim. Add a RUN step before line 10 such as:

RUN apt-get update && apt-get install -y build-essential libpq-dev && rm -rf /var/lib/apt/lists/*

This ensures packages like psycopg2 can compile and keeps image layers clean as required by the checklist.

self.stdout.write("Waiting for database...")
while True:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

COPY . . is fine for image build, but if you later mount the project directory as a volume in docker-compose (bind mount), it will overwrite the image contents at runtime and make the image less independent/thin. Reconsider the bind mount in compose if your goal is to make the service independent of the local machine, or document that mounting is for development only.

try:
connections["default"].cursor()
break
except OperationalError:
self.stdout.write("Database unavailable, waiting 1 second...")
time.sleep(1)
Comment on lines +13 to +16
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The adduser command used here appears to use Debian-style flags (e.g. --disabled-password, --no-create-home). On Alpine-based images the adduser implementation and flags are different and this RUN may fail during image build. Either use an Alpine-compatible user creation (e.g. adduser -D my_user) or use a different base image. Also consider creating a home directory if needed.

Comment on lines +13 to +16
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You create a user with adduser ... my_user. Make sure the flags used are supported by this base image (python:3.11-slim is Debian-based so these flags are likely fine). Still: it is better to set ownership of the application dir (/app) and switch to the non-root user with USER my_user so the container doesn't run as root. Also consider creating the user non-interactively and ensuring home/uid if needed.

self.stdout.write(self.style.SUCCESS("Database available!"))
194 changes: 0 additions & 194 deletions cinema/migrations/0001_initial.py

This file was deleted.

15 changes: 10 additions & 5 deletions cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
For the full list of settings and their values, see
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The app service runs Django on 0.0.0.0:8000 but there is no ports mapping for the service. Add ports: - "8000:8000" so the API is accessible on 127.0.0.1:8000 as required by the task.

https://docs.djangoproject.com/en/4.0/ref/settings/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical: the Dockerfile runs pip install -r requirements.txt without first installing OS build dependencies needed by DB drivers (e.g. psycopg2). Per the checklist you must add a step before pip install such as: apt-get update && apt-get install -y build-essential libpq-dev && rm -rf /var/lib/apt/lists/* so the image builds successfully on python:3.11-slim.

"""
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You bind-mount the project directory into the container (./:/app). This makes the container dependent on the host filesystem and contradicts the requirement that the service be fully independent of the local machine. For a production-like Docker setup remove the bind mount (use the image contents) or make it optional (development-only).

import os
from datetime import timedelta
from pathlib import Path

Expand Down Expand Up @@ -86,12 +87,15 @@

DATABASES = {
"default": {
"ENGINE": "django.db.backends.sqlite3",
"NAME": BASE_DIR / "db.sqlite3",
"ENGINE": os.environ.get("SQL_ENGINE", "django.db.backends.sqlite3"),
"NAME": os.environ.get("SQL_DATABASE", BASE_DIR / "db.sqlite3"),
Comment on lines +90 to +91
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The database ENGINE and NAME default to SQLite if environment variables are not provided. Ensure your .env (used by docker-compose) sets SQL_ENGINE=django.db.backends.postgresql and SQL_DATABASE to the PostgreSQL DB name so Django uses Postgres in Docker. Without these env vars Django will try to use SQLite instead of the required PostgreSQL.

"USER": os.environ.get("SQL_USER", "user"),
"PASSWORD": os.environ.get("SQL_PASSWORD", "password"),
"HOST": os.environ.get("SQL_HOST", "localhost"),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Default HOST is localhost. Inside the app container Django should connect to the Postgres service by host name (the service name in docker-compose, e.g. db). Make sure your .env sets SQL_HOST=db, otherwise Django will attempt to connect to a DB on the same container and fail.

"PORT": os.environ.get("SQL_PORT", "5432"),
}
}


# Password validation
# https://docs.djangoproject.com/en/4.0/ref/settings/#auth-password-validators

Expand Down Expand Up @@ -134,7 +138,8 @@
STATIC_URL = "static/"

MEDIA_URL = "/media/"
MEDIA_ROOT = BASE_DIR / "media"
MEDIA_ROOT = "/files/media"


# Default primary key field type
# https://docs.djangoproject.com/en/4.0/ref/settings/#default-auto-field
Expand Down Expand Up @@ -167,7 +172,7 @@
}

SIMPLE_JWT = {
"ACCESS_TOKEN_LIFETIME": timedelta(minutes=5),
"ACCESS_TOKEN_LIFETIME": timedelta(days=1),
"REFRESH_TOKEN_LIFETIME": timedelta(days=1),
"ROTATE_REFRESH_TOKENS": False,
}
34 changes: 34 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
services:
app:
build:
context: .
env_file:
- .env
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You currently ignore docker-compose.yml in .dockerignore. This can remove the compose file from the build context and is unnecessary — remove this entry so the compose file stays available.

command: >
sh -c "python manage.py wait_for_db && python manage.py migrate &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The management command defines help1 instead of the expected help attribute on BaseCommand. Rename help1 to help so the command follows Django conventions and manage.py help shows the description.

python manage.py runserver 0.0.0.0:8000"
volumes:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Installing Python dependencies on Alpine often requires system build libraries (gcc, musl-dev, postgresql-dev, libffi-dev, openssl-dev, etc.). Add an apk add --no-cache ... step before pip install -r requirements.txt or use prebuilt wheels (e.g. psycopg2-binary) to avoid build failures on Alpine.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RUN pip install -r requirements.txt may fail if any requirements need compilation (for example psycopg2). On Debian-slim you should install system build deps (apt-get update && apt-get install -y build-essential libpq-dev) before pip install, or use psycopg2-binary to avoid native compilation.

- media_data:/files/media
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

COPY . . copies the project into the image, but your docker-compose also binds the host project directory into /app at runtime. That bind mount will overwrite the image contents and can cause permission/consistency issues. If the goal is a service independent of the local machine, avoid bind-mounting the project in production compose.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good: the app mounts media_data:/files/media and MEDIA_ROOT is set to /files/media, so uploaded media will be stored in the mounted volume. Ensure the runtime user has write permission to that path.

depends_on:
- db
Comment on lines +12 to +13
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Operational note: depends_on ensures ordering for docker-compose up but docker-compose run app ... does not start dependent services automatically. To run docker-compose run app sh -c "python manage.py test" as required, ensure the db service is already running (for example docker-compose up -d db) or start services with docker-compose up -d before running tests.

ports:
- "8000:8000"

db:
image: postgres:16.0-alpine3.17
Comment on lines +15 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You create a my_user account but never change ownership of /app nor switch to that user. For better security and to avoid permission issues, add chown -R my_user:my_user /app (or ensure files copied are owned by that user) and add USER my_user before the final image runtime.

environment:
Comment on lines +16 to +19
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The adduser flags used here are GNU/Debian-style and may not be supported on Alpine. Use Alpine-compatible options like adduser -D my_user (and addgroup/adduser as needed) or switch to a non-Alpine base image. Otherwise the image build may fail.

POSTGRES_USER: "rarturus"
POSTGRES_PASSWORD: "password"
Comment on lines +20 to +21
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You only chown /files/media, not the application directory. If you plan to run the container as my_user, also chown -R my_user /app so the process can access app files.

POSTGRES_DB: "db"
restart: always
ports:
- "5432:5432"
env_file:
- .env
volumes:
- db_data:/var/lib/postgresql/data
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good: the Postgres service volume is mapped to /var/lib/postgresql/data so DB data will persist correctly as required by the checklist.



volumes:
db_data:
media_data:
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@ djangorestframework
djangorestframework-simplejwt
drf-spectacular
Pillow
psycopg==3.3.3
psycopg-binary==3.3.3
Loading
Loading