Skip to content

MINOR: [Python] Deduplicate ensure_s3_initialized() call - #38451

Merged
kou merged 1 commit into
apache:mainfrom
pentschev:deduplicate-s3init-call
Oct 25, 2023
Merged

MINOR: [Python] Deduplicate ensure_s3_initialized() call#38451
kou merged 1 commit into
apache:mainfrom
pentschev:deduplicate-s3init-call

Conversation

@pentschev

Copy link
Copy Markdown
Contributor

Rationale for this change

#38375 introduced duplicate calls to ensure_s3_initialized().

What changes are included in this PR?

Deduplicates call to ensure_s3_initialized().

Are these changes tested?

Yes, covered by existing S3 tests.

Are there any user-facing changes?

No.

@pentschev

Copy link
Copy Markdown
ContributorAuthor

@kou as per #38375 (comment), here's the PR to deduplicate the ensure_s3_initialized(). Thanks for catching that!

@kou

kou commented Oct 25, 2023

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g python -g wheel

@github-actions

Copy link
Copy Markdown

Revision: 4023801

Submitted crossbow builds: ursacomputing/crossbow @ actions-89f99927a7

TaskStatus
test-conda-python-3.10Github Actions
test-conda-python-3.10-cython2Github Actions
test-conda-python-3.10-hdfs-2.9.2Github Actions
test-conda-python-3.10-hdfs-3.2.1Github Actions
test-conda-python-3.10-pandas-latestGithub Actions
test-conda-python-3.10-pandas-nightlyGithub Actions
test-conda-python-3.10-spark-v3.5.0Github Actions
test-conda-python-3.10-substraitGithub Actions
test-conda-python-3.11Github Actions
test-conda-python-3.11-dask-latestGithub Actions
test-conda-python-3.11-dask-upstream_develGithub Actions
test-conda-python-3.11-hypothesisGithub Actions
test-conda-python-3.11-pandas-upstream_develGithub Actions
test-conda-python-3.11-spark-masterGithub Actions
test-conda-python-3.12Github Actions
test-conda-python-3.8Github Actions
test-conda-python-3.8-pandas-1.0Github Actions
test-conda-python-3.8-spark-v3.5.0Github Actions
test-conda-python-3.9Github Actions
test-conda-python-3.9-pandas-latestGithub Actions
test-cuda-pythonGithub Actions
test-debian-11-python-3Azure
test-fedora-35-python-3Azure
test-ubuntu-20.04-python-3Azure
test-ubuntu-22.04-python-3Github Actions
wheel-macos-big-sur-cp310-arm64Github Actions
wheel-macos-big-sur-cp311-arm64Github Actions
wheel-macos-big-sur-cp312-arm64Github Actions
wheel-macos-big-sur-cp38-arm64Github Actions
wheel-macos-big-sur-cp39-arm64Github Actions
wheel-macos-mojave-cp310-amd64Github Actions
wheel-macos-mojave-cp311-amd64Github Actions
wheel-macos-mojave-cp312-amd64Github Actions
wheel-macos-mojave-cp38-amd64Github Actions
wheel-macos-mojave-cp39-amd64Github Actions
wheel-manylinux-2-28-cp310-amd64Github Actions
wheel-manylinux-2-28-cp310-arm64Github Actions
wheel-manylinux-2-28-cp311-amd64Github Actions
wheel-manylinux-2-28-cp311-arm64Github Actions
wheel-manylinux-2-28-cp312-amd64Github Actions
wheel-manylinux-2-28-cp312-arm64Github Actions
wheel-manylinux-2-28-cp38-amd64Github Actions
wheel-manylinux-2-28-cp38-arm64Github Actions
wheel-manylinux-2-28-cp39-amd64Github Actions
wheel-manylinux-2-28-cp39-arm64Github Actions
wheel-manylinux-2014-cp310-amd64Github Actions
wheel-manylinux-2014-cp310-arm64Github Actions
wheel-manylinux-2014-cp311-amd64Github Actions
wheel-manylinux-2014-cp311-arm64Github Actions
wheel-manylinux-2014-cp312-amd64Github Actions
wheel-manylinux-2014-cp312-arm64Github Actions
wheel-manylinux-2014-cp38-amd64Github Actions
wheel-manylinux-2014-cp38-arm64Github Actions
wheel-manylinux-2014-cp39-amd64Github Actions
wheel-manylinux-2014-cp39-arm64Github Actions
wheel-windows-cp310-amd64Github Actions
wheel-windows-cp311-amd64Github Actions
wheel-windows-cp312-amd64Github Actions
wheel-windows-cp38-amd64Github Actions
wheel-windows-cp39-amd64Github Actions

@bdice

bdice commented Oct 25, 2023

Copy link
Copy Markdown

The failing Appveyor tests look similar to ones I just encountered. These may be solved by this PR to update the s3fstype_name strings used to determine which tests should be xfail'd, which changed with the recent release of s3fs 2023.10.0. #38455 I documented my findings here: conda-forge/arrow-cpp-feedstock#1211 (comment) and replied on the issue for PR #38455.

kou
kou approved these changes Oct 25, 2023

@koukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

We can ignore the AppVeyor failure in this PR.

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Oct 25, 2023
@kou
kou merged commit ff7b3bb into apache:mainOct 25, 2023
@koukou removed the awaiting merge Awaiting merge label Oct 25, 2023
@jakirkham

Copy link
Copy Markdown

Thanks all! 🙏

@conbench-apache-arrow

Copy link
Copy Markdown

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

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.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
)
### Rationale for this change
apache#38375 introduced duplicate calls to `ensure_s3_initialized()`.
### What changes are included in this PR?
Deduplicates call to `ensure_s3_initialized()`.
### Are these changes tested?
Yes, covered by existing S3 tests.
### Are there any user-facing changes?
No.
Authored-by: Peter Andreas Entschev <peter@entschev.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
)
### Rationale for this change
apache#38375 introduced duplicate calls to `ensure_s3_initialized()`.
### What changes are included in this PR?
Deduplicates call to `ensure_s3_initialized()`.
### Are these changes tested?
Yes, covered by existing S3 tests.
### Are there any user-facing changes?
No.
Authored-by: Peter Andreas Entschev <peter@entschev.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pentschev@kou@bdice@jakirkham