ci: Add a workflow for uploading files #1157
Conversation
WalkthroughThis pull request introduces a new GitHub Actions workflow named Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CI as GitHub Actions
participant S3 as S3 Bucket
Dev->>CI: Push/PR on master
CI->>CI: Checkout repository
CI->>CI: Check for changes in uploads folder
alt Changes detected
CI->>S3: Upload "CMakeEd-1.24.1.zip" (public-read)
else No changes detected
CI->>CI: Log file not found message
end
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci_uploads.yml (2)
34-39: Parameterize File Path and S3 Bucket Details.
The run block assigns the file path, bucket name, and remote path directly. For improved maintainability, consider parameterizing these values or defining them as workflow-level environment variables if they are subject to change.
40-45: File Upload and Error Handling.
The script correctly checks for the existence of the file before performing the S3 upload. It might be beneficial to capture and log any errors from theaws s3 cpcommand to aid debugging in case of failed uploads. Additionally, please add a newline character at the end of the file to comply with YAML formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
internal/com.espressif.idf.uploads/bin/module-info.classis excluded by!**/*.classinternal/com.espressif.idf.uploads/cmakeed/CMakeEd-1.24.1.zipis excluded by!**/*.zip
📒 Files selected for processing (6)
.github/workflows/ci_uploads.yml(1 hunks)internal/com.espressif.idf.uploads/.classpath(1 hunks)internal/com.espressif.idf.uploads/.project(1 hunks)internal/com.espressif.idf.uploads/.settings/org.eclipse.core.resources.prefs(1 hunks)internal/com.espressif.idf.uploads/.settings/org.eclipse.jdt.core.prefs(1 hunks)internal/com.espressif.idf.uploads/src/module-info.java(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- internal/com.espressif.idf.uploads/src/module-info.java
- internal/com.espressif.idf.uploads/.classpath
- internal/com.espressif.idf.uploads/.project
- internal/com.espressif.idf.uploads/.settings/org.eclipse.core.resources.prefs
- internal/com.espressif.idf.uploads/.settings/org.eclipse.jdt.core.prefs
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci_uploads.yml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build_macos
🔇 Additional comments (7)
.github/workflows/ci_uploads.yml (7)
1-1: Workflow Name is Clear.
The name "IDE File Uploads" succinctly describes the purpose of this workflow.
3-10: Trigger Configuration is Correct.
The workflow is triggered on both push and pull_request events targeting the master branch. Verify that these triggers align with your branch management strategy.
11-14: Job Configuration Check.
The jobupload-fileis correctly defined to run on theubuntu-latestenvironment.
15-18: Repository Checkout Step.
The use ofactions/checkout@v4ensures that the repository is checked out with the latest improvements.
19-27: Verify the Change Detection Logic.
This step usesgit diff --name-onlyto check for changes in theinternal/com.espressif.idf.uploads/directory. Please ensure that${{ github.event.before }}is available in all event contexts (especially for pull requests) to avoid false negatives.
28-29: Conditional Execution for S3 Upload.
Theif: env.changed == 'true'condition effectively gates the upload step to run only when changes are detected.
30-33: AWS Credentials Setup.
The AWS credentials are securely injected via GitHub secrets. Confirm that these secrets (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_DEFAULT_REGION) are properly configured in your repository settings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci_uploads.yml (2)
19-36: Update GitHub Script Action Version for Enhanced Compatibility
This step usesactions/github-script@v6to check for changes in the uploads folder. However, static analysis indicates that the runner for this version is outdated. Consider updating to a later version (e.g.,v6.1.0) to benefit from the latest enhancements and fixes.You can apply this change with the following diff:
- uses: actions/github-script@v6 + uses: actions/github-script@v6.1.0🧰 Tools
🪛 actionlint (1.7.4)
21-21: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
37-54: Enhance Shell Script Robustness and Ensure File Ends with a Newline
The "Upload file to S3" step correctly checks for the existence of the file and uploads it using AWS credentials. A couple of improvements are suggested:
Robust Error Handling: Prepend the run block with
set -euo pipefail(or at leastset -e) to ensure that the script fails immediately on errors. This makes the step more robust by catching any command failures automatically.YAML Lint Compliance: YAMLlint flagged that there is no new line at the end of the file. Please add a newline at the very end to adhere to best practices.
Below is a diff snippet addressing these suggestions:
- run: | - FILE="./internal/com.espressif.idf.uploads/cmakeed/CMakeEd-1.24.1.zip" - FILE_NAME=$(basename "$FILE") - BUCKET_NAME="dl.espressif.com" - REMOTE_PATH="dl/cmakeed/updates/" - - if [ -f "$FILE" ]; then - echo "Uploading $FILE_NAME to S3..." - aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read - else - echo "File $FILE not found, skipping..." - fi + run: | + set -euo pipefail + + FILE="./internal/com.espressif.idf.uploads/cmakeed/CMakeEd-1.24.1.zip" + FILE_NAME=$(basename "$FILE") + BUCKET_NAME="dl.espressif.com" + REMOTE_PATH="dl/cmakeed/updates/" + + if [ -f "$FILE" ]; then + echo "Uploading $FILE_NAME to S3..." + aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read + else + echo "File $FILE not found, skipping..." + fiAdditionally, please ensure there is a newline character at the end of the file to satisfy YAML linting requirements.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci_uploads.yml(1 hunks)internal/com.espressif.idf.uploads/changes.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- internal/com.espressif.idf.uploads/changes.txt
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci_uploads.yml
21-21: the runner of "actions/github-script@v6" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/ci_uploads.yml
[error] 54-54: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_macos
🔇 Additional comments (4)
.github/workflows/ci_uploads.yml (4)
1-2: Workflow Name is Clear and Descriptive
The name "IDE File Uploads" clearly reflects the purpose of this workflow.
3-10: Trigger Configuration is Correct
The workflow is correctly set to trigger on pushes and pull requests to themasterbranch.
11-14: Job Definition is Set Up Properly
The jobupload-fileis defined to run on the latest Ubuntu environment, which is appropriate for this CI task.
15-18: Repository Checkout Step is Implemented Correctly
Usingactions/checkout@v4ensures that the repository is available for subsequent steps.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci_uploads.yml (2)
29-32: File Path and Metadata ExtractionThe shell script correctly sets the
FILEvariable to the expected path and usesbasenameto extract the file name. Although the file path is hardcoded (i.e.CMakeEd-1.24.1.zip), it is acceptable given the current scope. If there is a possibility for future changes (for example, handling multiple files or version updates), consider parameterizing this value.
39-39: Add a Newline at End of FileYAML linting indicates that there is no newline at the end of the file. Please add a newline character to comply with coding style guidelines and avoid potential parsing issues.
- fi + fi +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci_uploads.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci_uploads.yml
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build_macos
- GitHub Check: build
🔇 Additional comments (5)
.github/workflows/ci_uploads.yml (5)
1-2: Descriptive Workflow NameThe workflow name is clear and descriptive ("IDE File Uploads"), which makes it easy to identify its purpose.
3-14: Well-Scoped CI TriggersThe "on" block is correctly configured to trigger on both
pushandpull_requestevents on themasterbranch, and it accurately monitors changes in theinternal/com.espressif.idf.uploads/**directory. This precise targeting helps avoid unnecessary workflow executions.
15-18: Job Configuration is CorrectThe job definition for
upload-fileis straightforward and correctly specifies the use of theubuntu-latestrunner.
19-22: Effective Repository CheckoutThe checkout step uses
actions/checkout@v4, which is aligned with current best practices. This ensures that the workflow always operates on the latest commit from the branch under test.
23-28: Secure and Clear Environment Variable SetupThe step "Upload file to S3" clearly assigns the AWS credentials and region via secrets. This helps keep sensitive information secure. Verify that the secrets (
AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY, andAWS_DEFAULT_REGION) are correctly configured in your repository settings.
| if [ -f "$FILE" ]; then | ||
| echo "Uploading $FILE_NAME to S3..." | ||
| aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read | ||
| else | ||
| echo "File $FILE not found, skipping..." | ||
| fi No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Robust File Existence Check with Opportunity for Enhanced Error Handling
The conditional check using [ -f "$FILE" ] is correctly implemented to determine if the file exists before attempting an upload. The script logs an informative message if the file is not found.
Suggestion: Consider capturing and evaluating the exit code of the aws s3 cp command. This can help in detecting and reporting potential upload failures, ensuring that the CI process fails explicitly if the file upload is critical. For example:
- aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read
+ aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read
+ if [ $? -ne 0 ]; then
+ echo "Error: File upload failed."
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -f "$FILE" ]; then | |
| echo "Uploading $FILE_NAME to S3..." | |
| aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read | |
| else | |
| echo "File $FILE not found, skipping..." | |
| fi | |
| if [ -f "$FILE" ]; then | |
| echo "Uploading $FILE_NAME to S3..." | |
| aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read | |
| if [ $? -ne 0 ]; then | |
| echo "Error: File upload failed." | |
| exit 1 | |
| fi | |
| else | |
| echo "File $FILE not found, skipping..." | |
| fi |
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci_uploads.yml (1)
34-39: 🛠️ Refactor suggestionEnhanced Error Handling for File Upload
Consider capturing and evaluating the exit status of theaws s3 cpcommand to ensure the workflow fails explicitly if the upload fails. This will add robustness to the deployment process. For example:- aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read + aws s3 cp "$FILE" "s3://$BUCKET_NAME/$REMOTE_PATH$FILE_NAME" --acl public-read + if [ $? -ne 0 ]; then + echo "Error: File upload failed." + exit 1 + fi🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
🧹 Nitpick comments (1)
.github/workflows/ci_uploads.yml (1)
39-39: Ensure File Ends with a Newline
The YAML linter indicates that the file is missing a newline at the end. Adding one will ensure compliance with YAML formatting standards and prevent potential linting errors.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci_uploads.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci_uploads.yml
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
.github/workflows/ci_uploads.yml (4)
1-2: Workflow Title Clarity
The workflow is clearly named "IDE File Uploads" and accurately reflects its purpose of automating file uploads.
3-14: Accurate Trigger Configuration
The workflow uses precise triggers based on push and pull request events to themasterbranch and correctly filters paths underinternal/com.espressif.idf.uploads/**. This ensures that changes in the corresponding directory will initiate the job.
15-22: Job Setup & Checkout Step Verification
Theupload-filejob is well-configured withruns-on: ubuntu-latest, and theCheckout repositorystep employsactions/checkout@v4correctly to access the repository’s content.
23-32: AWS Environment Variables and File Path Setup
The AWS credentials are securely provided using secrets, and the variables for the file path (FILE), bucket name (BUCKET_NAME), and remote path (REMOTE_PATH) are clearly defined. This structure promotes maintainability and secures sensitive data.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci_uploads.yml (2)
31-31: Remove Trailing Whitespace
There are trailing spaces on this line. Please remove them to comply with YAML style guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 31-31: trailing spaces
(trailing-spaces)
37-37: Add Newline at EOF
Please ensure that a newline is added at the end of the file to meet YAML style requirements.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci_uploads.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/ci_uploads.yml
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_macos
🔇 Additional comments (8)
.github/workflows/ci_uploads.yml (8)
1-2: Descriptive Workflow Name and CI Setup
The workflow name "IDE File Uploads" clearly conveys its purpose.
3-14: Accurate Event Trigger Configuration
The trigger section correctly watches both push and pull_request events on the master branch for changes in the designated uploads directory.
15-18: Well-configured Job Definition
The "upload-file" job is properly defined to run on the latest Ubuntu environment, which is suitable for executing AWS CLI commands.
19-22: Clean Repository Checkout Step
The use ofactions/checkout@v4ensures that the latest version of the repository is checked out as the first step.
23-28: Appropriate Environment Variable Setup
The environment variables for AWS credentials and region are set securely using repository secrets. This setup correctly prepares the environment for the subsequent upload operation.
29-30: Correct File Path and Name Extraction
The file path is specified, and the use ofbasenameto extract the file name is a clean solution.
32-35: Enhance AWS S3 Upload Error Handling
The AWS S3 copy command is executed without checking the exit code, which may mask upload failures. Enhancing error handling by capturing the exit status can ensure that the workflow fails explicitly if the file upload encounters issues.A possible diff suggestion:
- aws s3 cp "$FILE" "s3://${{ secrets.DL_BUCKET }}/dl/idf-eclipse-plugin/updates/$FILE_NAME" --acl public-read + aws s3 cp "$FILE" "s3://${{ secrets.DL_BUCKET }}/dl/idf-eclipse-plugin/updates/$FILE_NAME" --acl public-read + if [ $? -ne 0 ]; then + echo "Error: File upload failed." + exit 1 + fi
36-37: Graceful Handling of Missing Files
The else branch logs a message when the file is not found, which is a good way to skip the upload gracefully when the target file does not exist.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 37-37: no new line character at the end of file
(new-line-at-end-of-file)
Description
Please include a summary of the change and which issue is fixed.
Fixes # (IEP-XXX)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit