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

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.
18 changes: 18 additions & 0 deletions cinema/management/commands/wait_for_db.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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...")
while True:
db_conn = connections["default"]
db_conn.cursor()
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 try-except block should wrap db_conn.cursor() (line 11) to catch connection errors and allow retry. Currently, if the database is unavailable on the first iteration, an exception will be raised before reaching time.sleep(1), preventing retries.

if db_conn is not None:
break
self.stdout.write(
"Database unavailable, waiting for 1 second...")
time.sleep(1)
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 time.sleep(1) (line 17) will never be reached if the database is unavailable on first try, as an exception will be raised at line 11 before this line. The sleep should be in the except block to implement proper retry behavior.

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
35 changes: 35 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
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
environment:
PGDATA: /var/lib/postgresql/data/pgdata
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