Skip to content

Add --push flag to install_tpchgen_cli.sh to publish builder image - #368

Open
misiugodfrey wants to merge 2 commits into
mainfrom
misiug/installTPCHgen
Open

Add --push flag to install_tpchgen_cli.sh to publish builder image#368
misiugodfrey wants to merge 2 commits into
mainfrom
misiug/installTPCHgen

Conversation

@misiugodfrey

@misiugodfrey misiugodfrey commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a --push flag to install_tpchgen_cli.sh that tags the tpchgen-cli-builder image as ghcr.io/rapidsai/velox-testing-images:tpchgen-cli and pushes it to GHCR, instead of always deleting it after extracting the binary.

The motivation for this change is to enable the use of this binary in environments that don't support docker for a local build. Our Slurm environments can pull the docker image from the registry as an Enroot image and get access to the binary that way; but can't build it directly.

Test plan

  • Run install_tpchgen_cli.sh without --push and confirm the builder image is still cleaned up as before
  • Run install_tpchgen_cli.sh --push and confirm the image is tagged and pushed to ghcr.io/rapidsai/velox-testing-images:tpchgen-cli

Allows tagging and pushing the tpchgen-cli-builder image to GHCR instead of always discarding it after extracting the binary.
@misiugodfrey
misiugodfrey requested a review from a team as a code owner July 2, 2026 22:11

@TomAugspurger TomAugspurger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

One thing to maybe think about is whether you'll need some kind of cross-compilation for different CPU architectures. But if not, then there's no need to complicate things.

docker rm "$CONTAINER_ID"
docker rmi "$IMAGE_NAME"

if [[ "$PUSH" == true ]]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final output of this script is the tpchgen-cli binary. Why push the entire docker image in this case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the simplest workflow on a slurm cluster is to point at an external docker registry and pyxis/enroot auto-convert the container to a local enroot image.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I am missing something, copying over the binary to local_installs/bin and then running the datagen script seems much simpler than the above process?

@misiugodfrey
misiugodfrey requested a review from paul-aiyedun July 8, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants