feat: avoid redundant binary downloads and notify when skipping #210
Conversation
- Skip downloading the binary if it already exists, and print a message instead Signed-off-by: appleboy <appleboy.tw@gmail.com>
|
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. WalkthroughThe entrypoint script now checks whether the target client binary is executable before downloading; it downloads and sets executable only if missing or non-executable, otherwise logs a skip message. Invocation of the binary (e.g., --version) remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Container as Container Runtime
participant Entrypoint as entrypoint.sh
participant FS as File System
participant Net as curl
Container->>Entrypoint: Start
Entrypoint->>FS: test -x TARGET ?
alt TARGET is executable
Entrypoint-->>Container: Log "Binary ... already exists and is executable, skipping download."
else TARGET missing or not executable
Entrypoint->>Net: curl download CLIENT_BINARY
Net-->>Entrypoint: Binary bytes
Entrypoint->>FS: chmod +x TARGET
end
Entrypoint-->>Container: Invoke CLIENT_BINARY --version
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @appleboy, 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 efficiency of the entrypoint script by intelligently managing binary downloads. It ensures that the necessary client binary is only downloaded if it's not already present, thereby saving time and resources. This change also improves user experience by providing explicit feedback when a download is bypassed.
Highlights
- Binary Download Optimization: The pull request introduces a check to prevent redundant downloads of the client binary. If the target binary file already exists, the download process is now skipped.
- User Notification: When a binary download is skipped because the file already exists, a clear message is now printed to the console, informing the user of this action.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a check to avoid re-downloading the drone-scp binary if it already exists, which is a good optimization. My review includes a suggestion to make this check more robust by verifying not just the existence of the file, but also its execute permissions. This will prevent potential failures in edge cases where the file exists but is not executable.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
entrypoint.sh (2)
38-41: Avoid unbound variable errors underset -ufor INPUT_CURL_INSECURE.With
set -u, an unset INPUT_CURL_INSECURE will abort the script. Default it.-INSECURE_OPTION="" -if [[ "${INPUT_CURL_INSECURE}" == 'true' ]]; then +INSECURE_OPTION="" +if [[ "${INPUT_CURL_INSECURE:-false}" == 'true' ]]; then INSECURE_OPTION="--insecure" fi
52-60: Sameset -uissue for INPUT_CAPTURE_STDOUT; provide a default.Prevents unexpected exits when the input isn’t supplied.
-if [[ "${INPUT_CAPTURE_STDOUT}" == 'true' ]]; then +if [[ "${INPUT_CAPTURE_STDOUT:-false}" == 'true' ]]; then { echo 'stdout<<EOF' "${TARGET}" "$@" | tee -a "${GITHUB_OUTPUT}" echo 'EOF' } >>"${GITHUB_OUTPUT}" else "${TARGET}" "$@" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
entrypoint.sh(1 hunks)
entrypoint.sh
Outdated
| if [[ ! -f "${TARGET}" ]]; then | ||
| curl -fsSL --retry 5 --keepalive-time 2 ${INSECURE_OPTION} "${DOWNLOAD_URL_PREFIX}/${CLIENT_BINARY}" -o "${TARGET}" | ||
| chmod +x "${TARGET}" | ||
| else | ||
| echo "Binary ${CLIENT_BINARY} already exists, skipping download." | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure executable bit when using cache; fix misleading "Downloading" log.
- If the file exists but isn’t executable, the script will fail later. Check/repair the exec bit when reusing the cached binary.
- The unconditional “Downloading …” log (Line 36) is printed even when skipping, which is confusing. Move it into the download branch and add a clear cache message.
-echo "Downloading ${CLIENT_BINARY} from ${DOWNLOAD_URL_PREFIX}"
+echo "Binary path: ${TARGET}"
-if [[ ! -f "${TARGET}" ]]; then
- curl -fsSL --retry 5 --keepalive-time 2 ${INSECURE_OPTION} "${DOWNLOAD_URL_PREFIX}/${CLIENT_BINARY}" -o "${TARGET}"
- chmod +x "${TARGET}"
-else
- echo "Binary ${CLIENT_BINARY} already exists, skipping download."
-fi
+if [[ ! -f "${TARGET}" ]]; then
+ echo "Downloading ${CLIENT_BINARY} from ${DOWNLOAD_URL_PREFIX}"
+ curl -fsSL --retry 5 --keepalive-time 2 ${INSECURE_OPTION} \
+ "${DOWNLOAD_URL_PREFIX}/${CLIENT_BINARY}" -o "${TARGET}"
+ chmod +x "${TARGET}"
+elif [[ ! -x "${TARGET}" ]]; then
+ chmod +x "${TARGET}"
+ echo "Binary ${CLIENT_BINARY} found in cache; set executable bit."
+else
+ echo "Using cached binary ${CLIENT_BINARY}."
+fiAlso applies to: 36-36
🤖 Prompt for AI Agents
In entrypoint.sh around lines 36 and 42-47, the script currently prints a
"Downloading ..." message unconditionally and only sets the executable bit when
it actually downloads the file; fix by moving the "Downloading ..." log inside
the download branch so it only appears when curl runs, and when the file already
exists check and set the exec bit (chmod +x) before continuing and print a clear
cache message (e.g., "Using cached binary ... and ensured executable bit").
Ensure the existing branch does not skip the chmod step and updates logs
accordingly.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
fixed #202
fixed #201
Summary by CodeRabbit