fix: docker-compose-image-tag fails to start #31606
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Recently I did some work to use `uv pip install` in place of plain `pip` in our docker images. This made builds significantly faster, but broke compatibility between some docker compose setups. The issue is around the fact that `docker-compose-image-tag` mounts the local, newer, docker bootstrap scripts into the image, and those have evolve to use `uv`, which isn't installed in the image. One option would be to not mount the docker folder and use what's in the image, but I believe some of the script may have changed names or location, and `docker-compose-image-tag` specifies their location as it passes arguments to them. In any case, and at least for now, I decided to simply conditionally use uv or plain pip in the docker scripts based on what's installed in the image. Also added a CI step to make sure `docker compose -f docker-compose-image-tag.yml up` works.
mistercrunch
requested review from
villebro,
geido,
eschutho,
rusackas,
betodealmeida,
nytai,
craig-rueda,
kgabryje and
dpgaspar
as code owners
3 tasks
github-actions
bot
added
the
github_actions
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Missing pip fallback in DEV_MODE ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| docker/docker-bootstrap.sh | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ❌ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
Note
Korbit Pro is free for open source projects 🎉
Looking to add Korbit to your team? Get started with a free 2 week trial here
Comment on lines
22
to
27
| if [ "$DEV_MODE" == "true" ]; then | ||
| echo "Reinstalling the app in editable mode" | ||
| uv pip install -e . | ||
| if command -v uv > /dev/null 2>&1; then | ||
| echo "Reinstalling the app in editable mode" | ||
| uv pip install -e . | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Missing pip fallback in DEV_MODE 
Tell me more
What is the issue?
When UV is not available in DEV_MODE, the script silently skips the installation without falling back to regular pip or warning the user.
Why this matters
This could lead to missing dependencies in development environments where UV is not available, potentially causing runtime issues.
Suggested change
Add pip fallback:
if [ "$DEV_MODE" == "true" ]; then
echo "Reinstalling the app in editable mode"
if command -v uv > /dev/null 2>&1; then
uv pip install -e .
else
pip install -e .
fi
fi
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
There was a problem hiding this comment.
Don't need to reinstall in dev mode where uv is not present as it was the default install in that era
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recently I did some work to use
uv pip installin place of plainpipin our docker images. This made builds significantly faster, but broke compatibility between some docker compose setups.The issue is around the fact that
docker-compose-image-tagmounts the local, newer, docker bootstrap scripts into the image, and those have evolve to useuv, which isn't installed in the image.One option would be to not mount the docker folder and use what's in the image, but I believe some of the script may have changed names or location, and
docker-compose-image-tagspecifies their location as it passes arguments to them.In any case, and at least for now, I decided to simply conditionally use uv or plain pip in the docker scripts based on what's installed in the image.
Also added a CI step to make sure
docker compose -f docker-compose-image-tag.yml upworks.Closes #31600