Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.github/
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using the Alpine base is fine, but many Python packages that interface with PostgreSQL (e.g. psycopg2) need system build dependencies on Alpine. Add apk installation of required packages (e.g. build-base, postgresql-dev, musl-dev) or switch to a Debian-slim base or use psycopg2-binary to avoid compilation failures at pip install 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.

Using python:3.11-alpine can cause pip installs (notably psycopg2) to fail without additional build dependencies (gcc, musl-dev, postgresql-dev). For simplicity and smaller images, prefer python:3.11-slim or explicitly install the required build tools before pip install if you want to keep 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.

Base image is python:3.11-alpine. The task expects the container to support docker exec -it <container> bash and alpine images usually don't include bash and often require extra build packages to compile psycopg2. Prefer python:3.11-slim (recommended) or, if you keep alpine, explicitly install build deps (e.g. gcc, musl-dev, postgresql-dev) and bash so commands like docker exec -it <container> bash work and dependency builds succeed.

.venv/
.gitignore
README.md
.env
.pytest_cache/
*.pyc
__pycache__/
20 changes: 20 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
FROM python:3.11-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.

Using the python:3.11-alpine base can force you to install additional build dependencies (gcc, musl-dev, postgresql-dev) for packages like psycopg2. Consider using python:3.11-slim or explicitly installing required build deps and cleaning them up to keep the image thin.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(Medium) Using Alpine Python image can cause build issues for packages like psycopg2 and typically requires installing build dependencies (gcc, musl-dev, postgresql-dev). Consider using python:3.11-slim for smaller, simpler images or explicitly install and then remove build deps in this Dockerfile to keep the image thin.

LABEL maintainer="ys33ys33ys55@gmail.com"

ENV PYTHONUNBUFFERED=1

WORKDIR /app/
Comment on lines +1 to +6
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 sensitive and unnecessary files to .dockerignore (for example .env, __pycache__/, *.pyc, and .pytest_cache/) so secrets are not sent to the build context and image context is smaller. Also consider whether ignoring Dockerfile or docker-compose.yaml is intended — usually you do NOT need to include your build files in the build context, but double-check.



COPY . .
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

COPYing the entire project before installing dependencies prevents Docker from caching the installed dependencies layer. Copy requirements.txt first, run pip install, then COPY the rest of the project to take advantage of caching and speed up rebuilds.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(Medium) Copying the entire project context before installing Python dependencies prevents proper layer caching and makes rebuilds slower. Change order to: COPY requirements.txt .RUN pip install --no-cache-dir -r requirements.txtCOPY . ..


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.

(Medium) Use pip install --no-cache-dir -r requirements.txt to avoid wheel cache bloat and reduce final image size. Also ensure build dependencies for any packages that require compilation (or use psycopg2-binary if acceptable).


RUN mkdir -p /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.

(Informational) Creating /files/media/ and setting ownership/permissions is fine (you do it before switching to non-root user). Ensure this path matches your docker-compose named volume for media so files persist between container restarts.


RUN adduser --disabled-password --no-create-home my_user

Comment on lines +9 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.

(Low) The retry loop can become a tight busy-loop. Add import time and a small sleep like time.sleep(1) in the except block to avoid high CPU usage and excessive logs while Postgres starts.

RUN chown -R my_user /files/media/
RUN chmod -R 755 /files/media/

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.

(Informational / caution) You add a non-root user and switch to it (good). Remember package installation must run as root before USER my_user — ensure any future RUN steps that require root are placed above the USER instruction.

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

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.

Using the alpine base is fine but note: many DB drivers (psycopg2) require build dependencies on Alpine. Consider using python:3.11-slim or add and then remove build packages in the Dockerfile to keep the image thin as required by the task.

from django.db import OperationalError, connection


class Command(BaseCommand):
def handle(self, *args, **options):
self.stdout.write("Waiting for database...")
db_conn = None
while not db_conn:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

COPYing the whole project before installing dependencies prevents Docker layer caching for Python packages. To make the image thinner and faster to rebuild, COPY requirements.txt . then RUN pip install ... then COPY . ..

try:
connection.ensure_connection()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use pip install --no-cache-dir -r requirements.txt (and preferably combine with other RUN steps) to avoid pip cache left in layers and reduce final image size per the task requirement.

db_conn = True
except OperationalError:
Comment on lines +11 to +15
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 wait_for_db command loops without any delay which will spin the CPU while Postgres is not ready. Add import time and a time.sleep(1) (or similar) in the except branch to retry with pauses, and consider logging attempt counts or a timeout.

