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 @@
.git
.venv
__pycache__
*.pyc
db.sqlite3
media/
staticfiles/
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 can be made thinner: add --no-install-recommends to apt-get install to avoid extra packages, and consider removing build-time dependencies after pip install or use a multi-stage build so tools like gcc/libpq-dev aren't left in the final image.

.env
29 changes: 6 additions & 23 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,28 +1,11 @@
name: Test
name: Docker Test
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider pinning the base image more precisely (for example python:3.11-slim-bullseye or a specific patch version) to avoid unexpected changes between builds and to improve reproducible builds.


on:
pull_request:
branches:
- "master"
on: [push]

jobs:
test:
build:
runs-on: ubuntu-latest
timeout-minutes: 10

steps:
- name: Checkout repo
uses: actions/checkout@v2

- name: Set Up Python 3.10
uses: actions/setup-python@v2
with:
python-version: "3.10"

- name: Install requirements
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt

- name: Run flake8
run: flake8
- uses: actions/checkout@v3
- name: Build and Run
Comment on lines 7 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

When installing build-time packages use --no-install-recommends to reduce installed extras, and consider removing build dependencies after pip install or using psycopg2-binary in requirements to avoid needing gcc/libpq-dev. This will substantially reduce image size as required by the task.

run: docker compose build && docker compose up -d
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,9 @@ venv/
**__pycache__/
**db.sqlite3
media
.venv
.git
.gitignore
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 psycopg2, consider using psycopg2-binary to avoid the need for gcc/libpq-dev at build time, or use a multi-stage build where build dependencies are removed in the final image. This helps keep the runtime image small.

__pycache__
*.pyc
db.sqlite3
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rather than copying the whole repo and then running chown -R, consider using COPY --chown=django-user:django-user . . so ownership is set at copy time and you can remove the expensive chown -R step (makes the image build faster and layer-smaller). Also ensure your .dockerignore excludes development artifacts (you have one, good).

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

ENV PYTHONUNBUFFERED=1
WORKDIR /app


