Add Dockerfile and instructions#311
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has critical issues that will prevent the container from building and running correctly. Three blockers must be fixed: 1. Port mismatch (Critical): The Dockerfile EXPOSE directive declares port 8080, but the runserver command binds to port 8000 (0.0.0.0:8000). According to checklist item #7, the server must listen on 0.0.0.0:8080. Either change the runserver to 0.0.0.0:8080 and update the CMD accordingly, or change EXPOSE to 8000 — but these must match. 2. Invalid docker build syntax: In instruction.md, the command uses docker build -t todoapp:1.0.0 --name todo . — the --name flag is for container names, not image names, and cannot be used with docker build in this way. Remove --name todo entirely; the correct command is: docker build -t todoapp:1.0.0 . 3. ARG scope in multi-stage build: The second stage references ${PYTHON_VERSION} but ARGs are scoped per stage in multi-stage builds. You must redeclare ARG PYTHON_VERSION at the start of the second stage (line 5) before using it. These issues will cause the build to fail or the app to be inaccessible. Please fix these and resubmit.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| https://hub.docker.com/r/dmytro1970/todoapp:1.0.0 | ||
|
|
||
| For building: | ||
| docker build -t --name todo todoapp:1.0.0 . |
There was a problem hiding this comment.
The second stage of the multi-stage build (line 5) uses ${PYTHON_VERSION} ARG, but ARGs are scoped per stage. The ARG needs to be redeclared in the second stage or declared before all FROMs for multi-stage builds to work correctly.
| https://hub.docker.com/r/dmytro1970/todoapp:1.0.0 | ||
|
|
||
| For building: | ||
| docker build -t --name todo todoapp:1.0.0 . |
There was a problem hiding this comment.
The docker build command syntax is incorrect. The -t flag takes the image name and tag (todoapp:1.0.0), not after a --name flag. Remove --name todo from the build command.
| docker build -t --name todo todoapp:1.0.0 . | ||
|
|
||
| For run container: | ||
| docker run -d -p 8080:8000 todo |
There was a problem hiding this comment.
The port mapping is incorrect. Since the container's runserver listens on port 8000 (not 8080 as specified in requirements), the mapping should be -p 8080:8000 OR fix the runserver to use 8080 (per requirements) and use -p 8080:8080.
| COPY --from=base /app . | ||
| RUN pip install -r requirements.txt | ||
| RUN python manage.py migrate | ||
| EXPOSE 8080 |
There was a problem hiding this comment.
Port mismatch: EXPOSE declares 8080 but runserver is configured to run on port 8000. This contradicts requirement #7 which specifies 0.0.0.0:8080 for runserver. Either change runserver to 0.0.0.0:8080 or change EXPOSE to 8000.
| RUN pip install -r requirements.txt | ||
| RUN python manage.py migrate | ||
| EXPOSE 8080 | ||
| ENTRYPOINT ["python", "manage.py", "runserver", "0.0.0.0:8000"] |
There was a problem hiding this comment.
The runserver should use port 8080 to match the EXPOSE directive, or update EXPOSE to 8000. Currently, the application will be accessible on port 8000 inside the container, not 8080.
| FROM python:${PYTHON_VERSION} AS base | ||
| WORKDIR /app | ||
| COPY . . | ||
| FROM python:${PYTHON_VERSION}-slim |
There was a problem hiding this comment.
Invalid docker build syntax: The -t flag requires a value immediately after it. The --name flag is for container names, not image names. Should be: docker build -t todoapp:1.0.0 .
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation successfully addresses all critical issues from the previous review. The Dockerfile now properly implements a multi-stage build with correct ARG scoping (PYTHON_VERSION redeclared before use in the second stage), the runserver correctly binds to 0.0.0.0:8080 matching the EXPOSE directive, and the build command in INSTRUCTION.md uses valid syntax without the --name flag. All task requirements are met, including the multi-stage build structure, database migration, environment variables, and proper documentation.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.