GH-51327: [CI][Dev] Change download minIO URLs for GitHub releases URL - #51326
GH-51327: [CI][Dev] Change download minIO URLs for GitHub releases URL#51326raulcd wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved CI download, image revision, and s390x asset issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates CI MinIO downloads from retired dl.min.io URLs to pinned GitHub release assets.
Changes:
- Updates MinIO and
mcdownload URL construction. - Updates the Windows test image’s MinIO download URL.
File summaries
| File | Summary | Findings |
|---|---|---|
ci/scripts/install_minio.sh |
Uses GitHub release assets for MinIO and mc. |
Critical: Windows C++ CI still uses the retired URL. Moderate: The s390x asset URL is unavailable. |
ci/docker/python-wheel-windows-test-vs2022-base.dockerfile |
Downloads the Windows MinIO binary from GitHub. | Moderate: The image revision in .env must be bumped. |
Review details
Suppressed comments (1)
ci/docker/python-wheel-windows-test-vs2022-base.dockerfile:54
- The Dockerfile explicitly requires bumping
PYTHON_WHEEL_WINDOWS_TEST_IMAGE_REVISIONwhen it changes, but.envstill has the old2026-09-09value. Because the Compose image tag is derived from that revision, CI can reuse the previously built image and never apply this new download URL; please bump the test-image revision with this change.
RUN curl -L https://github.com/minio/minio/releases/download/RELEASE.2025-01-20T14-49-07Z/minio.windows-amd64.RELEASE.2025-01-20T14-49-07Z.exe `
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rok
left a comment
rok
left a comment
There was a problem hiding this comment.
Looks good, merge if CI passes!
One question - should we include hardcoded checksum checks?
github-actions
Bot
added
awaiting merge
There was a problem hiding this comment.
🔵 Needs a closer look
The s390x path needs an explicit supported installation path or caller handling instead of silently succeeding.
Review details
Suppressed comments (1)
ci/scripts/install_minio.sh:33
- Removing
s390xhere moves a previously handled architecture into theexit 0path at lines 36-38. The pinned MinIO server release has nolinux-s390xasset, so s390x callers now continue without${prefix}/bin/minio(and potentiallymc) and only fail later or silently skip tests. Please either provide a supported s390x installation path or make this unsupported case explicit in its callers instead of silently succeeding.
[aarch64]=arm64)
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
@github-actions crossbow submit wheel-cp314 |
|
Revision: 35c116b Submitted crossbow builds: ursacomputing/crossbow @ actions-c58570452f
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved checksum verification and s390x handling issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
ci/scripts/install_minio.sh:33
- Removing the
s390xentry makes this helper silently return success at lines 36–39 without installing eitherminioormcon s390x. The script is copied into S3-enabled Linux images (for example,ci/docker/debian-13-cpp.dockerfile:98-99), so this is a CI coverage regression rather than just a URL rename; please retain a supported s390x server source or explicitly fail/disable the MinIO-dependent path.
[aarch64]=arm64)
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
|
@github-actions crossbow submit wheel-cp313 |
|
Revision: fd2d61f Submitted crossbow builds: ursacomputing/crossbow @ actions-794c2bac07
|
|
@kou Could you take a quick look at the additional shell code? Otherwise I hope we can merge quickly to fix CI. |
raulcd
changed the title
|
|
Rationale for this change
MinIO download URLs return HTTP 410. They have been removed.
There's a longer discussion about finding out a long term solution for the MinIO deprecation here:
What changes are included in this PR?
Use the exact same releases we use but use the GitHub releases download URL temporarily before moving to something else.
Are these changes tested?
Yes via archery and CI
Are there any user-facing changes?
No
AI disclosure
The mechanical changes were done with Claude Opus 5 (1M context)
Everything was reviewed by me (a human).