self.stdout.write("Database unavailable...")
time.sleep(1)

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 --disabled-password --no-create-home flags are Debian-style and may not exist on Alpine. Either switch base image to a Debian-slim variant or adjust the user creation to Alpine syntax (e.g. adduser -D my_user) so the container build won't fail.

self.stdout.write(self.style.SUCCESS("Database available!"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Generated by Django 5.2.13 on 2026-04-16 13:35
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using alpine can reduce image size but often requires additional build dependencies for packages like psycopg2. Consider either installing required build libs (gcc, musl-dev, libpq) or using a slim Debian image for easier compatibility.

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 base image is python:3.11-alpine. Per the task notes you should either switch to python:3.11-slim (recommended) or, if you keep Alpine, add the necessary build tools (gcc, musl-dev, postgresql-dev, build-base, etc.) so psycopg2 and other compiled deps can be installed reliably.


import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't ignore the Dockerfile in .dockerignore — excluding the Dockerfile from the build context can break building the image. Remove this line so the Dockerfile is sent to the daemon during docker build.


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ignoring docker-compose.yaml may hide important compose configuration from the build context and tools; typically you don't need to ignore the compose file. Remove this entry to avoid surprises.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WORKDIR is set to /app/ but you never change ownership of this directory. When switching to a non-root user later the process may lack write permissions. Consider chown'ing /app to my_user (or create the user before COPY and set correct ownership).


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.

COPY . . before installing requirements prevents Docker build cache reuse. For faster and thinner builds, first COPY only requirements.txt, run pip install, then COPY the rest of the project.

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 COPY . . before installing dependencies. To leverage layer caching and keep images thin, first COPY requirements.txt ., then RUN pip install --no-cache-dir -r requirements.txt, and only then COPY . ..

dependencies = [
('cinema', '0001_initial'),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use pip install --no-cache-dir -r requirements.txt to avoid leaving pip cache in the image and keep the image thinner.

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 line should run immediately after copying only requirements.txt so that Docker caches the installed dependencies layer. If you keep Alpine, also ensure system build deps are installed in the same layer before pip install.

migrations.swappable_dependency(settings.AUTH_USER_MODEL),
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 project is missing the required wait_for_db Django management command. The app must run wait_for_db before applying migrations or starting, so docker-compose up doesn't fail while DB is initializing.

]

operations = [
migrations.AlterField(
model_name='movie',
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 chown /files/media/ but not /app/ after copying files; if the app or tests need to write into /app while running as my_user they may fail. Consider chown'ing /app (or create the user beforehand and set the correct permissions).

name='actors',
field=models.ManyToManyField(blank=True, related_name='movies', to='cinema.actor'),
),
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 add a non-root user and switch to it, but you don't install bash. The task explicitly suggests entering container with bash (docker exec -it bash); on Alpine bash is not present by default, so bash will fail. Install bash or document using sh, and ensure admin creation is possible from inside the container.

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 switch to a non-root user here. Make sure that after reordering COPY steps any files the app needs to write (e.g., static collection target, media dirs) are owned or writable by my_user (adjust chown after full copy if needed). Currently only /files/media/ is chowned; ensure other runtime paths won't cause permission errors.

migrations.AlterField(
model_name='movie',
name='genres',
field=models.ManyToManyField(blank=True, related_name='movies', to='cinema.genre'),
),
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),
),
]
16 changes: 12 additions & 4 deletions cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
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.

COPYing the entire project before installing dependencies prevents Docker layer caching for requirements. Better pattern: COPY only requirements.txt, run pip install --no-cache-dir -r requirements.txt, then COPY the rest of the code. This produces thinner and faster rebuilds.

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 COPY . . before installing dependencies. To leverage Docker layer caching and keep images thin you should COPY requirements.txt . then RUN pip install --no-cache-dir -r requirements.txt and only after that COPY . . so dependency installation is cached and image rebuilds are faster/smaller.

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.

Use pip install --no-cache-dir -r requirements.txt to avoid storing pip cache in the image. Also if your requirements include a non-binary psycopg2, you will need OS-level build dependencies on Alpine (e.g. build-base, postgresql-dev) or use psycopg2-binary to avoid build steps in the image.

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 currently runs after copying the whole project. Move the pip install --no-cache-dir -r requirements.txt step to immediately follow copying only requirements.txt (and ensure any system build deps are installed in the same layer if you stay on Alpine).

import os

Comment on lines 9 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.

Good: the command runs python manage.py wait_for_db before migrations and runserver. Note that depends_on only controls start order but not readiness — your explicit wait_for_db covers that requirement (keep it).

from datetime import timedelta
from pathlib import Path
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 with ./:/app/. Remove this host bind-mount so the image is self-contained; rely on the built image for the app code and use named volumes only for DB and media to satisfy the 'Docker-only' requirement.


