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
20 changes: 20 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
.idea/
.vscode/
*.iml
.DS_Store
.env
venv/
.pytest_cache/
**__pycache__/
*.pyc
**db.sqlite3
media/
.venv/
Comment on lines +1 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.

Missing collectstatic --noinput command. According to CHECKLIST ITEM #10, the Dockerfile should include collectstatic --noinput as part of the build process for proper static file handling with Docker. Add this before the USER directive: RUN python manage.py collectstatic --noinput

Dockerfile
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Static files directory is not created in Dockerfile. Add RUN mkdir -p /app/staticfiles before collectstatic to ensure the directory exists.

.git
.gitignore
backups
*.log
Comment on lines +14 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.

Critical logic bug: The while not db_conn loop will exit incorrectly. On line 17, db_conn = connections["default"] creates a connection object (not connected yet), so db_conn is truthy. The loop exits without actually verifying the database is accessible. Also, the 'Database unavailable' message (line 18-20) prints AFTER catching the error, which is backwards - it should print before the sleep.

tmp
docker-compose.yml
logs/
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.

The .dockerignore file excludes Dockerfile and docker-compose.yml from the build context. However, docker-compose.yml is not needed inside the app container (it's only for docker-compose), so this exclusion is fine. But if the Dockerfile is in the build context, it will be included - which is actually correct behavior. The exclusion may be redundant.

7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@
venv/
.pytest_cache/
**__pycache__/
*.pyc
**db.sqlite3
media
media/
.venv/
staticfiles/
*.log
logs/
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.

This line will never execute because db_conn is assigned inside the try block before reaching this line. The while condition should probably be while True with proper break condition after successful connection, or restructure the loop logic to properly detect connection success.

23 changes: 23 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
FROM python:3.11.15-alpine3.22

LABEL maintainer="yukhnenko.o@gmail.com"

ENV PYTHONUNBUFFERED=1

WORKDIR /app

COPY requirements.txt requirements.txt
RUN pip install -r requirements.txt

COPY . .
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.

Incorrect Exclusions: Dockerfile and docker-compose.yml are excluded from the Docker build context. This means when Docker builds the image, it won't have access to these files. These should NOT be in .dockerignore.

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 sequence is missing collectstatic --noinput. According to the requirements, you should use good practices for handling static files with docker. Static files won't be properly served without this step. Add it between migrate and runserver.


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 Bug: The while loop while not db_conn will never exit after database becomes available. db_conn is assigned BEFORE the try block that might raise OperationalError. After the first exception, db_conn is set and remains set forever. The loop should use an infinite loop with while True and break when connection succeeds, or reset db_conn = None in the except block.

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

RUN chown -R 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.

The wait_for_db command needs to break out of the loop after successful connection. After the try block succeeds (database is available), the loop should exit with break, otherwise it will loop forever.


USER my_user
Empty file added cinema/management/__init__.py
Empty file.
Empty file.
26 changes: 26 additions & 0 deletions cinema/management/commands/wait_for_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
from django.core.management.base import BaseCommand
from django.db import connections
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.yml service is named cinema but the task requirements specify the service should be named app (checklist item #11 references 'app' service).

from django.db.utils import OperationalError
import time


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

def handle(self, *args, **options):
self.stdout.write("Waiting for database...", ending="")
Comment on lines +8 to +11
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 command sequence is missing python manage.py collectstatic --noinput which is required for proper static files handling with Docker (checklist item #10). Add this between migrate and runserver.


db_conn = None
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 .dockerignore file excludes Dockerfile and docker-compose.yml from the build context. These files are needed by docker-compose to build the image. Remove these two entries from .dockerignore.


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 while loop will run indefinitely if db_conn is never None. Consider adding a max_attempts limit or removing the while not db_conn since the connection object exists but might not be usable.

while not db_conn:
try:
db_conn = connections["default"]
self.stdout.write(
"Database unavailable, waiting for a 1 second",
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 PGDATA environment variable is referenced here but not defined anywhere. Either define it in the .env file or use a named volume directly without $PGDATA.

ending=""
)
Comment on lines +18 to +26
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 has incorrect logic. The message 'Database unavailable, waiting for a 1 second' is printed AFTER a successful connection is established, not when connection fails. The cursor() call that might raise OperationalError should be inside the try block before the success message.

db_conn.cursor()
except OperationalError:
time.sleep(1)

self.stdout.write(self.style.SUCCESS("Database available!"))
19 changes: 12 additions & 7 deletions cinema_service/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@
For the full list of settings and their values, see
https://docs.djangoproject.com/en/4.0/ref/settings/
"""
Comment on lines 9 to 11
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 command sequence is missing collectstatic --noinput which is needed for checklist item #10 (handling static files with Docker). Add this command between migrate and runserver: python manage.py collectstatic --noinput &&

Comment on lines 9 to 11
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 sequence is missing collectstatic --noinput. According to the requirements (CHECKLIST ITEM #10), you should use good practices for handling static files with docker. Static files won't be properly collected without this step. Add python manage.py collectstatic --noinput && between migrate and runserver commands.

import os
from dotenv import load_dotenv
from datetime import timedelta
from pathlib import Path

load_dotenv()

# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent

Expand All @@ -20,9 +24,7 @@
# See https://docs.djangoproject.com/en/4.0/howto/deployment/checklist/

# SECURITY WARNING: keep the secret key used in production secret!
SECRET_KEY = (
"django-insecure-6vubhk2$++agnctay_4pxy_8cq)mosmn(*-#2b^v4cgsh-^!i3"
)
SECRET_KEY = os.getenv("DJANGO_SECRET_KEY")
Comment on lines 26 to +27
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 volume mapping uses $PGDATA but this environment variable is not defined in the docker-compose file or .env. PostgreSQL image defaults to /var/lib/postgresql/data but referencing undefined $PGDATA may cause issues. Either define PGDATA environment variable or remove $PGDATA and use the default path.


# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = True
Expand Down Expand Up @@ -86,8 +88,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 @@ -132,9 +138,8 @@
# https://docs.djangoproject.com/en/4.0/howto/static-files/

STATIC_URL = "static/"

MEDIA_URL = "/media/"
MEDIA_ROOT = BASE_DIR / "media"
MEDIA_ROOT = "/files/media"
Comment on lines 141 to +143
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

settings.py is missing STATIC_ROOT configuration. While Django will use BASE_DIR / "staticfiles" as default, explicitly setting STATIC_ROOT is a best practice for Docker deployments. Add: STATIC_ROOT = BASE_DIR / "staticfiles" near line 139.


# 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.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
services:
cinema:
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 but the task requirements (checklist item #11) specifically reference the service as app. Rename the service from cinema to app to match the requirements and ensure consistency with the expected service name.

build:
context: .
env_file:
- .env
ports:
- "8000:8000"
command: >
sh -c "python manage.py wait_for_db &&
python manage.py migrate &&
Comment on lines +9 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing collectstatic --noinput in the startup command. To follow good practices for handling static files with Docker, add this step between migrate and runserver. Otherwise static files won't be served properly.

python manage.py runserver 0.0.0.0:8000"
volumes:
- my_media:/files/media
depends_on:
- db


db:
image: postgres:16-alpine3.22
restart: always
env_file:
- .env
ports:
- "5432:5432"
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.

The database volume mapping uses $PGDATA environment variable (line 26), but this variable is not defined anywhere in the docker-compose file or .env. PostgreSQL Docker image uses /var/lib/postgresql/data as the default PGDATA path internally. Either define PGDATA environment variable or remove the $PGDATA suffix and let PostgreSQL use its default.

- my_db:$PGDATA

volumes:
my_db:
my_media:
4 changes: 4 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ djangorestframework
djangorestframework-simplejwt
drf-spectacular
Pillow
python-dotenv
psycopg
psycopg-binary

Loading