Copilot commented on code in PR #50082:
URL: https://github.com/apache/arrow/pull/50082#discussion_r3349373158
##########
compose.yaml:
##########
@@ -145,7 +145,6 @@ x-hierarchy:
- ubuntu-ruby
- ubuntu-python
- ubuntu-python-sdist-test
- - ubuntu-python-313-freethreading
- ubuntu-r
Review Comment:
The PR description says it only updates the quay.io manylinux/musllinux base
images, but this change also drops the ubuntu-python-313-freethreading
service/CI (and related 3.13 free-threading plumbing). Please update the PR
description (or add release notes / rationale) so reviewers understand this
additional scope.
##########
ci/scripts/python_wheel_unix_test.sh:
##########
@@ -106,13 +106,8 @@ is_free_threaded() {
if [ "${CHECK_UNITTESTS}" == "ON" ]; then
# Install testing dependencies
- if [ "$(is_free_threaded)" = "ON" ] && [[ "${PYTHON:-}" == *"3.13"* ]]; then
- echo "Free-threaded Python 3.13 build detected"
- python -m pip install -U -r
"${source_dir}/python/requirements-wheel-test-3.13t.txt"
- else
- echo "Regular Python build detected"
- python -m pip install -U -r
"${source_dir}/python/requirements-wheel-test.txt"
- fi
+ echo "Regular Python build detected"
+ python -m pip install -U -r
"${source_dir}/python/requirements-wheel-test.txt"
Review Comment:
This block no longer uses is_free_threaded(), so the helper is now dead code
in this script (only defined, never referenced). Either remove the helper to
avoid confusion, or keep it and use it for logging/branching here.
##########
ci/docker/python-free-threaded-wheel-musllinux-test-unittests.dockerfile:
##########
@@ -37,8 +37,7 @@ RUN apk update && \
# See available releases at:
https://github.com/astral-sh/python-build-standalone/releases
RUN set -e; \
case "${python_version}" in \
- 3.13) python_patch_version="3.13.9";; \
- 3.14) python_patch_version="3.14.0";; \
+ 3.14) python_patch_version="3.14.5";; \
esac && \
Review Comment:
The python_version -> python_patch_version case statement has no default
branch. If python_version is ever set to an unexpected value (e.g. via
compose/.env), the build will proceed with an empty python_patch_version and
fail later with a confusing download URL. Adding an explicit default that exits
makes failures immediate and easier to diagnose.
##########
ci/docker/python-free-threaded-wheel-musllinux-test-imports.dockerfile:
##########
@@ -35,8 +35,7 @@ RUN apk update && \
# See available releases at:
https://github.com/astral-sh/python-build-standalone/releases
RUN set -e; \
case "${python_version}" in \
- 3.13) python_patch_version="3.13.9";; \
- 3.14) python_patch_version="3.14.0";; \
+ 3.14) python_patch_version="3.14.5";; \
esac && \
Review Comment:
The python_version -> python_patch_version case statement has no default
branch. If python_version is ever set to an unexpected value (e.g. via
compose/.env), the build will proceed with an empty python_patch_version and
fail later with a confusing download URL. Adding an explicit default that exits
makes failures immediate and easier to diagnose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