Expand All @@ -27,7 +29,9 @@
# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = []
ALLOWED_HOSTS = [
"0.0.0.0",
]
Comment on lines +32 to +34
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ALLOWED_HOSTS includes 0.0.0.0, which allows the container to bind, but the task expects the app to be reachable at 127.0.0.1:8000/api/. Ensure docker-compose maps host port 8000 to container 8000 (not 8001), and verify ALLOWED_HOSTS/CORS settings match how you expose the service. (Port mismatch is in docker-compose and must be fixed.)


INTERNAL_IPS = [
"127.0.0.1",
Expand Down Expand Up @@ -86,8 +90,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"],
Comment on lines +93 to +98
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 Django DATABASES settings use os.environ[...] which will raise KeyError if any variable is missing. Per the task you must change these to os.environ.get(..., default) (for example default host 'db' and default port '5432') or ensure all env vars are always set. Replace the lookups on these lines accordingly.

}
Comment on lines +93 to 99
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your settings read DB credentials with os.environ[...] which will raise a KeyError if those environment variables are missing during import. At a minimum ensure your .env supplied to docker-compose defines POSTGRES_DB/USER/PASSWORD/HOST/PORT. It's also safer to use os.environ.get() with sensible defaults or raise clearer errors so that missing env variables are easier to debug.

}

Expand Down Expand Up @@ -134,7 +142,7 @@
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
31 changes: 31 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
services:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using python:3.11-alpine can cause build problems for packages that require compilation (e.g. psycopg2). Prefer python:3.11-slim or ensure you install and then remove Alpine build deps (gcc, musl-dev, postgresql-dev) to keep the final image thin.

app:
build:
context: .
env_file:
- .env
ports:
- "8000:8000"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Host port mapping is "8001:8000". The task requires the API to be exposed at 127.0.0.1:8000, so map "8000:8000" instead so visiting http://127.0.0.1:8000/api/ works as expected.

command: >
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

COPYing the entire project before installing dependencies prevents Docker layer caching and makes rebuilds slower. Reorder to: COPY requirements.txt .RUN pip install --no-cache-dir -r requirements.txtCOPY . ..

sh -c "python manage.py wait_for_db &&
python manage.py makemigrations &&
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use pip install --no-cache-dir -r requirements.txt to avoid leaving pip caches in image layers and reduce final image size as required by the task.

python manage.py migrate &&
python manage.py runserver 0.0.0.0:8000"
Comment on lines +9 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.

The startup command runs migrations and then runserver, but it does not call a required wait_for_db management command. Per the task, run python manage.py wait_for_db (and ensure that command is implemented) before running migrations/runserver so the app waits until Postgres is ready.

volumes:
Comment on lines +9 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 wait_for_db loop retries immediately and will busy-wait. Add import time and time.sleep(1) in the except block to pause between attempts and avoid high CPU/log spam while Postgres starts.

- my_media:/files/media
Comment on lines +14 to +15
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 project host directory into the container with ./:/app/ ties the container to the host filesystem and breaks the requirement that the service be independent of the local machine. For the final solution remove this bind mount (use an image-only approach or use named volumes for media/static only).

depends_on:
- db
Comment on lines +16 to +17
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

depends_on: - db only controls start order and does not wait for the database to become ready. This compose config must still run wait_for_db in the app startup (see previous comment) because depends_on alone won't prevent connection errors.


db:
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 switch to a non-root USER my_user. Ensure application files under /app are owned/readable by that user (chown /app) or perform root-required setup before USER so runtime operations (migrations, collectstatic, runserver) don't fail due to permission errors.

image: postgres:16.0-alpine3.17
restart: always
env_file:
- .env
Comment on lines +22 to +23
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 reference an .env file for both services. Ensure that .env contains all required variables (POSTGRES_DB, POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_PORT) and that POSTGRES_HOST is set to the DB service name (e.g. db) so Django can connect to the database inside Docker.

ports:
- "5432:5432"
volumes:
- my_db:/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.

The Postgres data volume is declared as my_db:/var/lib/postgresql/data/ (has a trailing slash). The task explicitly requires the volume be defined as my_db:/var/lib/postgresql/data — please remove the trailing slash to match the checklist exactly.


volumes:
my_db:
my_media:
6 changes: 6 additions & 0 deletions example.env
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
POSTGRES_DB=cinema
POSTGRES_USER=cinema
POSTGRES_PASSWORD=cinema
POSTGRES_HOST=db
POSTGRES_PORT=5432
PGDATA=/var/lib/postgresql/data/
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
django
psycopg
psycopg-binary
flake8
flake8-quotes
flake8-variables-names
Expand Down
Loading