Skip to content

Docker#281

Open
dimonalek wants to merge 3 commits into
mate-academy:mainfrom
dimonalek:main
Open

Docker#281
dimonalek wants to merge 3 commits into
mate-academy:mainfrom
dimonalek:main

Conversation

@dimonalek
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

I am asking for changes because a couple of checklist-critical items are still not satisfied and they currently break the automated build/validation. First, the CI build fails with failed to read dockerfile: open Dockerfile: no such file or directory because your file is named DockerFile instead of Dockerfile in the repository root; please rename it to Dockerfile and update any commands in INSTRUCTION.md (e.g., docker build -f DockerFile ...) to use Dockerfile so that docker build . works as expected. Second, INSTRUCTION.md still contains the placeholder Docker Hub URL https://hub.docker.com/r/<your-dockerhub-username>/todoapp; the checklist requires a direct link to your personal repository hosting todoapp:1.0.0, so replace that placeholder with your real URL and ensure your build/push examples use the :1.0.0 tag (for example, docker build -t yourname/todoapp:1.0.0 . and docker push yourname/todoapp:1.0.0). These two issues prevent the grader from building your image and from verifying the pushed Docker Hub image, so they must be fixed before the task can be considered complete. As a minor improvement, consider adding a note in INSTRUCTION.md that running python manage.py migrate during the Docker image build can fail if the database is remote or unavailable, and optionally suggest running migrations at container startup in such environments. Overall, your Dockerfile structure and instructions look solid; addressing the filename and Docker Hub link should bring your submission in line with the task requirements.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread INSTRUCTION.md Outdated

Replace `<your-dockerhub-username>` with your Docker Hub username:

https://hub.docker.com/r/<your-dockerhub-username>/todoapp
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 instructions currently use a placeholder. Replace https://hub.docker.com/r/<your-dockerhub-username>/todoapp with the actual URL of your Docker Hub repository (a direct link to your todoapp repository is required by the task checklist).

Comment thread INSTRUCTION.md Outdated
From the project root (`devops_todolist`):

```powershell
docker build -f DockerFile -t todoapp .
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 build command tags the image as todoapp and later you re-tag it for Docker Hub. Consider adding an example build command that directly tags with the required version (e.g. docker build -f DockerFile -t <your-dockerhub-username>/todoapp:1.0.0 .) to simplify the workflow, or explicitly note that the image will be re-tagged before pushing.

Comment thread Dockerfile

ENV PYTHONDONTWRITEBYTECODE=1
ENV PYTHONUNBUFFERED=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.

INSTRUCTION.md contains a placeholder link with <your-dockerhub-username>. The checklist requires a link to your personal Docker Hub repository hosting the todoapp:1.0.0 image — replace the placeholder with your actual Docker Hub URL (for example: https://hub.docker.com/r/yourusername/todoapp).

Comment thread Dockerfile
ENV PATH="/opt/venv/bin:$PATH"

COPY requirements.txt .
RUN pip install --upgrade pip \
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 build command in the instructions references -f DockerFile. The task description explicitly asks for a file named Dockerfile. Either rename the file to Dockerfile (preferred) or clearly state in INSTRUCTION.md that the Dockerfile is named DockerFile and ensure all commands use -f DockerFile. Renaming to Dockerfile avoids confusion and matches the checklist.

Comment thread Dockerfile
&& pip install --no-cache-dir -r requirements.txt

COPY . .
RUN python manage.py migrate
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 run python manage.py migrate during the image build (RUN python manage.py migrate). This satisfies the checklist item to execute migrations as a RUN instruction, but it may fail if your app uses a remote DB (the DB is often unavailable during image build). Consider documenting this caveat in INSTRUCTION.md or, if using an external DB, run migrations at container startup instead.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because all the required Docker-related checklist items are implemented correctly and the setup should allow the ToDo app to run inside a container as specified. The Dockerfile in the repo root uses a proper multi-stage build, defines the Python base image via ARG, sets ENV PYTHONUNBUFFERED=1, runs python manage.py migrate during the build, exposes port 8080, and starts Django with runserver 0.0.0.0:8080. INSTRUCTION.md correctly documents building and pushing dimonalek/todoapp:1.0.0, links to your personal Docker Hub repository, and explains how to access the app via / and /api/ in the browser.

There are a couple of minor documentation improvements you could still make for clarity: in INSTRUCTION.md, consider changing the Docker Hub link to the tags page or a tag-specific URL so it’s obvious that the 1.0.0 image exists, and in the “Run local image” section, either use the fully qualified image name dimonalek/todoapp:1.0.0 or show a preceding docker tag dimonalek/todoapp:1.0.0 todoapp:1.0.0 command before running todoapp. These are not blockers, but addressing them would make the instructions more foolproof for graders and other users. Overall, you’ve met the core functional and documentation requirements; nice work bringing everything in line with the checklist.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread INSTRUCTION.md

## Docker Hub Repository

https://hub.docker.com/r/dimonalek/todoapp
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 linking directly to the image tag (1.0.0) or to the repository tags page so the grader/user can quickly verify the 1.0.0 image exists. Right now this points to the repo root which is fine, but a tag-specific link is clearer for validation.

Comment thread INSTRUCTION.md
From the project root (`devops_todolist`):

```powershell
docker build -t dimonalek/todoapp:1.0.0 .
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 build command tags the image as dimonalek/todoapp:1.0.0. The task also mentions building an image named todoapp — if the grader expects an image named todoapp add an additional build/tag command (for example docker build -t todoapp:1.0.0 . or docker tag dimonalek/todoapp:1.0.0 todoapp:1.0.0).

Comment thread INSTRUCTION.md
If you want to run your local image (without pull), use:

```powershell
docker run --rm -p 8080:8080 todoapp
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 example runs todoapp (unqualified). That will fail unless you have a local image named todoapp. Consider replacing this with dimonalek/todoapp:1.0.0 or show a docker tag command to create the todoapp alias locally so users can run the unqualified name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants