Include PG 18 and 19 in the test matrix#42
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR replaces the pgEdge CLI-based node image and entrypoint with a source-built PostgreSQL/Spock setup driven by resolved release tags, converts entrypoint.sh to direct SQL/initdb/pg_ctl provisioning, updates Dockerfile.tester's CMD form, removes an unused env var, and reworks the CI workflow to resolve tags and build images with docker/build-push-action across PostgreSQL 16-19. ChangesSource-built image and SQL entrypoint
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
Up to standards ✅🟢 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 medium (1 false positive) |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
ab04acc to
0ee590b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docker/entrypoint.sh (2)
101-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: quote the path expansion (SC2086).
${PG_VER}is unquoted inside the binary path. Harmless with numeric versions, but quoting silences the linter and hardens against word splitting/globbing.Proposed tweak
-exec /home/pgedge/pgedge/pg${PG_VER}/bin/postgres -D "$PGDATA" 2>&1 +exec "/home/pgedge/pgedge/pg${PG_VER}/bin/postgres" -D "$PGDATA" 2>&1🤖 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 `@docker/entrypoint.sh` at line 101, The postgres exec in the entrypoint script is using an unquoted PG_VER expansion inside the binary path, which triggers SC2086. Update the exec command in the entrypoint logic to quote the version expansion within the pg${PG_VER} path while keeping the same postgres invocation and PGDATA argument.Source: Linters/SAST tools
70-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSubscription wiring hardcodes exactly two peers.
The wait loop above iterates over all
peer_names, but this heredoc only referencespeer_names[0]andpeer_names[1]. With the current 3-node topology this is fine, but any node configured with a differentPEER_NAMEScount would silently skip peers or emit an emptyhost=(aborting underON_ERROR_STOP=1). Consider generating these statements in the loop to match the peer set.🤖 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 `@docker/entrypoint.sh` around lines 70 - 75, The subscription setup in the entrypoint hardcodes only two peers, while the earlier wait loop already handles the full peer list. Update the heredoc logic in the subscription wiring block to iterate over peer_names dynamically and generate the spock.sub_create and spock.sub_add_repset statements for each peer, using the existing peer_names array and HOSTNAME-based subscription naming so it stays consistent for any PEER_NAMES count.
🤖 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/workflow.yml:
- Line 64: The tag validation in the workflow does not reliably fail when PG_TAG
is empty because the current chained test depends on set -e behavior. Update the
validation step in the workflow job to use an explicit conditional with a clear
exit path, checking both PG_TAG and SPOCK_TAG symmetrically before proceeding to
the build. Keep the fix localized to the existing tag-check step so the later
build job only runs after both tags are confirmed present.
---
Nitpick comments:
In `@docker/entrypoint.sh`:
- Line 101: The postgres exec in the entrypoint script is using an unquoted
PG_VER expansion inside the binary path, which triggers SC2086. Update the exec
command in the entrypoint logic to quote the version expansion within the
pg${PG_VER} path while keeping the same postgres invocation and PGDATA argument.
- Around line 70-75: The subscription setup in the entrypoint hardcodes only two
peers, while the earlier wait loop already handles the full peer list. Update
the heredoc logic in the subscription wiring block to iterate over peer_names
dynamically and generate the spock.sub_create and spock.sub_add_repset
statements for each peer, using the existing peer_names array and HOSTNAME-based
subscription naming so it stays consistent for any PEER_NAMES count.
🪄 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
Run ID: 8c65667c-a0f8-4eca-90d7-d95fac671e56
📒 Files selected for processing (5)
.github/workflows/workflow.ymldocker/Dockerfile.el9docker/Dockerfile.testerdocker/entrypoint.shdocker/pgedge.env
💤 Files with no reviewable changes (1)
- docker/pgedge.env
0ee590b to
12a5348
Compare
The pgedge CLI is deprecated (and only supports PostgreSQL 15-17), so the docker harness now compiles PostgreSQL from the latest REL_x_y tag of the requested major version, applies the Spock PG patches, and builds Spock from its latest vMAJOR.MINOR.PATCH release tag, following the spock repo's tests/docker approach. Everything installs into the same /home/pgedge/pgedge/pg$VER layout the CLI used, so the compose healthchecks and test suites are unchanged. The entrypoint replaces `pgedge setup` and the `pgedge spock ...` commands with initdb and the equivalent spock SQL calls. Because spock.sub_create connects to the provider synchronously, and every node restarts into its final foreground postgres at the end of its own setup, sub_create is retried until the peer is reachable, with an existence check so retries stay idempotent; otherwise a sub_create landing in a peer's stop/start window kills the node via ON_ERROR_STOP. The workflow resolves the exact PostgreSQL and Spock release tags on the runner and passes them as build args, with GitHub Actions docker layer caching scoped per PG version, so PostgreSQL and Spock are only recompiled when a new release is tagged or the Dockerfile changes. Verified green on PG 18 (3-node replication plus the javascript, junit, and python suites).
Spock's PG 19 support is merged on main but not yet in a release, so the PG 19 matrix entry builds Spock from the main branch, pinned to its tip SHA at run time so the docker layer cache is invalidated by every Spock merge. Drop the spock_ref override once a Spock release includes PG 19 support. Note the trade-off of tracking Spock main: a breaking change on Spock main can fail lolor's PG 19 job independently of the PR under test. The job is required anyway, since we are actively developing against PG 19 and want to see breakage early; if Spock main breaks, rerun after the fix lands or temporarily pin spock_ref to a known-good SHA. PostgreSQL itself has no stable REL_19_x tag yet, so tag resolution now falls back to the newest RC, then BETA, when a major has no stable minor release. Stable tags win automatically once REL_19_0 is cut. Verified green locally on 19beta1 with Spock main (3-node replication plus the javascript, junit, and python suites).
12a5348 to
fbf9a86
Compare
No description provided.