Skip to content

Conversation

@hiroyuki-sato
Copy link
Collaborator

@hiroyuki-sato hiroyuki-sato commented Nov 19, 2025

Rationale for this change

This is the sub issue #44748.

  • SC2048: Use "$@" (with quotes) to prevent whitespace problems.
  • SC2181: Check exit code directly with e.g. if mycmd;, not indirectly with $?.
In ci/scripts/util_download_apache.sh line 47:
for mirror in ${APACHE_MIRRORS[*]}
              ^------------------^ SC2048 (warning): Use "${array[@]}" (with quotes) to prevent whitespace problems.


In ci/scripts/util_download_apache.sh line 50:
  if [ $? == 0 ]; then
       ^-- SC2181 (style): Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

For more information:
  https://www.shellcheck.net/wiki/SC2048 -- Use "${array[@]}" (with quotes) t...
  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...

What changes are included in this PR?

  • SC2048: Use "${APACHE_MIRRORS[@]}" instead of ${APACHE_MIRRORS[*]}.
  • Sc2181: Use if cmd ; then statement instead of $?.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions
Copy link

⚠️ GitHub issue #48174 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added the awaiting review Awaiting review label Nov 19, 2025
@raulcd
Copy link
Member

raulcd commented Nov 19, 2025

@github-actions crossbow submit test-conda-python-3.10-hdfs-*

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, if you can rebase with main the linter failure should be fixed there now.
I am running the crossbow jobs for the test-conda-python-3.10-hdfs-* jobs which are the only ones that seem to use this script (to download hdfs)

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Nov 19, 2025
@github-actions
Copy link

Revision: 3091d25

Submitted crossbow builds: ursacomputing/crossbow @ actions-4a52ca93c7

Task Status
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions

@raulcd
Copy link
Member

raulcd commented Nov 19, 2025

Thanks for the PR @hiroyuki-sato !!!
The hdfs jobs are failing due to the jobs taking too long to download hadoop (~40 minutes). This seems to be cached on the docker image for the nightlies but as the script to download hadoop has changed on the PR it now has to download it again. We might want to temporarily increase the timeout for those docker files to succeed. We can add the timeout variable to the params here:

arrow/dev/tasks/tasks.yml

Lines 913 to 922 in 303d077

{% for hdfs_version in ["2.9.2", "3.2.1"] %}
test-conda-python-3.10-hdfs-{{ hdfs_version }}:
ci: github
template: docker-tests/github.linux.yml
params:
env:
HDFS: "{{ hdfs_version }}"
PYTHON: "3.10"
image: conda-python-hdfs
{% endfor %}

An example of a task with timeout here:

timeout: 90

@hiroyuki-sato hiroyuki-sato force-pushed the topic/shellcheck-util_download_apache branch from 3091d25 to db609ab Compare November 19, 2025 23:52
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Nov 19, 2025
@kou
Copy link
Member

kou commented Nov 20, 2025

@github-actions crossbow submit test-conda-python-3.10-hdfs-*

@github-actions
Copy link

Revision: db609ab

Submitted crossbow builds: ursacomputing/crossbow @ actions-7c1ea37107

Task Status
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions

@hiroyuki-sato
Copy link
Collaborator Author

@raulcd Thanks! rebased . It seems that `C++ / AMD64 macOS 15-intel C++ (pull_request) and C++ / ARM64 macOS 14 C++ (pull_request) failures don't relates to this PR.

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks for your PR! @hiroyuki-sato

@raulcd raulcd merged commit 9dfc3af into apache:main Nov 20, 2025
32 of 34 checks passed
@raulcd raulcd removed the awaiting change review Awaiting change review label Nov 20, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label Nov 20, 2025
@hiroyuki-sato hiroyuki-sato deleted the topic/shellcheck-util_download_apache branch November 20, 2025 11:49
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9dfc3af.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting merge Awaiting merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants