Add installer to release asset issue - #139 #149
Conversation
WalkthroughThe pull request introduces a trigger for release events in the GitHub Actions workflow and adds a step to update the version in Changes
Sequence Diagram(s)sequenceDiagram
participant R as Release Event
participant W as GitHub Workflow
participant S as Set Version Step
participant F as settings.json
participant U as Upload Artifact Step
R->>W: Release created (tag available)
W->>S: Initiate build-executable job
S->>F: Update version with tag value
W->>U: Upload OpenMS-App.zip as release asset
sequenceDiagram
participant D as Docker Build Process
participant G as GitHub Releases
D->>G: Execute 'gh release download OpenMS-App.zip'
G-->>D: Provide the release asset
D->>D: Unzip asset to /app/OpenMS-App
D->>D: Remove zip file after extraction
Suggested reviewers
Poem
🪧 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: 2
🧹 Nitpick comments (1)
.github/workflows/build-windows-executable-app.yaml (1)
180-186: Implement error handling for version processing.The PowerShell script effectively updates the version in settings.json using the release tag, but it lacks error handling if $VERSION is null or empty (which could happen if the workflow is triggered by an event other than a release).
Consider adding validation:
- - name: Set Version in settings.json - run: | - $VERSION="${{ github.event.release.tag_name }}" - $content = Get-Content -Raw settings.json | ConvertFrom-Json - $content.version = $VERSION - $content | ConvertTo-Json -Depth 100 | Set-Content settings.json + - name: Set Version in settings.json + run: | + $VERSION="${{ github.event.release.tag_name }}" + if ([string]::IsNullOrEmpty($VERSION)) { + Write-Host "No release tag found. Using default version." + $VERSION="0.0.0" + } else { + Write-Host "Setting version to $VERSION from release tag" + } + $content = Get-Content -Raw settings.json | ConvertFrom-Json + $content.version = $VERSION + $content | ConvertTo-Json -Depth 100 | Set-Content settings.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build-windows-executable-app.yaml(2 hunks)Dockerfile(1 hunks)Dockerfile_simple(1 hunks)settings.json(1 hunks)
🔇 Additional comments (2)
.github/workflows/build-windows-executable-app.yaml (1)
7-8: Good addition of release event trigger.Adding release event handling is appropriate for automating the deployment process when new releases are created. This will enable automatic building of the executable when a release is created.
settings.json (1)
15-16: Good addition of version field.Adding the version field to settings.json is necessary for tracking the application version and works well with the GitHub workflow that now updates this field based on release tags.
Dockerfile
Outdated
| # Download latest OpenMS App executable for Windows from GitHub Releases | ||
| RUN if [ -n "$GH_TOKEN" ]; then \ | ||
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | ||
| gh run download -R ${GITHUB_USER}/${GITHUB_REPO} $(gh run list -R ${GITHUB_USER}/${GITHUB_REPO} -b main -e push -s completed -w "Build executable for Windows" --json databaseId -q '.[0].databaseId') -n OpenMS-App --dir /app; \ | ||
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | ||
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ | ||
| rm /app/OpenMS-App.zip; \ | ||
| else \ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Improved release asset handling.
The update to use gh release download instead of the previous approach is more efficient and follows GitHub CLI best practices for fetching release assets.
Let's confirm the GitHub CLI command is correctly formatted:
🏁 Script executed:
#!/bin/bash
# Verify the `gh release download` command syntax
gh release download --help | grep -E "(-p|--pattern)"
# Check if unzip is available in the base image
apt list --installed | grep unzip || echo "unzip might not be installed in the base image"Length of output: 525
Release Asset Handling Verified; Ensure 'unzip' is Present
The update using gh release download -p is correctly implemented and aligns with GitHub CLI best practices. However, our verification indicates that the Docker image may not have the unzip utility installed. Please confirm that the base image includes unzip or add an installation step (e.g., via apt-get update && apt-get install -y unzip or an equivalent command) to ensure the asset extraction doesn’t fail at runtime.
Dockerfile_simple
Outdated
| # Download latest OpenMS App executable for Windows from GitHub Releases | ||
| RUN if [ -n "$GH_TOKEN" ]; then \ | ||
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | ||
| gh run download -R ${GITHUB_USER}/${GITHUB_REPO} $(gh run list -R ${GITHUB_USER}/${GITHUB_REPO} -b main -e push -s completed -w "Build executable for Windows" --json databaseId -q '.[0].databaseId') -n OpenMS-App --dir /app; \ | ||
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | ||
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ | ||
| rm /app/OpenMS-App.zip; \ | ||
| else \ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Improved release asset handling.
The updated approach to download release assets using gh release download improves efficiency and reliability. Unzipping the asset directly in the Dockerfile is also a good practice.
Checking for missing dependencies that might be needed for the unzip operation:
🏁 Script executed:
#!/bin/bash
# Check if unzip is installed in the base image
grep -E "apt-get install.*unzip" Dockerfile_simple || echo "unzip might not be explicitly installed"
# Check the base image for unzip
docker run --rm ubuntu:22.04 which unzip || echo "unzip is not available in the base image by default"Length of output: 445
Missing unzip Dependency in Base Image
The updated asset download approach using gh release download is efficient and streamlines the process. However, our checks indicate that the unzip utility is not available in the base image by default. To prevent build failures during the asset extraction step, please add an installation step for unzip before the download and unzip operations.
- Location:
Dockerfile_simple(Lines 100-106) - Suggestion: Add a command (e.g., for an Ubuntu-based image):
before the asset download step.
+ RUN apt-get update && apt-get install -y unzip
📝 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.
| # Download latest OpenMS App executable for Windows from GitHub Releases | |
| RUN if [ -n "$GH_TOKEN" ]; then \ | |
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | |
| gh run download -R ${GITHUB_USER}/${GITHUB_REPO} $(gh run list -R ${GITHUB_USER}/${GITHUB_REPO} -b main -e push -s completed -w "Build executable for Windows" --json databaseId -q '.[0].databaseId') -n OpenMS-App --dir /app; \ | |
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | |
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ | |
| rm /app/OpenMS-App.zip; \ | |
| else \ | |
| # Download latest OpenMS App executable for Windows from GitHub Releases | |
| RUN apt-get update && apt-get install -y unzip | |
| RUN if [ -n "$GH_TOKEN" ]; then \ | |
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | |
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | |
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ | |
| rm /app/OpenMS-App.zip; \ | |
| else \ |
There was a problem hiding this comment.
Thank you for your contribution. The artifact still needs to be updated as a release asset. You can take a look here as a reference: https://github.com/t0mdavid-m/FLASHViewer/blob/develop/.github/workflows/build-windows-executable-app.yaml
Dockerfile
Outdated
| RUN if [ -n "$GH_TOKEN" ]; then \ | ||
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | ||
| gh run download -R ${GITHUB_USER}/${GITHUB_REPO} $(gh run list -R ${GITHUB_USER}/${GITHUB_REPO} -b main -e push -s completed -w "Build executable for Windows" --json databaseId -q '.[0].databaseId') -n OpenMS-App --dir /app; \ | ||
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | ||
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ |
There was a problem hiding this comment.
This should be removed as the download button expects the zipped asset.
Dockerfile_simple
Outdated
| RUN if [ -n "$GH_TOKEN" ]; then \ | ||
| echo "GH_TOKEN is set, proceeding to download the release asset..."; \ | ||
| gh run download -R ${GITHUB_USER}/${GITHUB_REPO} $(gh run list -R ${GITHUB_USER}/${GITHUB_REPO} -b main -e push -s completed -w "Build executable for Windows" --json databaseId -q '.[0].databaseId') -n OpenMS-App --dir /app; \ | ||
| gh release download -R ${GITHUB_USER}/${GITHUB_REPO} -p "OpenMS-App.zip" -D /app; \ | ||
| unzip /app/OpenMS-App.zip -d /app/OpenMS-App && \ |
|
@t0mdavid-m |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/build-windows-executable-app.yaml (1)
180-186: Explicitly Specify the Shell for the Version Update Step
The "Set Version in settings.json" step uses PowerShell commands (e.g.,Get-ContentandConvertFrom-Json). To prevent any ambiguity or potential shell mismatches on Windows runners, explicitly setting the shell (for example,shell: pwsh) is recommended.- - name: Set Version in settings.json - run: | - $VERSION="${{ github.event.release.tag_name }}" - $content = Get-Content -Raw settings.json | ConvertFrom-Json - $content.version = $VERSION - $content | ConvertTo-Json -Depth 100 | Set-Content settings.json + - name: Set Version in settings.json + shell: pwsh + run: | + $VERSION = "${{ github.event.release.tag_name }}" + $content = Get-Content -Raw settings.json | ConvertFrom-Json + $content.version = $VERSION + $content | ConvertTo-Json -Depth 100 | Set-Content settings.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(3 hunks)Dockerfile(1 hunks)Dockerfile_simple(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Dockerfile
- Dockerfile_simple
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml
276-276: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/build-windows-executable-app.yaml
424-424: the runner of "softprops/action-gh-release@v1" 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/build-windows-executable-app-with-pyinstaller.yaml
[error] 273-273: trailing spaces
(trailing-spaces)
[error] 280-280: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/build-windows-executable-app.yaml (1)
7-8: Introduce Release Event Trigger
The addition of thereleasetrigger (withtypes: [created]) allows the workflow to execute on release events. This aligns well with the PR objective of handling installer release assets.
|
|
||
| - name: Upload Artifact as Release Asset | ||
| if: github.event_name == 'release' | ||
| uses: softprops/action-gh-release@v1 | ||
| with: | ||
| files: OpenMS-App.zip | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update the Release Asset Upload Step for Compatibility and Formatting
- The new "Upload Artifact as Release Asset" step uses
softprops/action-gh-release@v1, which is flagged by static analysis as outdated. Consider updating it to a newer version (for example,softprops/action-gh-release@v2) to ensure compatibility with current GitHub Actions runners. - Additionally, remove any trailing spaces (noted at line 273) and ensure the file ends with a newline (as flagged at line 280).
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2📝 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.
| - name: Upload Artifact as Release Asset | |
| if: github.event_name == 'release' | |
| uses: softprops/action-gh-release@v1 | |
| with: | |
| files: OpenMS-App.zip | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Upload Artifact as Release Asset | |
| if: github.event_name == 'release' | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: OpenMS-App.zip | |
| env: | |
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
🧰 Tools
🪛 actionlint (1.7.4)
276-276: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 273-273: trailing spaces
(trailing-spaces)
[error] 280-280: no new line character at the end of file
(new-line-at-end-of-file)
|
|
||
| - name: Upload Artifact as Release Asset | ||
| if: github.event_name == 'release' | ||
| uses: softprops/action-gh-release@v1 | ||
| with: | ||
| files: OpenMS-App.zip | ||
| env: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update the Release Asset Upload Step for Consistency
Similar to the PyInstaller workflow, the "Upload Artifact as Release Asset" step here also uses softprops/action-gh-release@v1. Update this to a newer version (e.g., softprops/action-gh-release@v2) to avoid potential runtime issues, as noted by static analysis.
- uses: softprops/action-gh-release@v1
+ uses: softprops/action-gh-release@v2📝 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.
| - name: Upload Artifact as Release Asset | |
| if: github.event_name == 'release' | |
| uses: softprops/action-gh-release@v1 | |
| with: | |
| files: OpenMS-App.zip | |
| env: | |
| - name: Upload Artifact as Release Asset | |
| if: github.event_name == 'release' | |
| - uses: softprops/action-gh-release@v1 | |
| + uses: softprops/action-gh-release@v2 | |
| with: | |
| files: OpenMS-App.zip | |
| env: |
🧰 Tools
🪛 actionlint (1.7.4)
424-424: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml (1)
273-273: YAML Formatting FixesStatic analysis flagged a trailing whitespace on line 273 and a missing newline at the end of the file (line 280). Please remove the extra space and add a newline at the end to ensure the file complies with YAML style guidelines.
Also applies to: 280-280
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 273-273: trailing spaces
(trailing-spaces)
.github/workflows/build-windows-executable-app.yaml (1)
180-185: Update Version in settings.jsonA new step updates
settings.jsonby setting itsversionfield to the release tag (${{ github.event.release.tag_name }}). This ensures that the version information remains current with each release. Consider adding error handling in case the JSON file is missing or malformed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml
[error] 273-273: trailing spaces
(trailing-spaces)
[error] 280-280: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml (1)
274-280: Release Asset Upload Step AddedThe newly introduced "Upload Artifact as Release Asset" step is well configured. It correctly restricts execution to release events (
if: github.event_name == 'release') and usessoftprops/action-gh-release@v2to upload the artifact fileOpenMS-App.zip. Ensure that the artifact is successfully generated during the build process so that the upload step functions as expected.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 280-280: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/build-windows-executable-app.yaml (2)
7-8: Release Event Trigger AddedThe workflow has been updated to include a release event trigger:
release: types: [created]This change enables the workflow to run when a new release is created, aligning well with the updated deployment process.
422-429: Release Asset Upload Configuration ConfirmedThe step that uploads the built artifact
OpenMS-App.zipusingsoftprops/action-gh-release@v2is correctly conditioned to run only on release events. Please verify that the artifact is properly generated in earlier steps to prevent any upload failures.
|
@t0mdavid-m |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml (1)
284-291: Ensure Consistent Release Asset Upload Configuration and YAML Formatting
The new step to upload the built artifact as a release asset usingsoftprops/action-gh-release@v2is correctly configured to trigger only on release events. However, YAMLlint has flagged trailing spaces at line 284 and a missing newline character at the end of the file (line 291). Removing extra whitespace and ensuring the file ends with a newline will help maintain formatting quality and prevent potential issues in downstream tooling.Below is a suggested diff improvement for formatting:
... - (EOF with no newline) +(Ensure a newline at the end of the file.)
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 284-284: trailing spaces
(trailing-spaces)
[error] 291-291: 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/build-windows-executable-app-with-pyinstaller.yaml(1 hunks).github/workflows/build-windows-executable-app.yaml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-windows-executable-app-with-pyinstaller.yaml
[error] 284-284: trailing spaces
(trailing-spaces)
[error] 291-291: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build
- GitHub Check: build-full-app
- GitHub Check: build-simple-app
- GitHub Check: build-openms
🔇 Additional comments (3)
.github/workflows/build-windows-executable-app.yaml (3)
7-8: Addition of Release Event Trigger
A new trigger for release events (withtypes: [created]) has been added alongside push, pull_request, and workflow_dispatch. This change correctly enables the workflow to run upon a new release and aligns with the PR goals of handling installer release assets.
191-197: Set Version in settings.json Step
The newly added step that updates theversionfield insettings.jsonusing the release tag (${{ github.event.release.tag_name }}) is a useful addition to automate version management. Ensure that the file exists and that appropriate error handling is in place (if needed) for cases wheresettings.jsonmight be missing or improperly formatted.
433-440: Consistent Release Asset Upload Configuration
This step mirrors the upload configuration in the PyInstaller workflow and usessoftprops/action-gh-release@v2to uploadOpenMS-App.zipas a release asset when a release event occurs. It adheres to the improvements suggested in previous reviews. Just as in the other workflow, please review for any trailing whitespace or formatting inconsistencies.
Summary by CodeRabbit
New Features
Chores