feat: nightly builds#285
Conversation
|
Review changes with Changed Files
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdded two new GitHub Actions workflows for automated Docker image builds: a nightly workflow triggering on a daily schedule that builds multi-architecture images for server, analytics, and dashboard services, and a prerelease workflow triggering on pull request merge. Updated the existing release workflow to use manual dispatch instead of pull request triggers. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
.github/workflows/prerelease_on_merge.yaml (4)
47-58: Consider adding a prerelease identifier to the version format.The version format
{base_version}-{sha}(e.g.,1.0.1-abc123def456) doesn't include a prerelease identifier likepreorrc. This makes it harder to distinguish prerelease images from nightly ones at a glance.Nightly uses:
1.0.1-nightly.20260325.abc123def456
Prerelease uses:1.0.1-abc123def456Consider using
1.0.1-pre.abc123def456for clarity and semantic versioning alignment.♻️ Add prerelease identifier
short_sha=$(git rev-parse --short=12 "${build_sha}") - version="${base_version}-${short_sha}" + version="${base_version}-pre.${short_sha}" tracking_tag="prerelease-v${version}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease_on_merge.yaml around lines 47 - 58, Change the generated version/tracking_tag to include a prerelease identifier instead of just the raw SHA: when computing version (currently using base_version, short_sha and variables build_sha/short_sha) compose it as base_version + "-pre." + short_sha (or similar prerelease label), and update tracking_tag to use that new version string so prereleases read like "1.0.1-pre.<sha>" rather than "1.0.1-<sha>"; adjust any uses of version/tracking_tag (the variables version and tracking_tag) accordingly.
3-17: Redundant branch check in job condition.The condition
github.event.pull_request.base.ref == 'main'on line 17 is redundant since the trigger already specifiesbranches: [main]on line 6. While harmless, it adds unnecessary noise.♻️ Simplified condition
- if: github.event.pull_request.merged == true && github.event.pull_request.base.ref == 'main' + if: github.event.pull_request.merged == true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease_on_merge.yaml around lines 3 - 17, Remove the redundant branch check from the job condition: in the prepare-prerelease-version job, simplify the if expression by dropping "&& github.event.pull_request.base.ref == 'main'" because the workflow trigger already limits events to branches: [main]; keep only "if: github.event.pull_request.merged == true" to preserve the merged-only guard.
60-76: Partial failure scenario not fully handled.If a previous run created the tracking tag but failed during Docker builds, a re-run would:
- Skip tag creation (tag exists) ✓
- Proceed with Docker builds (version is non-empty) ✓
This works, but the downstream jobs gate on
version != ''which always passes if the version step succeeded, even when the tag already existed. Consider adding ashould_buildoutput similar to the nightly workflow to make the flow more explicit about whether builds should proceed.Also applies to: 78-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease_on_merge.yaml around lines 60 - 76, Update the "Create prerelease tracking tag" step to emit a boolean output (e.g., should_build) that is false when the tag already exists and true when you create the tag (use the same signals tracking_tag and build_sha from steps.version); then change downstream job conditions to gate on both version != '' AND steps.version.outputs.should_build == 'true' (mirror the nightly workflow pattern) so a rerun that finds an existing tag will skip Docker builds. Also add the same should_build behavior for the other tag-handling block referenced in the comment.
84-94: Unusedplatformmatrix field.The
platformfield (e.g.,linux/amd64) is defined in the matrix but never referenced in the job steps. The builds rely on native runner architecture rather than cross-compilation.Either remove the unused field or use it with
platforms:indocker/build-push-actionfor explicit platform targeting.♻️ Remove unused field
matrix: include: - - platform: linux/amd64 - tag: linux-amd64 + - tag: linux-amd64 os: ubuntu-latest - - platform: linux/arm64 - tag: linux-arm64 + - tag: linux-arm64 os: ubuntu-24.04-arm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease_on_merge.yaml around lines 84 - 94, The matrix defines an unused platform field (matrix.platform) — either remove the platform entries under strategy.matrix.include or wire them into the build step by passing matrix.platform to docker/build-push-action via the platforms: input; update the job that calls docker/build-push-action to use platforms: ${{ matrix.platform }} (or delete the platform keys entirely along with any unused references) so the matrix is consistent with the actual build invocation and avoids dead fields..github/workflows/nightly_build.yaml (1)
82-99: Tag existence check doesn't prevent rebuilds.When the tracking tag already exists (lines 91-94), the step exits successfully without updating
should_build. This means downstream Docker builds will still proceed, potentially overwriting existing images.If rebuilds on tag collision are undesirable, consider outputting a flag or failing the step:
♻️ Option: Skip builds when tag exists
- name: Create nightly tracking tag if: steps.version.outputs.should_build == 'true' + id: tag shell: bash run: | set -euo pipefail tracking_tag="${{ steps.version.outputs.tracking_tag }}" build_sha="${{ steps.version.outputs.build_sha }}" if git rev-parse "${tracking_tag}" >/dev/null 2>&1; then echo "Tracking tag ${tracking_tag} already exists" + echo "tag_created=false" >> "$GITHUB_OUTPUT" exit 0 fi git config user.email 'airborne_bot@juspay.in' git config user.name 'Airborne Bot' git tag "${tracking_tag}" "${build_sha}" git push origin "${tracking_tag}" + echo "tag_created=true" >> "$GITHUB_OUTPUT"Then downstream jobs can additionally check
tag_created == 'true'if you want to prevent overwriting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly_build.yaml around lines 82 - 99, The current "Create nightly tracking tag" step only exits when the tracking_tag already exists, but doesn't set any output to prevent downstream builds; update this step to emit a step output (e.g., tag_created) that is set to 'false' when git rev-parse "${tracking_tag}" finds the tag and set to 'true' after creating and pushing the tag, so downstream jobs can check tag_created == 'true' (or fail the step instead if you prefer) before running Docker builds; key symbols: tracking_tag, build_sha, and the step named "Create nightly tracking tag".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/nightly_build.yaml:
- Around line 82-99: The current "Create nightly tracking tag" step only exits
when the tracking_tag already exists, but doesn't set any output to prevent
downstream builds; update this step to emit a step output (e.g., tag_created)
that is set to 'false' when git rev-parse "${tracking_tag}" finds the tag and
set to 'true' after creating and pushing the tag, so downstream jobs can check
tag_created == 'true' (or fail the step instead if you prefer) before running
Docker builds; key symbols: tracking_tag, build_sha, and the step named "Create
nightly tracking tag".
In @.github/workflows/prerelease_on_merge.yaml:
- Around line 47-58: Change the generated version/tracking_tag to include a
prerelease identifier instead of just the raw SHA: when computing version
(currently using base_version, short_sha and variables build_sha/short_sha)
compose it as base_version + "-pre." + short_sha (or similar prerelease label),
and update tracking_tag to use that new version string so prereleases read like
"1.0.1-pre.<sha>" rather than "1.0.1-<sha>"; adjust any uses of
version/tracking_tag (the variables version and tracking_tag) accordingly.
- Around line 3-17: Remove the redundant branch check from the job condition: in
the prepare-prerelease-version job, simplify the if expression by dropping "&&
github.event.pull_request.base.ref == 'main'" because the workflow trigger
already limits events to branches: [main]; keep only "if:
github.event.pull_request.merged == true" to preserve the merged-only guard.
- Around line 60-76: Update the "Create prerelease tracking tag" step to emit a
boolean output (e.g., should_build) that is false when the tag already exists
and true when you create the tag (use the same signals tracking_tag and
build_sha from steps.version); then change downstream job conditions to gate on
both version != '' AND steps.version.outputs.should_build == 'true' (mirror the
nightly workflow pattern) so a rerun that finds an existing tag will skip Docker
builds. Also add the same should_build behavior for the other tag-handling block
referenced in the comment.
- Around line 84-94: The matrix defines an unused platform field
(matrix.platform) — either remove the platform entries under
strategy.matrix.include or wire them into the build step by passing
matrix.platform to docker/build-push-action via the platforms: input; update the
job that calls docker/build-push-action to use platforms: ${{ matrix.platform }}
(or delete the platform keys entirely along with any unused references) so the
matrix is consistent with the actual build invocation and avoids dead fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00a2df3b-4297-4661-b387-dcd5534ff2fe
📒 Files selected for processing (3)
.github/workflows/nightly_build.yaml.github/workflows/prerelease_on_merge.yaml.github/workflows/release.yaml
1 participant
Summary by CodeRabbit