build: Improve Docker image and add version in hash.txt #119
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @blarghmatey, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on improving the Docker image build process and adding version information to the hash.txt file. The changes include integrating hadolint for Dockerfile linting, updating the Dockerfile to use more efficient commands and include version information in the hash.txt file, which is intended to allow Doof to properly detect production deployments.
Highlights
- Dockerfile Linting: The pull request introduces
hadolintto the pre-commit configuration for linting the Dockerfile, ensuring best practices and code quality. - Dockerfile Improvements: The Dockerfile is updated to use more efficient commands, such as combining
apt-get updateandapt-get installinto a single command, removing unnecessary files, and setting the shell to bash with pipefail. The poetry version is also updated from 1.7.1 to 1.8.5. - Version Information in hash.txt: The pull request adds the
RELEASE_VERSIONas an argument to the Docker build process, and includes both the version and git hash in the/src/static/hash.txtfile in JSON format. This change is intended to allow Doof to properly detect production deployments.
Changelog
Click here to see the changelog
- .pre-commit-config.yaml
- Added
hadolint-dockerto the pre-commit hooks to lint the Dockerfile. - Added
hadolintas a pre-commit repository with a hook forhadolint-docker.
- Added
- Dockerfile
- Added a
hadolintglobal ignore directive. - Updated the
LABELtoorg.opencontainers.image.authors. - Set the shell to bash with pipefail.
- Combined
apt-get updateandapt-get installcommands for efficiency and added--no-install-recommends. - Added commands to clean up apt lists and cache after installing packages.
- Updated the poetry version from 1.7.1 to 1.8.5.
- Added
PYTHON_UNBUFFERED=1environment variable. - Modified the installation of poetry to use
--no-cache-dir. - Combined multiple RUN commands using
&&to reduce the number of layers. - Removed the collectstatic command and related environment variables.
- Added
RELEASE_VERSIONas an argument to the Docker build process. - Modified the creation of
hash.txtto include both the version and git hash in JSON format. - Set the
PORTenvironment variable using a simple assignment.
- Added a
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Docker's a whale,
Linting keeps it from fail,
Versions in the hash,
No more deployment clash,
Doof's happy, all's well.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces improvements to the Docker image build process, including the addition of version information to the hash.txt file and the integration of hadolint for Dockerfile linting. The changes appear to be well-structured and contribute to better image management and code quality. However, there are a few points that require closer attention to ensure correctness and maintainability.
Summary of Findings
- Missing RELEASE_VERSION argument: The Dockerfile introduces a new argument
RELEASE_VERSIONbut it's not clear how this argument is being passed during the build process. It's crucial to ensure that this argument is properly set when building the Docker image to ensure the version information is correctly included in thehash.txtfile. - Removal of collectstatic command: The pull request removes the
collectstaticcommand and related environment variables. It's important to verify that static files are still being collected and served correctly, as this could impact the application's functionality.
Merge Readiness
The pull request introduces valuable improvements to the Docker image build process. However, the missing RELEASE_VERSION argument and the removal of the collectstatic command need to be carefully reviewed and addressed before merging. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issues should be addressed before merging.
| ARG GIT_REF | ||
| RUN mkdir -p /src/static | ||
| RUN echo $GIT_REF >> /src/static/hash.txt | ||
|
|
||
| # Run collectstatic | ||
| ENV DATABASE_URL="postgres://postgres:postgres@localhost:5433/postgres" | ||
| ENV MITOL_SECURE_SSL_REDIRECT="False" | ||
| ENV MITOL_DB_DISABLE_SSL="True" | ||
| ENV MITOL_FEATURES_DEFAULT="True" | ||
| ENV CELERY_TASK_ALWAYS_EAGER="True" | ||
| ENV CELERY_BROKER_URL="redis://localhost:6379/4" | ||
| ENV CELERY_RESULT_BACKEND="redis://localhost:6379/4" | ||
| ENV MITOL_APP_BASE_URL="http://localhost:8002/" | ||
| ENV MAILGUN_KEY="fake_mailgun_key" | ||
| ENV MAILGUN_SENDER_DOMAIN="other.fake.site" | ||
| ENV MITOL_COOKIE_DOMAIN="localhost" | ||
| ENV MITOL_COOKIE_NAME="cookie_monster" | ||
| RUN python3 manage.py collectstatic --noinput --clear | ||
|
|
||
| RUN apt-get clean && apt-get purge | ||
| ARG RELEASE_VERSION |
There was a problem hiding this comment.
It's good to see both GIT_REF and RELEASE_VERSION being used. However, ensure that RELEASE_VERSION is properly defined and passed during the build process. Without it, the version field in hash.txt will be empty.
| @@ -7,6 +7,7 @@ ci: | |||
| # Because these are local hooks it seems like they won't easily run in pre-commit CI | |||
| - eslint | |||
| - style-lint | |||
| - hadolint-docker | |||
There was a problem hiding this comment.
Adding hadolint-docker to the ci section is a good step to ensure Dockerfile linting in CI. However, it's worth noting that this hook might not run locally unless the necessary environment is set up. Consider adding a comment to clarify this for developers.
| RUN mkdir -p /src/static \ | ||
| && echo "{\"version\": \"$RELEASE_VERSION\", \"hash\": \"$GIT_REF\"}" >> /src/static/hash.txt |
There was a problem hiding this comment.
Consider using jq to create the JSON file for better readability and maintainability. This would also allow for easier addition of new fields in the future.
ARG RELEASE_VERSION
RUN mkdir -p /src/static \
&& echo "{\"version\": \"$RELEASE_VERSION\", \"hash\": \"$GIT_REF\"}" | jq . > /src/static/hash.txt
| RUN mkdir -p /src/static \ | ||
| && echo "{\"version\": \"$RELEASE_VERSION\", \"hash\": \"$GIT_REF\"}" >> /src/static/hash.txt |
There was a problem hiding this comment.
This line creates the hash.txt file. Consider adding a check to ensure that the file is created successfully and contains the expected content. This can help prevent issues related to missing or malformed version information.
ARG RELEASE_VERSION
RUN mkdir -p /src/static \
&& echo "{\"version\": \"$RELEASE_VERSION\", \"hash\": \"$GIT_REF\"}" > /src/static/hash.txt \
&& cat /src/static/hash.txt
In mitodl/release-script#400 Doof gains the ability to parse versions. This adds the necessary information in the hash.txt to allow Doof to properly detect production deployments of this app.
blarghmatey
force-pushed
the
set_version_for_doof
branch
from
c69e77b to
53069bd
Compare
What are the relevant tickets?
N/A
Description (What does it do?)
In mitodl/release-script#400 Doof gains the ability to parse versions. This adds the necessary information in the hash.txt to allow Doof to properly detect production deployments of this app.
How can this be tested?
Deploy to RC and verify that Doof detects the released app properly after PR 400 gets merged and deployed.