fix: Updating/fixing ollama url, now with zst compression#481
fix: Updating/fixing ollama url, now with zst compression#481marazik wants to merge 2 commits intoAsyncFuncAI:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical build failure in the Docker image creation process by rectifying an outdated Ollama download URL. It also modernizes the download mechanism by adopting Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the Dockerfile for the local Ollama setup to use a new download URL and handle .zst compressed archives. The changes correctly address the broken URL issue. I've provided two suggestions for improvement: one to make the download and extraction process more robust against failures, and another to optimize the Docker image size by cleaning up the apt cache. These are both best practices for writing Dockerfiles.
Dockerfile-ollama-local
Outdated
| curl -L "https://ollama.com/download/ollama-linux-${OLLAMA_ARCH}.tar.zst" \ | ||
| | zstd -d | tar -x -C /usr |
There was a problem hiding this comment.
The pipeline curl | zstd | tar can fail silently if an intermediate command fails. For example, if curl encounters an HTTP error (like a 404), the build step might still succeed incorrectly. To make the build more robust, use set -o pipefail to ensure the pipeline fails if any command within it fails. Additionally, adding the -f flag to curl will make it exit with an error on server errors.
(set -o pipefail; curl -fL "https://ollama.com/download/ollama-linux-${OLLAMA_ARCH}.tar.zst" | zstd -d | tar -x -C /usr)
Dockerfile-ollama-local
Outdated
| FROM python:3.11-slim AS ollama_base | ||
| RUN apt-get update && apt-get install -y \ | ||
| curl | ||
| curl zstd |
There was a problem hiding this comment.
To reduce Docker image size, it's a best practice to clean up the apt cache within the same RUN layer where packages are installed. You can also use --no-install-recommends to avoid installing unnecessary packages. The RUN command starting on line 28 should be updated to include these optimizations. For example: RUN apt-get update && apt-get install -y --no-install-recommends curl zstd && rm -rf /var/lib/apt/lists/*
…ls if any command within it fails
This PR fixes the issue of Ollama url is broken, resulting that the docker build fails to create an image.
Fixes and closes #480