CI: add pgvector CI workflow#14
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow that builds Apache Cloudberry, packages build artifacts, and runs pgvector regression tests against a fresh Cloudberry demo cluster. ChangesCI Workflow for Cloudberry + pgvector
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant build_cloudberry
participant test_pgvector
participant CloudberryCluster
GitHubActions->>build_cloudberry: checkout and build Cloudberry
build_cloudberry->>GitHubActions: upload source and install artifacts
GitHubActions->>test_pgvector: download Cloudberry artifacts
test_pgvector->>CloudberryCluster: create demo cluster
test_pgvector->>CloudberryCluster: build and install pgvector via pg_config
test_pgvector->>CloudberryCluster: run make installcheck for regression
test_pgvector->>GitHubActions: write PASS/FAIL summary
test_pgvector->>GitHubActions: upload test-logs on failure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/pgvector-cloudberry-ci.yml (4)
59-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin the container image away from
:latest.Both jobs use
apache/incubator-cloudberry:cbdb-build-rocky9-latest. An unpinnedlatesttag can change underneath the workflow and silently break CI builds without any change in this repo.Please check whether
apache/incubator-cloudberrypublishes dated/versioned tags (or digests) suitable for pinning here.Also applies to: 172-179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pgvector-cloudberry-ci.yml around lines 59 - 67, The workflow jobs are using the mutable apache/incubator-cloudberry:cbdb-build-rocky9-latest image, which should be pinned to a stable version or digest. Update the container.image in both affected jobs to a dated/versioned tag (or an explicit digest) published by apache/incubator-cloudberry, and keep the existing job setup unchanged.
31-42: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReconsider
editedPR trigger withcancel-in-progress: false.
pull_requesttriggers onedited(title/body/base changes, not just new commits) whilecancel-in-progress: falsemeans each trigger queues a full (up to 180-minute) build rather than replacing an in-flight one. Repeated PR metadata edits can pile up long-running, redundant jobs.♻️ Suggested tweak
pull_request: branches: [ main ] - types: [ opened, synchronize, reopened, edited, ready_for_review ] + types: [ opened, synchronize, reopened, ready_for_review ] workflow_dispatch: concurrency: group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: false + cancel-in-progress: true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pgvector-cloudberry-ci.yml around lines 31 - 42, The workflow trigger configuration is queuing redundant long-running PR builds because the `pull_request` event includes `edited` while `concurrency.cancel-in-progress` is set to false. Update the trigger rules in the workflow so metadata-only edits do not start a fresh full run, or change the concurrency behavior so a new run replaces the in-flight one; keep the fix centered on the existing `pull_request` and `concurrency` settings in the workflow file.
59-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSubstantial duplication between
build_cloudberryandtest_pgvectorjobs.The container definition, "Free Disk Space" step, and "Cloudberry Environment Initialization" step are duplicated verbatim across both jobs. Consider extracting these into a composite action (or a reusable workflow) to avoid drift when one copy is updated but not the other.
Also applies to: 69-89, 99-116, 172-179, 187-207, 234-251
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pgvector-cloudberry-ci.yml around lines 59 - 67, The `build_cloudberry` and `test_pgvector` jobs duplicate the same container setup, Free Disk Space step, and Cloudberry Environment Initialization step, which risks drift. Extract the shared setup into a reusable composite action or reusable workflow, then have both jobs call that shared logic instead of keeping separate copies. Use the existing job sections and the duplicated step names as the main locations to refactor.
300-326: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUnreachable code after
casestatement.Both
regression)and the default*)branch callexitdirectly, sotest_status=${PIPESTATUS[0]},set -e,popd, and the trailingexit ${test_status}(lines 322-326) can never execute.popdnever runs either, though it's harmless since the process exits first. Harmless today but confusing if another test target is added later without noticing exits bypass the fallthrough path.♻️ Suggested cleanup
- set +e case "${TEST_TARGET}" in regression) echo "=== Running pgvector regression tests (make installcheck) ===" # pg_regress connects to the running demo cluster via PG* env vars. + set +e make PG_CONFIG=$(which pg_config) installcheck 2>&1 | tee "${TEST_LOG_ROOT}/pgvector-installcheck.log" rc=${PIPESTATUS[0]} + set -e # Preserve regression diffs / results for debugging. cp -a regression.diffs "${TEST_LOG_ROOT}/regression.diffs" 2>/dev/null || true cp -a regression.out "${TEST_LOG_ROOT}/regression.out" 2>/dev/null || true cp -a results "${TEST_LOG_ROOT}/results" 2>/dev/null || true if [ "${rc}" -ne 0 ]; then echo "=== regression.diffs ===" cat regression.diffs 2>/dev/null || true fi exit ${rc} ;; *) echo "unknown test target: ${TEST_TARGET}" exit 2 ;; esac - test_status=${PIPESTATUS[0]} - set -e popd - - exit ${test_status} SCRIPT🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pgvector-cloudberry-ci.yml around lines 300 - 326, The `case` block in the workflow step exits from every branch, so the `test_status` assignment, `set -e`, `popd`, and trailing `exit` are unreachable and should be removed or restructured. Update the shell logic around the `case "${TEST_TARGET}"` handling so `regression)` and `*)` do not bypass the shared cleanup path, and keep the flow centered on the existing `test_status`/`popd` section if you need post-case cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pgvector-cloudberry-ci.yml:
- Around line 90-97: The checkout steps for Cloudberry and pgvector are leaving
persisted Git credentials in .git/config, which can be packaged into the tarred
workspace and uploaded artifact. Update both actions/checkout usages in the
workflow to set persist-credentials: false so the token is not written into the
checked-out repositories. Use the existing checkout steps that reference
env.CLOUDBERRY_REPO, env.CLOUDBERRY_REF, and the pgvector checkout block to
apply the fix consistently.
- Around line 335-338: The coordinator log collection block is unreachable
because MASTER_DATA_DIRECTORY is only set inside the child gpadmin shell, not in
this outer workflow step. Update the log collection logic in the workflow so it
runs in the same shell/session that sources gpdemo-env.sh or otherwise captures
MASTER_DATA_DIRECTORY from that context, and keep the copy step tied to the
existing Cloudberry log collection block so gpdb-log is actually populated on
failures.
---
Nitpick comments:
In @.github/workflows/pgvector-cloudberry-ci.yml:
- Around line 59-67: The workflow jobs are using the mutable
apache/incubator-cloudberry:cbdb-build-rocky9-latest image, which should be
pinned to a stable version or digest. Update the container.image in both
affected jobs to a dated/versioned tag (or an explicit digest) published by
apache/incubator-cloudberry, and keep the existing job setup unchanged.
- Around line 31-42: The workflow trigger configuration is queuing redundant
long-running PR builds because the `pull_request` event includes `edited` while
`concurrency.cancel-in-progress` is set to false. Update the trigger rules in
the workflow so metadata-only edits do not start a fresh full run, or change the
concurrency behavior so a new run replaces the in-flight one; keep the fix
centered on the existing `pull_request` and `concurrency` settings in the
workflow file.
- Around line 59-67: The `build_cloudberry` and `test_pgvector` jobs duplicate
the same container setup, Free Disk Space step, and Cloudberry Environment
Initialization step, which risks drift. Extract the shared setup into a reusable
composite action or reusable workflow, then have both jobs call that shared
logic instead of keeping separate copies. Use the existing job sections and the
duplicated step names as the main locations to refactor.
- Around line 300-326: The `case` block in the workflow step exits from every
branch, so the `test_status` assignment, `set -e`, `popd`, and trailing `exit`
are unreachable and should be removed or restructured. Update the shell logic
around the `case "${TEST_TARGET}"` handling so `regression)` and `*)` do not
bypass the shared cleanup path, and keep the flow centered on the existing
`test_status`/`popd` section if you need post-case cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68e2a379-75b1-4b24-aac1-e334c2e1482e
📒 Files selected for processing (1)
.github/workflows/pgvector-cloudberry-ci.yml
fix #ISSUE_Number
Change logs
Contributor's checklist
Here are some reminders before you submit your pull request:
Summary by CodeRabbit
Tests
Chores