-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
1136 docker build is broken #1159
Conversation
+ pyarrow build breaks in Linux Alpine
Thanks for taking the lead on this, @captivus! @ATheorell and @TheoMcCabe - would appreciate your review on this so we can merge this shortly. Also, @zigabrencic if you have any comments, more than welcome to share. |
Looks good to me since all the points from Issue 1136 discussion are considered. I tested it on MacOS and it works. Once people run it on Linux and Windows we can merge this as far as I'm concerned. P.S On MacOS the image is now 759.07 MB so it's also smaller. |
FROM python:3.11-alpine | ||
FROM python:3.11-slim | ||
|
||
RUN apt-get update && apt-get install -y --no-install-recommends git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This git installation is not followed by the best practice of using a multi-stage build in Docker. The purpose of multi-stage builds is to reduce image size, improve security, simplify the build process, and enhance maintainability. I have posted a proper Dockerfile in my last comment on #1159
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the comment you're linking to, @k1lgor, as you're referencing this same PR. I'm not a Docker expert, so I'd welcome guidance if you have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the Docker file I have posted:
# Stage 1: Builder stage
FROM python:3.11-slim AS builder
RUN apt-get update && apt-get install -y --no-install-recommends \
tk \
tcl \
curl \
git \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /app
COPY . .
RUN pip install --no-cache-dir -e .
# Stage 2: Final stage
FROM python:3.11-slim
WORKDIR /app
COPY --from=builder /usr/local/lib/python3.11/site-packages /usr/local/lib/python3.11/site-packages
COPY --from=builder /usr/local/bin /usr/local/bin
COPY --from=builder /usr/bin /usr/bin
COPY --from=builder /app .
COPY docker/entrypoint.sh .
ENTRYPOINT ["sh", "/app/entrypoint.sh"]
The difference is that the git is installed at the building stage instead of the final, and since git and the other packages are installed via the OS package manager, the binaries are located in the /usr/bin/
folder and just copy that folder in the final stage. This allows gpte
to use git
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, I see. I've just built the container using the file you've provided and ... it works!
I wasn't trying to front run your solution with this PR. I'll kindly offer a bit of reciprocal best practice advice for submitting solutions to issues. ;) The best practice is to submit a pull request containing your solution and then solicit feedback on the PR directly, rather than pasting code into an issues discussion.
We'd welcome this PR from you and, given that your solution predates mine, I'm happy to close this PR (and will do so)! Will you please submit a pull request today? We discussed this issue during our dev meeting yesterday and were keen to resolve it as it's been broken for longer than we'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing -- please update the README for the Docker container in a similar manner as I have. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sure. I will prep the PR as soon as possible.
To be clarify, |
Yes, I had written a previous version of the Dockerfile which successfully compiled |
Closing, having requested a PR from @k1lgor. |
``` | ||
|
||
2. **Build and Run using Docker Compose** | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some alternatives using docker compose
:
Since there is only one docker-compose.yml
file, you could run it without the -f
option.
docker compose up -d --build
- Build images before starting containers, when they are built, run the containers in the backgrounddocker compose up -d
- Only start the containers in the backgrounddocker compose down
- Stops containers and removes containers, and networksdocker compose down -v
- Removes the volumes as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! Please incorporate in the updated README in your forthcoming PR.
After doing some digging, I see, now, @k1lgor, that you are the great community member who originally rewrote the Dockerfile for this a couple of months back to make it lean and mean! Please do build upon your prior good work. Also, we'd love to have you contribute more broadly to the project, if you're keen. We meet every Thursday at 19:00 CEST on Discord to collaborate. Will you join us? |
@captivus Sure, I'd love to contribute beyond my full-time job. Although I may not be very active on Discord, I'll be happy to join the Thursday meetings whenever possible. |
This solves issue #1136 by migrating the Docker image to use
python:3.11-slim
, as buildingpyarrow
is broken in Alpine Linux.I'm not aware of the specific reason that
apk
was chosen as the previous image, but this PR replaces it with a working container that successfully executesgpt-engineer
CLI.Added
langchain-community
to Poetry to resolve the application (container) crashing upon startup.We should discuss container optimization, if that remains a concern.