CI: Update and improve database image build workflow#172
Draft
Marty-Byrde wants to merge 5 commits into
Draft
Conversation
This action is a reworked, improved and more generic version of the already existing check-changed-files action. It essentially checks whether the files that were changed within a commit-range match a given regex-pattern.
This way the checking of whether or not the database schema has been changed is done by the composite-action, reducing code duplication.
Contributor
WalkthroughAdds a reusable composite GitHub Action at Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant FileChangeAction as "file-change Action"
participant Git
Workflow->>FileChangeAction: call(base_commit, newest_commit, regex_pattern, skip, consider_initial_commit_relevant)
alt consider_initial_commit_relevant && base_commit empty/zero
FileChangeAction->>FileChangeAction: set changed = true
else
FileChangeAction->>Git: git diff --name-only base_commit...newest_commit
Git-->>FileChangeAction: list of changed files
FileChangeAction->>FileChangeAction: filter list by regex_pattern => MATCHED_FILES
alt MATCHED_FILES non-empty
FileChangeAction->>FileChangeAction: set changed = true
else
FileChangeAction->>FileChangeAction: set changed = false
end
end
alt skip == 1 or true
FileChangeAction->>FileChangeAction: override changed = true
end
FileChangeAction-->>Workflow: output changed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET 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 |
Marty-Byrde
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (1)
.github/workflows/semantic-versioning.yml (1)
143-162: Output reference mismatch: accessing non-existent step output.The
schema-modifiedjob output at line 147 referencessteps.changed.outputs.db_changed, but the file-change action only outputschanged. This will cause the workflow to fail when downstream jobs try to reference this output.Compare with
.github/workflows/build-db-docker-image.ymlline 18, which correctly maps the output:db_changed: ${{ steps.changed.outputs.changed }}.schema-modified: runs-on: ubuntu-latest name: "Check for Database Schema Update" outputs: - db_changed: ${{ steps.changed.outputs.db_changed }} + db_changed: ${{ steps.changed.outputs.changed }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/file-change/action.yml(1 hunks).github/workflows/build-db-docker-image.yml(1 hunks).github/workflows/semantic-versioning.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Production Build
- GitHub Check: Instrumented Build
🔇 Additional comments (3)
.github/actions/file-change/action.yml (1)
23-51: Action logic is sound with proper edge-case handling.The implementation correctly:
- Treats new branches (empty or all-zero base_commit) as having changes
- Safely handles grep with
|| trueto avoid failure when no matches are found- Applies the skip override after the main logic
- Outputs the result as a GitHub Actions-compatible string
.github/workflows/semantic-versioning.yml (1)
156-162: Step configuration aligns with new action interface.The step now correctly uses the new file-change action with appropriate inputs (base_commit, newest_commit, regex_pattern) and the step id is properly set to
changedfor referencing outputs..github/workflows/build-db-docker-image.yml (1)
14-32: Output binding and action integration are correct.The job correctly:
- Maps the action's
changedoutput to the job-leveldb_changedoutput (line 18)- Configures the file-change action with appropriate inputs including the regex pattern for schema files
- Allows downstream jobs to reference this output consistently
All subsequent conditional checks throughout the workflow reference
needs.schema-modified.outputs.db_changed, which aligns with this output definition.
Comment on lines
+1
to
+56
| name: Check for File Change | ||
| description: 'Checks whether a commit has modified files that match a given regex pattern.' | ||
|
|
||
| inputs: | ||
| base_commit: | ||
| description: 'The base_commit commit to compare against. When left empty "" it will assume its a new branch or initial commit and set `changed` to true.' | ||
| required: true | ||
| newest_commit: | ||
| description: The newest commit that the base is compared against | ||
| default: ${{ github.sha }} | ||
| required: false | ||
| regex_pattern: | ||
| description: 'Used to determinate if a file was modified within the commit-range; when it matches the pattern' | ||
| required: true | ||
| skip: | ||
| description: 'When set to true, the action will always return changed=true' | ||
| default: '0' | ||
| required: false | ||
|
|
||
| runs: | ||
| using: 'composite' | ||
| steps: | ||
| - run: | | ||
| set -euo pipefail | ||
| # For a new branch or initial commit, treat as relevant | ||
| if [ "${{ inputs.base_commit }}" = "" ] || [ "${{ inputs.base_commit }}" = "0000000000000000000000000000000000000000" ]; then | ||
| echo "New branch or initial commit. Considering changes relevant." | ||
| echo "changed=true" >> $GITHUB_OUTPUT | ||
| else | ||
| CHANGED_FILES=$(git diff --name-only ${{ inputs.base_commit }} ${{ inputs.newest_commit || github.sha }}) | ||
|
|
||
| printf "Changed files: \n%s\n\n" "$CHANGED_FILES" | ||
|
|
||
| MATCHED_FILES=$(printf '%s\n' "$CHANGED_FILES" | grep -E "${{ inputs.regex_pattern }}" || true) | ||
| if [ -n "$MATCHED_FILES" ]; then | ||
| echo "Files matching regex '${{ inputs.regex_pattern }}':" | ||
| printf '%s\n' "$MATCHED_FILES" | ||
| echo "File change(s) were detected. Setting output `changed` to true." | ||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "No changes detected." | ||
| echo "changed=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| fi | ||
|
|
||
| if [ "${{ inputs.skip }}" = "1" ] || [ "${{ inputs.skip }}" = "true" ]; then | ||
| echo "File matching has been overridden. Setting `changed` to true." | ||
| echo "changed=true" >> $GITHUB_OUTPUT | ||
| fi | ||
| shell: bash | ||
| id: check | ||
|
|
||
| outputs: | ||
| changed: | ||
| description: 'Whether relevant file changes were detected' | ||
| value: ${{ steps.check.outputs.changed }} |
Contributor
There was a problem hiding this comment.
Typos in input descriptions.
Lines 6 and 13 contain minor typos:
- Line 6: "base_commit commit" should be "base_commit" (redundant word)
- Line 13: "determinate" should be "determine"
These don't affect functionality but should be corrected for clarity.
- description: 'The base_commit commit to compare against. When left empty "" it will assume its a new branch or initial commit and set `changed` to true.'
+ description: 'The base_commit to compare against. When left empty "" it will assume its a new branch or initial commit and set `changed` to true.'- description: 'Used to determinate if a file was modified within the commit-range; when it matches the pattern'
+ description: 'Used to determine if a file was modified within the commit-range; when it matches the pattern'📝 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.
Suggested change
| name: Check for File Change | |
| description: 'Checks whether a commit has modified files that match a given regex pattern.' | |
| inputs: | |
| base_commit: | |
| description: 'The base_commit commit to compare against. When left empty "" it will assume its a new branch or initial commit and set `changed` to true.' | |
| required: true | |
| newest_commit: | |
| description: The newest commit that the base is compared against | |
| default: ${{ github.sha }} | |
| required: false | |
| regex_pattern: | |
| description: 'Used to determinate if a file was modified within the commit-range; when it matches the pattern' | |
| required: true | |
| skip: | |
| description: 'When set to true, the action will always return changed=true' | |
| default: '0' | |
| required: false | |
| runs: | |
| using: 'composite' | |
| steps: | |
| - run: | | |
| set -euo pipefail | |
| # For a new branch or initial commit, treat as relevant | |
| if [ "${{ inputs.base_commit }}" = "" ] || [ "${{ inputs.base_commit }}" = "0000000000000000000000000000000000000000" ]; then | |
| echo "New branch or initial commit. Considering changes relevant." | |
| echo "changed=true" >> $GITHUB_OUTPUT | |
| else | |
| CHANGED_FILES=$(git diff --name-only ${{ inputs.base_commit }} ${{ inputs.newest_commit || github.sha }}) | |
| printf "Changed files: \n%s\n\n" "$CHANGED_FILES" | |
| MATCHED_FILES=$(printf '%s\n' "$CHANGED_FILES" | grep -E "${{ inputs.regex_pattern }}" || true) | |
| if [ -n "$MATCHED_FILES" ]; then | |
| echo "Files matching regex '${{ inputs.regex_pattern }}':" | |
| printf '%s\n' "$MATCHED_FILES" | |
| echo "File change(s) were detected. Setting output `changed` to true." | |
| echo "changed=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "No changes detected." | |
| echo "changed=false" >> "$GITHUB_OUTPUT" | |
| fi | |
| fi | |
| if [ "${{ inputs.skip }}" = "1" ] || [ "${{ inputs.skip }}" = "true" ]; then | |
| echo "File matching has been overridden. Setting `changed` to true." | |
| echo "changed=true" >> $GITHUB_OUTPUT | |
| fi | |
| shell: bash | |
| id: check | |
| outputs: | |
| changed: | |
| description: 'Whether relevant file changes were detected' | |
| value: ${{ steps.check.outputs.changed }} | |
| name: Check for File Change | |
| description: 'Checks whether a commit has modified files that match a given regex pattern.' | |
| inputs: | |
| base_commit: | |
| description: 'The base_commit to compare against. When left empty "" it will assume its a new branch or initial commit and set `changed` to true.' | |
| required: true | |
| newest_commit: | |
| description: The newest commit that the base is compared against | |
| default: ${{ github.sha }} | |
| required: false | |
| regex_pattern: | |
| description: 'Used to determine if a file was modified within the commit-range; when it matches the pattern' | |
| required: true | |
| skip: | |
| description: 'When set to true, the action will always return changed=true' | |
| default: '0' | |
| required: false | |
| runs: | |
| using: 'composite' | |
| steps: | |
| - run: | | |
| set -euo pipefail | |
| # For a new branch or initial commit, treat as relevant | |
| if [ "${{ inputs.base_commit }}" = "" ] || [ "${{ inputs.base_commit }}" = "0000000000000000000000000000000000000000" ]; then | |
| echo "New branch or initial commit. Considering changes relevant." | |
| echo "changed=true" >> $GITHUB_OUTPUT | |
| else | |
| CHANGED_FILES=$(git diff --name-only ${{ inputs.base_commit }} ${{ inputs.newest_commit || github.sha }}) | |
| printf "Changed files: \n%s\n\n" "$CHANGED_FILES" | |
| MATCHED_FILES=$(printf '%s\n' "$CHANGED_FILES" | grep -E "${{ inputs.regex_pattern }}" || true) | |
| if [ -n "$MATCHED_FILES" ]; then | |
| echo "Files matching regex '${{ inputs.regex_pattern }}':" | |
| printf '%s\n' "$MATCHED_FILES" | |
| echo "File change(s) were detected. Setting output `changed` to true." | |
| echo "changed=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "No changes detected." | |
| echo "changed=false" >> "$GITHUB_OUTPUT" | |
| fi | |
| fi | |
| if [ "${{ inputs.skip }}" = "1" ] || [ "${{ inputs.skip }}" = "true" ]; then | |
| echo "File matching has been overridden. Setting `changed` to true." | |
| echo "changed=true" >> $GITHUB_OUTPUT | |
| fi | |
| shell: bash | |
| id: check | |
| outputs: | |
| changed: | |
| description: 'Whether relevant file changes were detected' | |
| value: ${{ steps.check.outputs.changed }} |
🤖 Prompt for AI Agents
In .github/actions/file-change/action.yml around lines 1 to 56, there are two
minor typos in the inputs descriptions: remove the redundant word in the
base_commit description so it reads "The base_commit to compare against..."
instead of "The base_commit commit to compare against", and change "determinate"
to "determine" in the regex_pattern description; update those strings
accordingly to correct the wording and save the file.
By adding the `consider_initial_commit_relevant` input one can specify whether the initial commit to a branch is considered as a change or not.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The reason for the removal of fallback-tags for the main and canary branch, when there are no available tags, is that the existence of tags is required. Hence, in case there are none an error should be displayed instead of following the branch-tag format
The reason for this is that this way the current implementation of initializing the respective database service works. Unless a new image is published while the code-changes that were made along side the schema changes are not merged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
The idea of this pull request is to both reduce code duplication within the build-database-image workflows. At the same time the workflows aims to update the usage of the
knowledgecheckr-databaseimage based on the current branch-name.This means that when the database-schema is changed on a new feature-branch --> a new docker-image is created. This image should then be used for all workflows that are executed on that branch. This way the code stays in sync with the database, e.g. when the schema was modified and the respective queries, but the new database-image is not used could cause errors.
Similarly, when a such a feature-branch is merged into the canary or main branch, the build-docker-image workflow will be triggered as well. I these cases all workflows should use either the
canaryorstabletag to use the most up-to-date and robust image.Summary
Summary by CodeRabbit