RUN apt-get update && apt-get install -y \
gcc \
libpq-dev \
&& rm -rf /var/lib/apt/lists/*
Comment on lines +7 to +10
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 --no-install-recommends to the apt-get install command (and consider cleaning up build-time packages after pip install or using a multi-stage build). This reduces image size by avoiding unnecessary recommended packages and allows you to remove build dependencies (gcc/libpq-dev) after they are no longer needed.


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


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.

COPY . . will copy all files and then you run a chown later. Prefer using the COPY --chown=django-user:django-user form to set ownership during copy (or create the user before COPY), which avoids an expensive recursive chown at build time.



RUN adduser --disabled-password --no-create-home django-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.

You create the non-root user after copying files. If you plan to use COPY --chown (recommended), create the user earlier so the chown can target the correct user during the COPY step.



RUN chown -R django-user:django-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 explicit recursive chown is expensive and can be avoided if you use COPY --chown or if the user is created before copying files. Consider removing this step after changing COPY to set ownership.



USER django-user
15 changes: 11 additions & 4 deletions cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"""
from datetime import timedelta
from pathlib import Path
import os

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent
Expand All @@ -27,7 +28,7 @@
# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True

ALLOWED_HOSTS = []
ALLOWED_HOSTS = os.environ.get("ALLOWED_HOSTS", "127.0.0.1,localhost").split(",")

Comment on lines +31 to 32
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 only controls start order and does not guarantee the DB is ready. You already run wait_for_db in the app command which is good; as an improvement consider adding a healthcheck for the db service and using depends_on: condition: service_healthy (Compose v3.4+) so Compose can reason about service health explicitly.

INTERNAL_IPS = [
"127.0.0.1",
Expand All @@ -47,6 +48,7 @@
"debug_toolbar",
"cinema",
"user",
"core",
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 file still uses SQLite (see DATABASES block) which prevents the app from using the PostgreSQL service defined in docker-compose. Update DATABASES to use the PostgreSQL engine (django.db.backends.postgresql) and read connection parameters from environment variables (DB_HOST, DB_NAME, DB_USER, DB_PASS, optional DB_PORT default 5432) so the app connects to the db container. Also set ALLOWED_HOSTS to include the host(s) you will access (e.g. ['127.0.0.1', 'localhost'] or ['*'] for dev) so runserver 0.0.0.0:8000 is accessible. Finally, add STATIC_ROOT (for example: STATIC_ROOT = BASE_DIR / 'staticfiles') so you can run collectstatic and serve static files from a volume in Docker; MEDIA_ROOT is present but confirm it matches your docker volumes. Optionally, consider sourcing SECRET_KEY and DEBUG from environment variables for better Docker practice.

]

MIDDLEWARE = [
Expand Down Expand Up @@ -85,9 +87,13 @@
# https://docs.djangoproject.com/en/4.0/ref/settings/#databases

DATABASES = {
"default": {
"ENGINE": "django.db.backends.sqlite3",
"NAME": BASE_DIR / "db.sqlite3",
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': os.environ.get('DB_NAME', 'cinema_db'),
'USER': os.environ.get('DB_USER', 'cinema_user'),
'PASSWORD': os.environ.get('DB_PASS', 'secretpassword'),
'HOST': os.environ.get('DB_HOST', 'db'),
'PORT': os.environ.get('DB_PORT', '5432'),
}
}

Expand Down Expand Up @@ -132,6 +138,7 @@
# https://docs.djangoproject.com/en/4.0/howto/static-files/

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

MEDIA_URL = "/media/"
MEDIA_ROOT = BASE_DIR / "media"
Expand Down
Empty file added core/__init__.py
Empty file.
Empty file added core/management/__init__.py
Empty file.
Empty file.
21 changes: 21 additions & 0 deletions core/management/commands/wait_for_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import time
from django.core.management.base import BaseCommand
from django.db import connections
from django.db.utils import OperationalError

class Command(BaseCommand):
"""Django command to pause execution until database is available"""

Comment on lines +5 to +8
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

docker-compose defines postgres but does not expose a DB port to the host (not necessary for container-to-container comms) — fine. However ensure your Django settings read the DB env vars defined below (DB_HOST, DB_NAME, DB_USER, DB_PASS) so the app will actually connect to this postgres service.

def handle(self, *args, **options):
self.stdout.write('Waiting for database...')
Comment on lines +7 to +10
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 apt-get install line should use --no-install-recommends to avoid pulling unnecessary packages. Also consider removing build-time packages after pip install or use a multi-stage build (or psycopg[binary]) to keep the final image thin.

db_conn = None
while not db_conn:
try:
connection = connections['default']
connection.cursor()
db_conn = 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.

You COPY the entire project and then run a recursive chown -R, which creates an extra expensive layer. Prefer creating the user before COPY and use COPY --chown=django-user:django-user . . to set ownership during copy and avoid the chown step.

except OperationalError:
self.stdout.write('Database unavailable, waiting 1 second...')
time.sleep(1)
Comment on lines +12 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.

Consider limiting the retry behavior or adding a timeout/logging: currently this loops indefinitely with 1s sleeps. That's acceptable for many setups, but you may want to surface a clear error after N attempts for easier debugging.


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For production deployments avoid bind-mounting the project source into the container (the .:/app mount). Baking code into the image makes the deployment self-contained; keep bind mounts only for local development to allow live code edits.

self.stdout.write(self.style.SUCCESS('Database available!'))
37 changes: 37 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
services:
db:
image: postgres:15-alpine
restart: always
environment:
POSTGRES_DB: cinema_db
POSTGRES_USER: cinema_user
POSTGRES_PASSWORD: secretpassword
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 expose DB credentials directly in the compose file (POSTGRES_PASSWORD for the db and DB_PASS for the app). Avoid hardcoding secrets in the repository; consider using an .env file (kept out of VCS), Docker secrets, or CI/CD injection instead.

volumes:
- postgres_data:/var/lib/postgresql/data

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.

Consider adding a restart policy (for example restart: always or restart: unless-stopped) and/or a healthcheck for the app service so it has predictable behavior if the container stops or the DB becomes temporarily unavailable.

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.

For a fully self-contained deployment (one of the task requirements) avoid relying on a host bind-mount for the application code. Keep build: . and remove or make the .:/app mount optional for local development so the baked image contains the app code.

# Команда, яка чекає базу, мігрує базу і запускає сервер
command: >
sh -c "python manage.py wait_for_db &&
python manage.py migrate &&
python manage.py runserver 0.0.0.0:8000"
Comment on lines +16 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.

Good: the app command runs python manage.py wait_for_db before migrations and starting the server, which ensures the service will wait for the DB rather than fail on startup.

volumes:
- .:/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 app service bind-mounts the project directory from the host (- .:/app). That makes the container depend on local files and breaks the requirement that Docker alone should be sufficient. For a self-contained deployment, avoid bind mounts so the image includes the code (or make mounts optional for development only).

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 host bind mount .:/app overrides the image contents and prevents the image from being self-contained. Remove this mount for production deployments (or make it conditional for local dev) so the service depends only on the built image and Docker volumes.

- static_data:/app/staticfiles
- media_data:/app/media
ports:
- "8000:8000"
Comment on lines +19 to +24
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is no named volume for media/static files. Add volumes for media/ (and use STATIC_ROOT + collectstatic) so media and static can persist in Docker, for example a named volume like - media_data:/app/media and - static_data:/app/staticfiles.

environment:
- DB_HOST=db
- DB_NAME=cinema_db
- DB_USER=cinema_user
- DB_PASS=secretpassword
Comment on lines +26 to +29
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 receives DB environment variables (DB_HOST, DB_NAME, DB_USER, DB_PASS). Ensure your Django settings actually read these variables and configure the PostgreSQL backend accordingly; otherwise the app will not connect to the postgres service at runtime.

Comment on lines +25 to +29
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Database credentials are hardcoded in the environment block. Move these values to an .env file or Docker secrets to avoid committing sensitive information and to follow the task suggestion to avoid hardcoding credentials.

- ALLOWED_HOSTS=127.0.0.1,localhost
depends_on:
- db
Comment on lines +31 to +32
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 ensures service start order but does not wait for the DB to be ready; your command runs wait_for_db which addresses this. As an enhancement you can also add a short healthcheck for the db service so orchestration and tools can detect readiness more explicitly.


volumes:
postgres_data:
static_data:
media_data:
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ djangorestframework
djangorestframework-simplejwt
drf-spectacular
Pillow
psycopg2-binary