Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
__pycache__
.git
.pyc
.env
media/
staticfiles/*
27 changes: 18 additions & 9 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
.idea/
.vscode/
*.iml
# Python envs and caches
.venv
__pycache__/
*.pyc
# Git
.git
.gitignore
# Env and secrets
.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.

Installing Python packages may require OS-level build dependencies (for example if psycopg2 is in requirements). Consider either using psycopg2-binary in requirements or installing necessary system packages (e.g. libpq-dev, gcc) before pip install so the image build doesn't fail. Alternatively, add a multi-stage build or use --no-install-recommends and apt-get to keep the image thin.

.DS_Store
venv/
.pytest_cache/
**__pycache__/
**db.sqlite3
media
# Media and static (не включати в образ)
media/
staticfiles/
# IDE / editor
.vscode/
Comment on lines +7 to +14
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 Dockerfile installs build dependencies (gcc, libpq-dev) before pip install, which is valid to build psycopg2 from source but increases image size. To make the image thinner (task requirement) consider either: 1) using psycopg2-binary so you can skip OS build deps, or 2) installing build deps, running pip install, then removing the build deps and cleaning apt caches, or 3) using a multi-stage build. Also ensure removal/purge of build packages where appropriate so they don't remain in the final image.

.idea/
# Test / build artifacts
Comment on lines +12 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.

Minor: you COPY . /app which is fine because you already COPY requirements.txt and install before copying the rest (good for layer caching). Make sure your .dockerignore excludes local files you don't want in the image (you have .env, .venv, media/, staticfiles/* listed — good). If you want even slimmer images, consider copying only the necessary sources instead of the whole repo.

dist/
build/
*.egg-info/
21 changes: 21 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
FROM python:3.11-slim

ENV PYTHONDONTWRITEBYTECODE=1 PYTHONUNBUFFERED=1

WORKDIR /app

COPY requirements.txt .

RUN pip install --no-cache-dir -r requirements.txt

COPY . /app

RUN mkdir -p /app/media/uploads/movies /app/staticfiles \
&& adduser --disabled-password --no-create-home my_user \
&& chown -R my_user:my_user /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 success message is printed inside the except branch. That means you log "Database available!" when an OperationalError occurs (DB unavailable). Move this message into the successful branch (e.g., use an else: after the try/except) and break or return so the loop stops once db_conn.cursor() succeeds. This addresses checklist item #23 (HIGH).

&& chmod +x /app/entrypoint.sh


USER my_user

Comment on lines +13 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 success message is printed on every loop iteration because self.stdout.write(self.style.SUCCESS("Database available!")) is placed outside the try/except. Move this success message inside the try block immediately after db_conn.cursor() and then break (or return) so the loop ends only when the DB is actually available.

ENTRYPOINT ["/app/entrypoint.sh"]
Empty file added cinema/management/__init__.py
Empty file.
Empty file.
20 changes: 20 additions & 0 deletions cinema/management/commands/wait_for_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import time
from django.db.utils import OperationalError
from django.db import connections
from django.core.management.base import BaseCommand


class Command(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.

In .dockerignore you currently ignore staticfiles/* which excludes the contents but not necessarily the directory itself. Prefer ignoring the directory with staticfiles/ so the whole staticfiles tree is excluded from the image build context.

help = "Waits for DB to be available" # noqa: VNE003

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This RUN pip install -r requirements.txt step will pull in build-time dependencies if psycopg2 (non-binary) is in requirements. To satisfy the checklist about making images thin ([CHECKLIST ITEM #15 / #26]) either switch to psycopg2-binary in requirements.txt, or install gcc/libpq-dev only during build and remove them afterwards (or use a multi-stage build) so build deps don't remain in the final image.

def handle(self, *args, **options):
self.stdout.write("Waiting for database...")
db_conn = None
while not db_conn:
try:
db_conn = connections["default"]
db_conn.cursor()
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 /app/staticfiles here which is where collectstatic writes files. To persist those files across container recreation (checklist [#16]) you must declare and mount a named volume (e.g. staticfiles:/app/staticfiles) in your docker-compose.yml. Without that, collectstatic output will be lost when containers are recreated.

except OperationalError:
self.stdout.write("Database unavailable, waiting 1 second")
time.sleep(1)
self.stdout.write(self.style.SUCCESS("Database available!"))
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 success message is printed regardless of whether the DB connection attempt succeeded because this line is outside the try/except. Move this success write so it only runs when db_conn.cursor() succeeds (for example, place it inside the try after db_conn.cursor() or use a try/except/else and print success in the else). This currently breaks the core requirement that the command should wait until the DB is available.

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 5.2.13 on 2026-04-20 12:46

import django.db.models.deletion
Comment on lines +1 to +3
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 entrypoint script should fail fast if any command fails to avoid continuing with a partially initialized container. Add set -e (or set -o errexit) near the top so the script exits on errors instead of continuing silently.

from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):

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 (pip install -r requirements.txt) on slim images often requires OS-level build dependencies for database adapters (e.g. libpq-dev, gcc) if you're using psycopg2. Either use psycopg2-binary in requirements or install required OS packages before pip install (and then remove caches) to avoid build failures and keep the image thin.

dependencies = [
('cinema', '0001_initial'),
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
Comment on lines +11 to +12
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mounting the host project directory into /app at runtime (bind mount .:/app) will override files from the built image — this can remove the executable bit you set on /app/entrypoint.sh during image build and/or change ownership permissions. For production images avoid bind mounts, or ensure the host file has correct permissions, or use a dedicated code volume strategy.

]

operations = [
migrations.AlterField(
model_name='movie',
name='actors',
field=models.ManyToManyField(blank=True, related_name='movies', to='cinema.actor'),
),
Comment on lines +13 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.

In the wait_for_db command the success message is printed regardless of whether the DB connection succeeded because the success write is outside the try/except and the loop never breaks on success. Move the success message to run only after db_conn.cursor() succeeds (e.g. place it inside the try block or use else:) and break/return to exit the loop so the command truly waits until DB is available.

migrations.AlterField(
model_name='movie',
name='genres',
field=models.ManyToManyField(blank=True, related_name='movies', to='cinema.genre'),
),
Comment on lines +24 to +25
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 db service volume uses my_db:$PGDATA. Relying on the $PGDATA environment variable here is fragile unless you guarantee it's defined in .env. Consider using the canonical Postgres data directory (for example /var/lib/postgresql/data) or ensure $PGDATA is set in the env file to avoid an unintended empty mount target.

migrations.AlterField(
model_name='moviesession',
name='cinema_hall',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='movie_sessions', to='cinema.cinemahall'),
),
migrations.AlterField(
model_name='moviesession',
name='movie',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='movie_sessions', to='cinema.movie'),
),
migrations.AlterField(
model_name='order',
name='user',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='orders', to=settings.AUTH_USER_MODEL),
),
]
12 changes: 9 additions & 3 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 image installs Python packages with pip install -r requirements.txt, but building extensions (for example psycopg2) often requires OS-level build deps (e.g. libpq-dev, gcc). Add necessary apt-get installs before pip install, or use psycopg2-binary to avoid compilation in the image and ensure reliable builds.

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.

This bind mount .:/app makes the container depend on your local filesystem (overrides image content and permissions). It violates the task requirement that the service be runnable using only Docker. Replace the bind mount with an image-only setup or a named volume for code so the image is self-contained.

import os
from datetime import timedelta
from pathlib import Path

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

DATABASES = {
"default": {
"ENGINE": "django.db.backends.sqlite3",
"NAME": BASE_DIR / "db.sqlite3",
"ENGINE": "django.db.backends.postgresql",
"NAME": os.environ["POSTGRES_DB"],
"USER": os.environ["POSTGRES_USER"],
"PASSWORD": os.environ["POSTGRES_PASSWORD"],
"HOST": os.environ["POSTGRES_HOST"],
"PORT": os.environ["POSTGRES_PORT"],
}
}

Expand Down Expand Up @@ -131,7 +136,8 @@
# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/4.0/howto/static-files/

STATIC_URL = "static/"
STATIC_URL = "/static/"
STATIC_ROOT = BASE_DIR / "staticfiles"

MEDIA_URL = "/media/"
MEDIA_ROOT = BASE_DIR / "media"
Expand Down
29 changes: 29 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
services:
cinema:
build:
context: .
env_file:
- .env
ports:
- "8000:8000"
command: python manage.py runserver 0.0.0.0:8000
volumes:
- .:/app
- my_media:/app/media/uploads/movies
depends_on:
- db


db:
image: postgres:16.0-alpine3.17
restart: always
env_file:
- .env
ports:
- "5432:5432"
volumes:
- my_db:$PGDATA

volumes:
my_db:
my_media:
7 changes: 7 additions & 0 deletions entrypoint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/sh

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 docker-compose service is named cinema (line shown) while the task's example commands use app (for example: docker-compose run app sh -c "python manage.py test"). Either rename the service to app or document the service name difference so reviewers can run the example commands as-is.

python manage.py wait_for_db
Comment on lines +1 to +5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add a fail-fast option so the script stops on any error (for example, set -e near the top). Without this, if migrate or collectstatic fails the script will still exec the final command and the container may start in a broken state.

python manage.py migrate
python manage.py collectstatic --noinput
Comment on lines +5 to +7
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Be aware that bind-mounting the project directory over /app (for example with .:/app in docker-compose) can overwrite the entrypoint file or its executable bit set during image build. If you keep that mount in development, verify the host copy of entrypoint.sh is executable, or consider placing the entrypoint outside the mounted directory or running the script via sh /app/entrypoint.sh to avoid permission issues.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In .dockerignore you ignore staticfiles/*. Prefer ignoring the whole directory with staticfiles/ so the entire staticfiles tree is excluded from the build context (safer and clearer). This helps keep the image context small as required by the checklist.


exec "$@"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This pip install --no-cache-dir -r requirements.txt step can leave build-time dependencies in the final image if psycopg2 (non-binary) is used. To satisfy the checklist about making images thin, either switch to psycopg2-binary in requirements.txt or install gcc/libpq-dev only for the pip install step and then remove/purge them (or use a multi-stage build) so build deps do not remain in the final image.

Binary file modified requirements.txt
Binary file not shown.
Loading