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

1136 docker build is broken #1159

Closed
wants to merge 3 commits into from
Closed

Conversation

captivus
Copy link
Collaborator

This solves issue #1136 by migrating the Docker image to use python:3.11-slim, as building pyarrow 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 executes gpt-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.

@captivus captivus linked an issue May 23, 2024 that may be closed by this pull request
@captivus captivus requested review from ATheorell and TheoMcCabe May 23, 2024 23:41
@viborc
Copy link
Collaborator

viborc commented May 24, 2024

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.

@zigabrencic
Copy link
Collaborator

zigabrencic commented May 24, 2024

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
Copy link
Contributor

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

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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!

Copy link
Contributor

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.

@k1lgor
Copy link
Contributor

k1lgor commented May 24, 2024

This solves issue #1136 by migrating the Docker image to use python:3.11-slim, as building pyarrow 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 executes gpt-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.

To be clarify, pyarrow is supported on Alpine, it lacks a wheel. In other words, you need to build it from source.

@captivus
Copy link
Collaborator Author

This solves issue #1136 by migrating the Docker image to use python:3.11-slim, as building pyarrow 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 executes gpt-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.

To be clarify, pyarrow is supported on Alpine, it lacks a wheel. In other words, you need to build it from source.

Yes, I had written a previous version of the Dockerfile which successfully compiled pyarrow from source but it would not install as a Python package. After some time mucking about trying to resolve that, I decided to change the distro for to enable a prompt resolution of this issue that has been open for longer than we'd like.

@captivus
Copy link
Collaborator Author

Closing, having requested a PR from @k1lgor.

@captivus captivus closed this May 24, 2024
```

2. **Build and Run using Docker Compose**
```
Copy link
Contributor

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 background
  • docker compose up -d - Only start the containers in the background
  • docker compose down - Stops containers and removes containers, and networks
  • docker compose down -v - Removes the volumes as well

Copy link
Collaborator Author

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.

@captivus
Copy link
Collaborator Author

captivus commented May 24, 2024

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?

@k1lgor k1lgor mentioned this pull request May 29, 2024
@k1lgor
Copy link
Contributor

k1lgor commented May 29, 2024

@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.

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.

Docker build is broken
4 participants