ci: reorder job steps #102
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Wdyt about these suggestions? 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
🧹 Outside diff range and nitpick comments (2)
.github/workflows/connector-tests.yml (2)
Line range hint
140-160: Consider enhancing error handling and output formatting? 🤔A few suggestions to make this more robust and user-friendly, wdyt?
- The artifact URL is referenced before the upload step runs:
- echo -e "\n[Download Job Output](${{steps.upload_job_output.outputs.artifact-url}})" >> $GITHUB_STEP_SUMMARY + # Move this line to the upload step
- We could add error handling for the file operations:
- json_output_file=$(find airbyte/airbyte-ci/connectors/pipelines/pipeline_reports -name 'output.json' -print -quit) + json_output_file=$(find airbyte/airbyte-ci/connectors/pipelines/pipeline_reports -name 'output.json' -print -quit) + if [ -z "$json_output_file" ]; then + echo "::error::No output.json file found" + exit 1 + fi
- The duration might be more readable with proper formatting:
- echo "- Test Duration: $(printf "%.0f" ${run_duration})s" >> $GITHUB_STEP_SUMMARY + duration_mins=$(printf "%.1f" $(echo "${run_duration}/60" | bc -l)) + echo "- Test Duration: ${duration_mins} minutes" >> $GITHUB_STEP_SUMMARY🧰 Tools
🪛 actionlint (1.7.4)
142-142: shellcheck reported issue in this script: SC2086:info:3:18: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:4:16: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:5:20: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:6:21: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2129:style:7:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:7:51: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:8:33: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:9:40: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:9:62: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:11:43: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: shellcheck reported issue in this script: SC2086:info:13:89: Double quote to prevent globbing and word splitting
(shellcheck)
142-142: property "upload_job_output" is not defined in object type {no_changes: {conclusion: string; outcome: string; outputs: {string => string}}}
(expression)
161-168: How about enhancing the artifact handling? 💭A couple of suggestions to improve the upload step:
- We could move the artifact URL link here and ensure it's always available:
- name: Upload Job Output id: upload_job_output if: always() && steps.no_changes.outputs.status != 'cancelled' uses: actions/upload-artifact@v4 with: name: ${{matrix.connector}}-job-output path: airbyte/airbyte-ci/connectors/pipelines/pipeline_reports + compression-level: 9 # Maximum compression to save storage + - name: Update Summary + if: always() && steps.no_changes.outputs.status != 'cancelled' + run: | + echo -e "\n[Download Job Output](${{steps.upload_job_output.outputs.artifact-url}})" >> $GITHUB_STEP_SUMMARY
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/connector-tests.yml(2 hunks)
🔇 Additional comments (1)
.github/workflows/connector-tests.yml (1)
Line range hint 140-168: Overall changes look great! 👍
The reordering of steps and addition of the evaluation step improves the workflow's output handling and readability. The changes maintain proper error handling and artifact management while adding useful test result summaries.
Summary by CodeRabbit
Bug Fixes
Chores