-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Handle CLI run fallback when Docker Compose plugin is missing #136
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
fix: Handle CLI run fallback when Docker Compose plugin is missing #136
Conversation
Summary of ChangesHello @gocanto, 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 enhances the robustness of the Highlights
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
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSwitches Makefile run logic from direct docker run to conditional docker compose/docker-compose invocation. Adds CLI availability checks, selects the appropriate command, preserves env var propagation, and exits with guidance when neither CLI exists. Maintains non-zero status propagation and failure messaging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as make run-cli
participant Sh as Shell
participant DC as docker compose
participant DCo as docker-compose
Dev->>Sh: Invoke run target
Sh->>Sh: Check for `docker compose`
alt docker compose available
Sh->>DC: run <service> <cmd> with env
DC-->>Sh: exit code
else docker-compose available
Sh->>DCo: run <service> <cmd> with env
DCo-->>Sh: exit code
else neither available
Sh-->>Dev: Print hint and exit 1
end
Sh-->>Dev: Propagate non-zero status if failure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Code Review
This pull request adds a fallback mechanism to use the legacy docker-compose binary when the docker compose plugin is not available. This improves the developer experience on systems that might not have the latest Docker setup. The implementation is correct, but I've suggested a refactoring to reduce code duplication and improve maintainability by first determining the available command and then executing 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
|
@codex review |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| DB_SECRET_USERNAME="$(DB_SECRET_USERNAME)" DB_SECRET_PASSWORD="$(DB_SECRET_PASSWORD)" DB_SECRET_DBNAME="$(DB_SECRET_DBNAME)" docker compose run --rm api-runner go run ./metal/cli/main.go || status=$$?; \ | ||
| elif command -v docker-compose >/dev/null 2>&1; then \ | ||
| printf "Using docker-compose to run the CLI.\n"; \ | ||
| DB_SECRET_USERNAME="$(DB_SECRET_USERNAME)" DB_SECRET_PASSWORD="$(DB_SECRET_PASSWORD)" DB_SECRET_DBNAME="$(DB_SECRET_DBNAME)" docker-compose run --rm api-runner go run ./metal/cli/main.go || status=$$?; \ |
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.
Suppress command echo to avoid leaking secret values
The new compose detection block no longer prefixes the docker compose run and docker-compose run lines with @. make will now echo these recipe lines, including the literal values of DB_SECRET_USERNAME, DB_SECRET_PASSWORD, and DB_SECRET_DBNAME. When callers override these variables with real credentials (instead of file paths), the credentials will be printed to the console and to any captured logs, undermining the redaction logic above. Restoring the leading @ (or otherwise silencing these commands) keeps secrets from being disclosed while executing make run-cli.
Useful? React with 👍 / 👎.
Summary
make run-clito fall back to the legacydocker-composebinary when the Docker Compose plugin is unavailableTesting
https://chatgpt.com/codex/tasks/task_e_68edcd7fbea083339891ec4031100ae9
Summary by CodeRabbit