-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move "upload binlogs" CI step earlier in pipeline and run even if failed #273
Move "upload binlogs" CI step earlier in pipeline and run even if failed #273
Conversation
📝 WalkthroughWalkthroughThe changes in the pull request involve modifications to the GitHub Actions workflow configuration in Changes
Sequence Diagram(s)sequenceDiagram
participant CI as Continuous Integration
participant Test as Testing Phase
participant Upload as Upload Binlogs
participant Results as Upload Test Results
CI->>Test: Run Tests
Test-->>CI: Test Results
CI->>Upload: Upload Binlogs
Upload-->>CI: Binlogs Uploaded
CI->>Results: Upload Test Results
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 (
|
Code Climate has analyzed commit 2a2f3bb and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
.github/workflows/main.yml
(1 hunks)
🔇 Additional comments (2)
.github/workflows/main.yml (2)
69-76
: Verify binlog generation path
Ensure the upload path captures all generated binlogs, including the build.release.binlog from the build step.
Let's verify the binlog generation paths:
✅ Verification successful
Binlog paths are correctly configured
The binlog generation and upload paths are properly aligned:
- Build step generates binlog at
./artifacts/logs/release/build.release.binlog
- Upload step captures
./artifacts/logs
directory, which includes the generated binlog
The if-no-files-found: error
setting ensures the workflow fails if no logs are present, providing additional validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for commands that generate binlog files
rg -A 2 "/bl:|/binlog:" .github/workflows/main.yml
Length of output: 181
Script:
#!/bin/bash
# Let's verify if the upload path matches the binlog generation path
rg -A 5 "path: .*logs" .github/workflows/main.yml
Length of output: 213
69-76
: Verify artifact upload sequence
The placement ensures binlogs are available early while maintaining a logical upload sequence with other artifacts (test results, coverage, packages).
Let's verify there's no duplicate binlog uploads:
✅ Verification successful
Artifact upload sequence is well-organized and non-redundant
The binlogs upload step is correctly placed and there are no duplicate binlog uploads. The sequence is logical:
- Binlogs are uploaded first (available early for debugging)
- Test-related artifacts follow (received files, SARIF, test reports, coverage)
- Build packages are uploaded last
Each artifact upload has its own distinct purpose and path:
./artifacts/logs
for binlogs**/*.received.*
for test verification./artifacts/obj/**/*.sarif
for SARIF filesartifacts/TestResults/**/*.trx
for test reportsartifacts/TestResults/coverage/**
for coverage./artifacts/package
for packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other upload steps that might handle binlog files
rg -A 5 "upload.*binlog|path:.*binlog" .github/workflows/main.yml
Length of output: 67
Script:
#!/bin/bash
# Let's search for all artifact upload steps to analyze the sequence
rg -B 2 -A 5 "uses: actions/upload-artifact" .github/workflows/main.yml
Length of output: 1361
Currently, the "upload binlogs" step is too far down in the CI pipeline (and also doesn't run if the build fails). Move it up a bit so that the order of log upload mirrors the order of actions and add
success() || failure()
so they are uploaded even during build failures.