Conversation
Signed-off-by: banginji <ban.ginji@gmail.com>
Dockerfile
Outdated
|
|
||
| RUN pip install --upgrade pip | ||
| RUN pip install --no-cache-dir poetry==2.3.2 poetry-plugin-export==1.9.0 | ||
| COPY pyproject.toml poetry.lock README.md /build/ |
There was a problem hiding this comment.
suggestion (blocking): Where's this build directory coming from? There's not one in context.
| COPY pyproject.toml poetry.lock README.md /build/ | |
| COPY pyproject.toml poetry.lock README.md |
There was a problem hiding this comment.
Reverted to using src folder
| if [[ -n "${INPUT_POETRY_VERSION}" ]]; then | ||
| echo "Poetry version set to [${INPUT_POETRY_VERSION}], overriding default version" | ||
| pip install --force-reinstall poetry=="${INPUT_POETRY_VERSION}" | ||
| fi | ||
| poetry --directory /src run python3 -m diff_poetry_lock.run_poetry |
There was a problem hiding this comment.
issue: I don't think we should get rid of this. I can think of a potential use case where someone might want to use an older version of Poetry for some reason. We might not be able to go super old though, and couldn't really guarantee anything but latest.
There was a problem hiding this comment.
Added back the runtime configurable poetry version
There was a problem hiding this comment.
Something to note though. When I was testing locally, since the run_poetry.py file does import from poetry, I was seeing some issues with when there was a mismatch between runtime poetry version and the build wheels' poetry version and if the method signature changed between them the tool failed to run
There was a problem hiding this comment.
That's concerning. How, uh, far apart were the versions? I'd expect that kind of problem between 1.x and 2.x but hopefully not in e.g., 2.2.x and 2.3.x.
There was a problem hiding this comment.
Yeah it was between 1.8.5 and 2.3.2
There was a problem hiding this comment.
OK, I added that warning/disclaimer and we'll go from there. Maybe folks will think it through before they go too old, and if we get some bug reports wanting older versions of Poetry, we can figure something out.
1- Added back the capability to allow users specify the runtime poetry version 2- Reverted the use of src folder instead of build in Dockerfile Signed-off-by: banginji <7316646+banginji@users.noreply.github.com>
|
|
||
| RUN pip install poetry==2.3.2 && mkdir /src | ||
| COPY poetry.lock pyproject.toml README.md entrypoint.sh /src | ||
| RUN pip install --upgrade pip | ||
| RUN pip install --no-cache-dir poetry==2.3.2 poetry-plugin-export==1.9.0 | ||
| COPY pyproject.toml poetry.lock README.md /src/ |
There was a problem hiding this comment.
thought: It just dawned on me why you'd used build here. It was fine after all. My bad. I'll blame it on my not having had enough ☕ lifeblood yesterday when I looked at this 👼
Let's leave it src for now, no need to revert it.
| pip install --force-reinstall poetry=="${INPUT_POETRY_VERSION}" | ||
| set -euo pipefail | ||
|
|
||
| if [[ -n "${INPUT_POETRY_VERSION:-}" ]]; then |
There was a problem hiding this comment.
praise: This is a good idea, and good information for the user.
colindean
left a comment
colindean
left a comment
There was a problem hiding this comment.
This is looking great! Thanks. Address the last couple comments and this is good to ship.
Signed-off-by: Colin Dean <colindean@users.noreply.github.com>
2 participants
The Docker image is built as a multi-stage build:
poetry-plugin-exportto:poetry.locktorequirements.txtpipfrom:requirements.txtThis keeps Poetry out of the runtime/install path while retaining lockfile-pinned dependency resolution.
Fixes #40