Skip to content

Solution #284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vanyas627 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Solution #284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4c499cd
Solution
vanyas627 Apr 21, 2026
80ace12
Change INSTRUCTION.md
vanyas627 Apr 21, 2026
9c0ab8d
Solution2
vanyas627 Apr 21, 2026
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
8 changes: 8 additions & 0 deletions .idea/.gitignore
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions .idea/devops_todolist.iml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions .idea/inspectionProfiles/profiles_settings.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions .idea/misc.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions .idea/modules.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions .idea/vcs.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
ARG PYTHON_VERSION=3.12
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md is missing the docker push command to push the built image to your Docker Hub repo (todoapp:1.0.0) as required by checklist item #11 and it's missing explicit instructions to build with the ARG or specify Python version if needed. Add exact commands such as docker build -t yourdockerhubusername/todoapp:1.0.0 . and docker push yourdockerhubusername/todoapp:1.0.0. Also ensure you instruct to run with -p 8080:8080 and explain the URL (you already have that).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

Dockerfile uses ARG PYTHON_VERSION and multi-stage build which is good, but the resulting image name required by the task must be todoapp and pushed with tag todoapp:1.0.0 to your Docker Hub repository. Update INSTRUCTION.md to show build/tag/push commands that use your Docker Hub username (for example docker build -t <youruser>/todoapp:1.0.0 . and docker push <youruser>/todoapp:1.0.0).

FROM python:${PYTHON_VERSION}-slim AS build

