Skip to content
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.

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

Dockerfile and INSTRUCTIONS.md #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Killreel
Copy link

No description provided.

Copy link

@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.


Your solution meets the core requirements of the task, and the code is functional and well-structured. 🎉 While there are a couple of best practice improvements you could make, such as using a .dockerignore file to prevent unnecessary files from being copied into the Docker image and handling database readiness checks with an entrypoint script, these are minor and do not critically impact the functionality of your code. Keep up the great work, and consider these suggestions for future enhancements! 🚀

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


WORKDIR /app

COPY . .

Choose a reason for hiding this comment

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

Using COPY . . can lead to unnecessary files being copied into the Docker image, increasing its size. Consider using a .dockerignore file to exclude files that are not needed in the image.


RUN pip install --no-cache-dir -r requirements.txt

RUN python manage.py migrate

Choose a reason for hiding this comment

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

Running database migrations during the build process might cause issues if the database is not ready when the container starts. Consider using an entrypoint script to handle database readiness checks and run migrations.

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