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
4 changes: 4 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.venv
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.

Critical: Dockerfile should NOT be in .dockerignore. The Dockerfile is needed inside the Docker build context to construct the image. Remove this line.

.idea
docker-compose.yaml
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 typo: PYTHOUNNBUFFERED should be PYTHONUNBUFFERED (missing 'N' after 'PYTHOU'). This environment variable ensures Python output is not buffered, which is important for Docker logs.

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.yaml file is referenced in task requirements but not provided. This file is essential for managing multiple services (app + db) with docker-compose up.

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.6-alpine3.18
LABEL maintainer="tanakutova74@gmail.com"
Comment on lines +1 to +2
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 being in .dockerignore will cause Docker build to fail. Docker needs the Dockerfile in the build context to execute. Remove 'Dockerfile' from .dockerignore.


ENV PYTHOUNNBUFFERED 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.

Typo: 'PYTHOUNNBUFFERED' should be 'PYTHONUNBUFFERED'. This environment variable is currently being ignored.


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.

The OperationalError is imported on line 4 but never used. The db_conn.cursor() call on line 12 will raise OperationalError when the database is unavailable, crashing the app instead of waiting. Wrap this in a try/except block to properly catch connection failures.

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

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

The wait logic needs to be restructured: call db_conn.cursor() inside a try block, catch OperationalError, print the waiting message, sleep, and continue the loop. Only break when successful.

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

USER my_user
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.

Service is named 'cinema' but requirements specify 'app' service (used in verification commands like docker ps)


from django.core.management import BaseCommand
from django.db import connections, OperationalError
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo: should be PYTHONUNBUFFERED not PYTHOUNNBUFFERED for proper log output in Docker

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 OperationalError import is unused - it's not being caught by the current try-except block. Consider using it to catch specific database connection errors for more robust error handling.



class Command(BaseCommand):
def handle(self, *args, **options):
self.stdout.write("waiting for db...")
db_conn = None
while not db_conn:
try:
db_conn = connections["default"]
db_conn.cursor()
except OperationalError:
self.stdout.write(
"Database unavailable, waiting for 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.

Missing $PGDATA environment variable for PostgreSQL data directory. Should add PGDATA: /var/lib/postgresql/data/pgdata under db environment variables to properly persist database data in the volume

time.sleep(1)
self.stdout.write(self.style.SUCCESS("Database available!"))
11 changes: 8 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
https://docs.djangoproject.com/en/4.0/ref/settings/
"""
import os
Comment on lines 10 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.

The OperationalError is imported but never used. The db_conn.cursor() call on line 12 will raise OperationalError when the database is unavailable, causing the script to crash. Wrap this in a try-except block to catch the exception and retry.

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 @@ -134,7 +139,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
33 changes: 33 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
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.

Service is named cinema but the task verification mentions checking app service with docker ps. Consider renaming to app for consistency with expected output in verification steps.

build:
context: .
env_file:
- .env
ports:
- "8001:8000"
volumes:
- ./:/app
- my_media:/files/media
command: >
sh -c " python manage.py wait_for_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.

The wait_for_db command has a logic issue - connections["default"] returns a wrapper object even when the database is unavailable. The loop will exit after one iteration without actually verifying the connection. Consider using while True with break on success instead.

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

The wait_for_db loop only calls time.sleep(1) in the except block. If connections["default"] raises an exception on every iteration, this works, but the logic is fragile. A cleaner approach would be while True: try: connections["default"].cursor(); break except OperationalError: 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 wait_for_db implementation uses time.sleep(1) in the exception handler but this means it only sleeps when an error occurs. The initial while not db_conn check might pass immediately if connections["default"] returns a truthy object. Consider restructuring to ensure proper retry behavior.


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

volumes:
my_db:
my_media:

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>=2.9.5
Loading