feat(packaging): Build connector packages locally and in CI. - #22
Closed
jackluo923 wants to merge 7 commits into
Closed
jackluo923 wants to merge 7 commits into
jackluo923 wants to merge 7 commits into
Conversation
The host-side scripts relied on GNU-only tooling that isn't available on macOS: `stat -c` and `sed -z` in build-env.sh's hash derivation, `cp --remove-destination` when copying finished artifacts, and an unguarded empty-array expansion under `set -o nounset` that only trips on bash 3.2, which is what ships as /bin/bash on macOS. Replace each with a portable equivalent so the script runs the same on both platforms.
|
Warning Review limit reached
Next review available in: 45 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds local and CI packaging for Debian, RPM, and tarball artifacts, with containerized builds, architecture-specific workflows, build caching, CA trust handling, dependency-image hashing, package specifications, and Velox dependency installation wiring. ChangesInstallable package build system
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant BuildPackages
participant BuildDependencyImage
participant BuildArtifacts
participant ArtifactStore
Developer->>BuildPackages: invoke local packaging command
BuildPackages->>BuildDependencyImage: resolve or build hashed dependency image
BuildPackages->>BuildArtifacts: run containerized artifact build
BuildArtifacts->>BuildPackages: return deb, rpm, and tar.gz artifacts
BuildPackages->>Developer: copy artifacts to output directory
ArtifactStore->>BuildArtifacts: provide CI dependency-image workflow output
BuildArtifacts->>ArtifactStore: upload architecture-specific artifacts
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/build-packages.yaml:
- Line 100: Update the reusable dependency-image workflow output consumed by the
build workflow so it emits the built platform image or manifest-list digest,
then change the image reference at the existing
needs.build-dependency-image.outputs.image usage to use the immutable `@sha256`:
digest instead of the mutable env-<hash> tag.
- Around line 128-150: Update the three upload steps—“Upload deb package,”
“Upload rpm package,” and “Upload tarball”—to use actions/upload-artifact v6.0.0
with SHA b7c566a772e6b6bfb58ed0dc250532a479d7789f, including updating their
version comments.
In `@tools/build-packages/internal/host/build-env.sh`:
- Around line 70-85: Update derive_build_env_hash to use portable command
fallbacks for null-delimited sorting, symlink resolution, and SHA-256 hashing:
select gsort or sort, greadlink or readlink, and sha256sum or shasum -a 256
based on available commands. Preserve the existing hashing output and file-mode
handling across macOS and GNU/Linux.
In `@tools/build-packages/package-specs/rpm/clp-plugin-presto-connector.spec`:
- Around line 37-43: Update the %files manifest in the RPM spec to explicitly
own the package root directory /opt/clp-plugin-presto-connector, in addition to
the existing presto_jar_dir and velox_so_dir entries. Preserve all current file
and subdirectory ownership entries so package removal also cleans up the empty
root directory.
In `@tools/build-packages/README.md`:
- Around line 88-93: Update the tarball installation example in the README so
the copy command targeting /opt explicitly uses sudo or otherwise states that
root access is required. Keep the extraction command unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 92b7579b-caee-4c31-82e2-d813e1161334
📒 Files selected for processing (26)
.dockerignore.github/workflows/build-dependency-image.yaml.github/workflows/build-packages.yaml.gitignoreREADME.mdpresto-connector/.gitignoretaskfiles/velox-connector/main.yamltools/build-packages/README.mdtools/build-packages/build-dependency-image.shtools/build-packages/build-packages.shtools/build-packages/dependency-image/Dockerfiletools/build-packages/dependency-image/use-host-ca.shtools/build-packages/dependency-image/utils.shtools/build-packages/internal/build-cache/README.mdtools/build-packages/internal/build-cache/container.shtools/build-packages/internal/build-cache/host.shtools/build-packages/internal/ca-trust/README.mdtools/build-packages/internal/ca-trust/container.shtools/build-packages/internal/ca-trust/generators/java-pkcs12/CreateJavaTrustStore.javatools/build-packages/internal/ca-trust/generators/java-pkcs12/README.mdtools/build-packages/internal/ca-trust/generators/java-pkcs12/generate.shtools/build-packages/internal/ca-trust/host.shtools/build-packages/internal/container/build-artifacts.shtools/build-packages/internal/host/build-env.shtools/build-packages/package-specs/deb/clp-plugin-presto-connector.control.intools/build-packages/package-specs/rpm/clp-plugin-presto-connector.spec
💤 Files with no reviewable changes (2)
- tools/build-packages/dependency-image/use-host-ca.sh
- tools/build-packages/dependency-image/utils.sh
v6 only bumps the action's runtime from Node 20 to Node 24, which doesn't affect the GitHub-hosted runners this workflow uses; inputs and outputs are unchanged from v4.6.2.
Older macOS releases don't ship sha256sum by default, only shasum -a 256, which produces byte-identical output so the derived hash still matches what CI computes with sha256sum on Linux.
The example copied straight into /opt/clp-plugin-presto-connector/, which needs root and doesn't distinguish coordinator from worker hosts. Document it as a placeholder plugin directory and role instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/build-packages/README.md`:
- Line 92: Update the coordinator/worker installation instructions in the README
to provide separate, directly executable shell commands for each role. Replace
the invalid “<coordinator|worker>” syntax and angle-bracket path placeholder
with the appropriate concrete source names and a normal placeholder path that
users can substitute safely.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e5d75958-4b2d-469e-a0c3-32d23cab5dd2
📒 Files selected for processing (3)
.github/workflows/build-packages.yamltools/build-packages/README.mdtools/build-packages/internal/host/build-env.sh
Verified by install/remove in a container: /opt/clp-plugin-presto-connector was left behind as an empty directory after rpm -e, since rpm only tracks paths declared with %dir. Add a plugin_root macro alongside the existing presto_jar_dir/velox_so_dir ones instead of hardcoding the path, since both remain independently overridable via env.
Member
Author
|
Split this PR into multiple smaller PR |
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.
Description
Add a local and CI packaging pipeline for the CLP Presto connector. The pipeline builds the coordinator JAR and Velox worker plugin once per architecture, stages a shared payload, and emits installable Debian, RPM, and relocatable tar packages for
amd64andarm64.The change also:
manylinux_2_28build environment that CI publishes to GHCR and reuses across package builds.Validation performed
build-packagesworkflow completed successfully at the current branch tip:amd64andarm64.The successful workflow currently reports a Node.js 20 deprecation warning for the SHA-pinned
actions/upload-artifactv4.6.2 action; GitHub runs it under the Node.js 24 compatibility path.Checklist
Summary by CodeRabbit
.deb,.rpm, and.tar.gzconnector artefacts for AMD64 and ARM64, with a dedicated build entry point.