Comment on lines +1 to +3
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile includes the required ARG for Python base image (meets checklist #7) and defines a two-stage build (build + run) which satisfies the build/run stage requirement (#6). However there are a few issues to fix:

  1. The image build command in INSTRUCTION.md and description require the image name to be todoapp:1.0.0 and pushed to Docker Hub under todoapp:1.0.0 (checklist items solution for django_todo_list #10 and Implement Docker-by-practice task #11). Your INSTRUCTION.md builds image as todoapp:1.0 (line 3). Update build and run instructions to use todoapp:1.0.0 and include push command with your Docker Hub repo (checklist Solution #12). Also ensure the Docker Hub URL in INSTRUCTION.md points to your personal repository (it currently points to vanyas627 which may be correct only if that's your account).

WORKDIR /build

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md still uses local runserver without the required host/port. The task requires python manage.py runserver 0.0.0.0:8080 for both the Docker CMD and local instructions. Update step 3 in Local Installation accordingly and include explicit URLs http://localhost:8080/ and http://localhost:8080/api/ as required by the checklist.

Comment on lines +1 to +5
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md local setup is missing an explicit statement that Python 3.8+ is required (you have a brief note but it should be explicit in the local steps) and must list the exact commands required by the task: pip install -r requirements.txt, python manage.py migrate, and python manage.py runserver 0.0.0.0:8080. Also explicitly list the URLs http://localhost:8080/ and http://localhost:8080/api/ as required by the checklist so reviewers can access both endpoints.

COPY requirements.txt .
COPY . .

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md links to DockerHub but does not contain the full build/push commands to the personal repository. Add exact commands such as:

  • docker build -t <youruser>/todoapp:1.0.0 .
  • docker push <youruser>/todoapp:1.0.0
  • docker run -p 8080:8080 <youruser>/todoapp:1.0.0
    Also ensure the README notes that Python 3.8+ is required for local steps.

FROM python:${PYTHON_VERSION}-slim as run

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md currently uses the image tag todoapp:1.0 in build/run examples. The checklist HIGH correction requires using todoapp:1.0.0 and building/pushing to your personal Docker Hub repo (e.g. youruser/todoapp:1.0.0). Replace the existing examples and show the full build/tag/push flow.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile uses ARG and multi-stage build (good) but the assignment requires the image be named todoapp pushed as yourdockeruser/todoapp:1.0.0. In INSTRUCTION.md you show vanyas627/todoapp:1.0.0 — that's acceptable if that is your Docker Hub username, but the previous review flagged that earlier you used todoapp:1.0. Ensure INSTRUCTION.md contains the exact build/tag/push commands as required, including docker push vanyas627/todoapp:1.0.0. Add the push command if missing.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md lists docker build -t vanyas627/todoapp:1.0.0 . but does not include the docker push command. The checklist requires documenting the full build/tag/push flow and the docker run -p 8080:8080 vanyas627/todoapp:1.0.0 command — add docker push vanyas627/todoapp:1.0.0 explicitly.

ENV PYTHONUNBUFFERED=1

WORKDIR /app

COPY --from=build /build /app
Comment on lines +6 to +15
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile copies the entire repository into /build in the build stage, then copies it into /app. But pip install -r requirements.txt runs in the run stage after copying files — it's fine. However to reduce image size it's common to install in the build stage and copy only installed packages or use a virtualenv; not required by checklist but worth noting.


RUN pip install --no-cache-dir -r requirements.txt
RUN python manage.py migrate
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

Dockerfile runs python manage.py migrate at build time (RUN layer) which will attempt to run migrations during image build. This commonly fails because migrations require a proper database (and often environment variables) present at build time. Checklist item #19 required executing database migration as RUN in the Dockerfile — if this is intentional, ensure the project uses SQLite and migrations can run at build time without external DB, otherwise explain in INSTRUCTION.md how to supply DB or change to running migrations at container start. If migrations cannot run at build time, this violates the requirement and will prevent the container from starting correctly.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

Dockerfile runs migrations at build time which can fail if the image build doesn't have writable DB or proper DB config. The task allows running migrations during build only if it will work with SQLite — ensure the project uses SQLite by default or move migrations to container start and document the approach in INSTRUCTION.md (checklist requires either ensuring SQLite works at build or documenting DB configuration).

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile runs python manage.py migrate during image build (line 18). The project description requires either documenting that migrations succeed during build because SQLite is used, or moving migrations to container startup. Currently INSTRUCTION.md states migrations should be run locally but doesn't clarify build-time migrations. Clarify which approach you took or move migrate to container startup (e.g., in CMD or an entrypoint) to avoid build-time DB errors if external DB is later used.


CMD ["python", "manage.py", "runserver", "0.0.0.0:8080"]
7 changes: 7 additions & 0 deletions INSTRUCTION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
DockerHub - https://hub.docker.com/repository/docker/vanyas627/todoapp/general
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile includes ARG PYTHON_VERSION and uses it in both stages — this satisfies checklist item #7. Good.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md includes a Docker Hub link which satisfies checklist item #12, but it points to vanyas627/todoapp. If that is not your personal repository, update the link to your own Docker Hub repo as required. Also the build command shown docker build -t todoapp:1.0 omits the final . — include the build context and the push command to Docker Hub per checklist items #10 and #11.


Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

The Dockerfile meets several checklist items but is missing a build stage separation: the build stage copies the entire repo into /build and later RUN pip install is executed in the run stage — functional but suboptimal. More importantly, checklist item #10 requires building an image named todoapp and #11 requires pushing to your personal Docker Hub todoapp:1.0; these are delivery steps (not encoded in the Dockerfile) and should be documented in INSTRUCTION.md. Ensure you actually executed docker build -t todoapp:1.0 . and docker push <your-dockerhub-username>/todoapp:1.0 as required.

Image: docker build -t todoapp:1.0
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md's build step should specify the exact commands required by the task: use Python 3.8+ locally to run pip install -r requirements.txt, python manage.py migrate, and the run command. The file currently only documents Docker build/run. Add the local development commands as required by checklist items #13–#17 and ensure the INSTRUCTION.md contains full build and run instructions per checklist items #20–#21.


Run Container: docker run -p 8080:8080 todoapp:1.0

Open the app in browser: http://localhost:8080
Comment on lines +3 to +14
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor Apr 21, 2026

Choose a reason for hiding this comment

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

INSTRUCTION.md local setup must explicitly state Python 3.8+ is required and must list the exact commands and URLs from the checklist. Currently you have them but be explicit about URLs for both web UI and API: http://localhost:8080/ and http://localhost:8080/api/. Add the API URL to the documentation.

Loading