Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-42006: [CI][Python] Use pip install -e instead of setup.py build_ext --inplace for installing pyarrow on verification script #42007

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Jun 6, 2024

Rationale for this change

Due to #37929 we require a higher version of setuptools and setuptools_scm to be installed otherwise the job fails with setuptools_scm failing with TypeError: Configuration.__init__() got an unexpected keyword argument 'version_file'

What changes are included in this PR?

Remove the dependencies for the environment and let installation handle those using pip install -e instead of setup.py build_ext --inplace for installing pyarrow on verification script

Are these changes tested?

Via Archery

Are there any user-facing changes?

No

@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

@github-actions crossbow submit verify-rc-source-python-*

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Jun 6, 2024

This comment was marked as outdated.

@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

I was missing dev tags on my fork that's why the jobs failed to generate the correct version, I've pushed them and will re-run

@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

@github-actions crossbow submit verify-rc-source-python-*

Copy link

github-actions bot commented Jun 6, 2024

Revision: ec8ce7e

Submitted crossbow builds: ursacomputing/crossbow @ actions-d9b2b774f4

Task Status
verify-rc-source-python-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-python-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-python-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-python-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-python-macos-amd64 GitHub Actions
verify-rc-source-python-macos-arm64 GitHub Actions
verify-rc-source-python-macos-conda-amd64 GitHub Actions

@@ -756,7 +756,7 @@ test_python() {
show_header "Build and test Python libraries"

# Build and test Python
maybe_setup_virtualenv "cython>=0.29.31" numpy "setuptools_scm<8.0.0" setuptools
maybe_setup_virtualenv "cython>=0.29.31" numpy "setuptools_scm>=8.0.0" "setuptools>=64"
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, for release verification I think we should actually remove this line entirely, and rely on the fact that we specify those dependencies in pyproject.toml (i.e. replacing the python setup.py build_ext --inplace below with python -m pip install (-e) .

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jun 6, 2024
@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

The verify-rc-source-python-linux-almalinux-8-amd64 are unrelated and are failing on some python gandiva tests.

@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

@github-actions crossbow submit verify-rc-source-python-linux-ubuntu-20.04-amd64

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Revision: f098baf

Submitted crossbow builds: ursacomputing/crossbow @ actions-75636dfc2c

Task Status
verify-rc-source-python-linux-ubuntu-20.04-amd64 GitHub Actions

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jun 6, 2024
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

@github-actions crossbow submit verify-rc-source-python-linux-ubuntu-20.04-amd64

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Revision: f39efac

Submitted crossbow builds: ursacomputing/crossbow @ actions-ac4eb35fba

Task Status
verify-rc-source-python-linux-ubuntu-20.04-amd64 GitHub Actions

@raulcd
Copy link
Member Author

raulcd commented Jun 6, 2024

I don't think running the other extra CI is required

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

The verify-rc-source-python-linux-almalinux-8-amd64 failure is #39695.
Workaround:

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jun 7, 2024
@raulcd raulcd changed the title GH-42006: [CI][Python] Install required setuptools_scm>=8.0.0 and setuptools>=64 for Python verification GH-42006: [CI][Python] Use pip install -e instead of setup.py build_ext --inplace for installing pyarrow on verification script Jun 7, 2024
@raulcd
Copy link
Member Author

raulcd commented Jun 7, 2024

👍 Updated title + description with final fix!

@raulcd raulcd merged commit a045770 into apache:main Jun 7, 2024
8 checks passed
@raulcd raulcd removed the awaiting merge Awaiting merge label Jun 7, 2024
@raulcd raulcd deleted the GH-42006 branch June 7, 2024 08:17
Copy link

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants