Skip to content

Dockerfile uses poetry in development and no venv in production #3

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

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

Conversation

niquerio
Copy link
Collaborator

@niquerio niquerio commented Nov 21, 2024

Dockerfile now has poetry in the development stage, meaning one can install packages with poetry in the container, and one does not have to have poetry installed locally.

The production stage runs pip install on the requirements.txt generated by poetry in the build stage.

The init.sh script has shellcheck improvements, and adds installing poetry packages.

Copy link
Collaborator

@ssciolla ssciolla left a comment

Choose a reason for hiding this comment

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

Looks good, couple minor comments which you can take or leave. I will give it a spin later and approve.

ENV PYTHONUNBUFFERED=1

# Extend the socket timeout. Default would be 15s
ENV PIP_DEFAULT_TIMEOUT=100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ENV PIP_DEFAULT_TIMEOUT=100
ENV PIP_DEFAULT_TIMEOUT=100

Comment on lines +22 to +23
# RUN apt-get update -yqq && apt-get install -yqq --no-install-recommends \
# vim-tiny
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be removed? Or do we want to add a comment explaining this is how to add packages if you need them?

Comment on lines +68 to +69
# RUN apt-get update -yqq && apt-get install -yqq --no-install-recommends \
# wget\
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove?


# Setting this to 'off' actually turns off the cache. This is set to decrease
# the size of the image.
ENV PIP_NO_CACHE_DIR=off
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ENV PIP_NO_CACHE_DIR=off
ENV PIP_NO_CACHE_DIR=off

ENV PIP_NO_CACHE_DIR=off

# Speed up pip usage by not checking for the version
ENV PIP_DISABLE_PIP_VERSION_CHECK=on
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ENV PIP_DISABLE_PIP_VERSION_CHECK=on
ENV PIP_DISABLE_PIP_VERSION_CHECK=on

COPY pyproject.toml poetry.lock README.md ./
# Ensure that the virtual environment directory is in the project. This path
# will be be `/app/.venv/`
ENV POETRY_VIRTUALENVS_IN_PROJECT=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ENV POETRY_VIRTUALENVS_IN_PROJECT=1
ENV POETRY_VIRTUALENVS_IN_PROJECT=1

# Set up our final runtime layer
FROM python:3.11-slim-bookworm as runtime
# Create the virtual environment if it does not already exist
ENV POETRY_VIRTUALENVS_CREATE=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ENV POETRY_VIRTUALENVS_CREATE=1
ENV POETRY_VIRTUALENVS_CREATE=1

* Changes Docker structure so that there is poetry in the development
  step, and pip in the production step
* Cleands up the documentation in the dockerfile
* Adds some directories and filenames to .gitignore and .dockerignore
* init.sh has shellcheck improvements and installs python packages
Copy link
Collaborator

@ssciolla ssciolla left a comment

Choose a reason for hiding this comment

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

Worked for me!

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