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

Reproducible build with docker #344

Closed
francbartoli opened this issue Jan 16, 2020 · 21 comments
Closed

Reproducible build with docker #344

francbartoli opened this issue Jan 16, 2020 · 21 comments
Assignees
Labels
enhancement New feature or request stale Issue marked stale by stale-bot
Milestone

Comments

@francbartoli
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Using tools like poetry or pipenv the docker build could reach the state of a deterministic artifact beyond the problem to have all requirements pinned (<some package>==x.y.z)at the current version. Also, this would allow us to publish and tag them only if they pass the build on travis.

Describe the solution you'd like
I would go with poetry because it is also a good replacement of setup.py and it can manage the lifecycle of publishing the package to the pypi repository.
With poetry everything happens in a pyproject.toml, even dependencies, so we should adopt it but this might have a high impact.

With poetry:

# Upgrade pip
RUN pip install pip --upgrade
# Install poetry:
RUN pip install poetry
# Copy only requirements to cache them in docker layer
# Install pygeoapi
COPY poetry.lock /pygeoapi/poetry.lock
COPY pyproject.toml /pygeoapi/pyproject.toml
# install dependencies from poetry.lock for reproducible builds
RUN POETRY_VIRTUALENVS_CREATE=false poetry install

or use tools like dephells or poetrify to convert from requirements.txt to pyproject.toml:

dephell

dephell deps convert --from=requirements.txt --to=pyproject.toml

poetrify

poetrify generate -s requirements.txt

With pipenv:

#generate Pipfiles from requirements.txt
pipenv install -r requirements.txt

In the Dockerfile:

# Upgrade pip
RUN pip install pip --upgrade
# Install pipenv
RUN pip install pipenv

# Install pygeoapi
COPY Pipfile /pygeoapi/Pipfile
COPY Pipfile.lock /pygeoapi/Pipfile.lock
# install dependencies from Pipfile.lock for reproducible builds
RUN pipenv install --system --deploy --ignore-pipfile

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@francbartoli francbartoli added the enhancement New feature or request label Jan 16, 2020
@francbartoli francbartoli added this to the 1.0.0 milestone Mar 2, 2020
@francbartoli francbartoli self-assigned this Apr 13, 2020
@frafra
Copy link
Contributor

frafra commented Dec 14, 2022

I can work on that.
I would prefer pdm instead of Poetry. It still used pyproject.toml, but it is less buggy in my experience.

@francbartoli
Copy link
Contributor Author

Hi @frafra, let's discuss that in-depth cause I've been using poetry for three years and haven't found so many issues. I see it is widely used now, but I'm happy to see what are the benefits of pdm as compared to poetry.
I'm very happy you are working with pygeoapi btw 👏

@frafra
Copy link
Contributor

frafra commented Jan 10, 2023

Hi @francbartoli :)
I am ok with both of them; I just use pdm since I hit various bugs in Poetry that required years to be fixed. pdm also supports multiple packages group (like prod, dev, test), take the Python version into account when solving the dependencies, and it follows the newer PEPs more closely.

There is something else we could take into account: creating wheels first, and then install them, using a multi-stage Docker image, so we do not have to keep build dependencies in the final image (I briefly mentioned that here: #1093 (comment)).

@francbartoli
Copy link
Contributor Author

@frafra I would start with the migration from setup.py and the different requirements to poetry since its adoption looks broader. If we find issues it's worth moving to something more robust like pdm.
In case you can work on it I'm happy to review it. Also, I believe it's worth creating a specific ticket for that task.

@frafra
Copy link
Contributor

frafra commented Jan 10, 2023

@francbartoli I will then use pdm to convert setup.py to pyproject.toml and then adapt it to Poetry, since Poetry has no import function :-)

@frafra
Copy link
Contributor

frafra commented Jan 11, 2023

I noticed that Poetry got better, but is still lagging behind with respect to PEPs, as it does not use PEP 621. It uses its own sections, which are Poetry-specific. I wonder if it wouldn't be better to use a standard that can be understood by setuptools as well.

@francbartoli
Copy link
Contributor Author

what about hatch that looks pretty much similar to poetry and supports PEP 621?

@frafra
Copy link
Contributor

frafra commented Jan 12, 2023

I didn't know about hatch. Poetry is much similar to PDM than hatch. hatch doesn't do dependency pinning, nor it installs dependencies in a virtual env, like Poetry and PDM do.

@francbartoli
Copy link
Contributor Author

francbartoli commented Jan 12, 2023

you are right, it doesn't install/add packages and lock the dependencies indeed

@frafra frafra mentioned this issue Jan 16, 2023
2 tasks
@frafra
Copy link
Contributor

frafra commented Jan 16, 2023

I made a draft PR with pinned dependencies and an updated Dockerfile.

@justb4
Copy link
Member

justb4 commented Jan 16, 2023

@frafra Ok, see this now after my comment of #1093
#1093 (comment)

I like the MultiStage with caching Dockerfile approach! Unfamiliar with PDM, Poetry used a bit, think in FastAPI projects. Will take us (at least me) some time to grasp & review :-).
I am currently on a very slow network limited GB (4G MIFI), can't do the build now. Will further comment in #1105. Hope others find time as well.

@justb4
Copy link
Member

justb4 commented Jan 16, 2023

Question for group: should we proceed with #1093 or #1105 ? Or #1093 and then #1105? #1105 is quite involved. However we are quite backed by our unit tests (though many fail for unrelated reasons, like stale WFS URLs, .dockerignore etc), but others may be version-pin-related, like GDAL (for e.g. deegree WFS).

@francbartoli
Copy link
Contributor Author

@justb4 I guess you meant #1105, in my opinion that should happen after #1093. I believe @frafra may ask relevant changes in there before #1093 will be merged.

@frafra
Copy link
Contributor

frafra commented Jan 16, 2023

I can rebase mine based on the work that is already being done in PR #1093, which is far closer to being merged compared to #1105.

@francbartoli
Copy link
Contributor Author

Just as a reference to the direction we are taking with #1105, this is a good comparison to get an idea about the topic. To me, the only concern is about the adoption of a tool at an early stage. I'd like if other PSC's members might express their opinion

@frafra
Copy link
Contributor

frafra commented Jan 17, 2023

I agree, that is a major change.
I stumbled on this article yesterday about the state of Python packaging, which provides a great comparison of the existing tools: https://chriswarrick.com/blog/2023/01/15/how-to-improve-python-packaging/

@justb4
Copy link
Member

justb4 commented Jan 17, 2023

Ha, both links refer to the same article. @frafra and @frafra are taking us to next-level Python and Docker! Never too old to learn.

@francbartoli
Copy link
Contributor Author

Too many @fra something between us @justb4 :D

@justb4
Copy link
Member

justb4 commented Jan 17, 2023

Pff I meant @francbartoli ; GH autosuggest :-).

Copy link

As per RFC4, this Issue has been inactive for 90 days. In order to manage maintenance burden, it will be automatically closed in 7 days.

@github-actions github-actions bot added the stale Issue marked stale by stale-bot label Mar 10, 2024
Copy link

As per RFC4, this Issue has been closed due to there being no activity for more than 90 days.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale Issue marked stale by stale-bot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants