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-40268: [Archery] Bump the version of pygit2, adapt to API changes #40269

Merged
merged 7 commits into from
Mar 1, 2024

Conversation

jonkeane
Copy link
Member

@jonkeane jonkeane commented Feb 27, 2024

Rationale for this change

archery crossbow submit ... fails with newer versions of pygit2

What changes are included in this PR?

Adapt away from deprecated [sic] APIs in pygit2 to ones that work with current versions, bump the pin

Are these changes tested?

Manually, yes, I can use archery crossbow submit ... again. CI will run using archery in a bunch of places on this PR too.

Are there any user-facing changes?

No

Copy link

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

@github-actions github-actions bot added the awaiting committer review Awaiting committer review label Feb 27, 2024
@jonkeane jonkeane requested review from kszucs and raulcd and removed request for kou, raulcd and assignUser February 27, 2024 23:35
@kou
Copy link
Member

kou commented Feb 27, 2024

It seems that pygit 1.14 requires Python 3.9 or later.
How about using recent Python (Python 3.12?) for Archery?

@jonkeane
Copy link
Member Author

How about using recent Python (Python 3.12?) for Archery?

Unless I'm missing something obvious, it looks like this actually would mean bumping python versions in a bunch of places. I wonder if we should instead either not require 3.9 except for archery[crossbow] or maybe pin to a lower version of pygit2 instead. What do folks think?

@kou
Copy link
Member

kou commented Feb 28, 2024

It's not a strong opinion but it may be a good time to update Python version for Archery.
Python 3.8 will reach EOL on 2024-10: https://devguide.python.org/versions/

@pitrou
Copy link
Member

pitrou commented Feb 28, 2024

I agree with requiring Python 3.9 on Archery-related CI jobs.

Edit: oh, I see you already did this. Thank you!

@pitrou
Copy link
Member

pitrou commented Feb 28, 2024

3 of the 4 failing PR checks are due to missing Python version bumps, can you take a look @jonkeane ?

@pitrou
Copy link
Member

pitrou commented Feb 28, 2024

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

Copy link

Revision: c7e1e2a

Submitted crossbow builds: ursacomputing/crossbow @ actions-730595ee2c

Task Status
test-conda-python-3.10 GitHub Actions
test-conda-python-3.10-cython2 GitHub Actions
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions
test-conda-python-3.10-pandas-latest GitHub Actions
test-conda-python-3.10-pandas-nightly GitHub Actions
test-conda-python-3.10-spark-v3.5.0 GitHub Actions
test-conda-python-3.10-substrait GitHub Actions
test-conda-python-3.11 GitHub Actions
test-conda-python-3.11-dask-latest GitHub Actions
test-conda-python-3.11-dask-upstream_devel GitHub Actions
test-conda-python-3.11-hypothesis GitHub Actions
test-conda-python-3.11-pandas-upstream_devel GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.12 GitHub Actions
test-conda-python-3.8 GitHub Actions
test-conda-python-3.8-pandas-1.0 GitHub Actions
test-conda-python-3.8-spark-v3.5.0 GitHub Actions
test-conda-python-3.9 GitHub Actions
test-conda-python-3.9-pandas-latest GitHub Actions
test-cuda-python GitHub Actions
test-debian-11-python-3-amd64 Azure
test-debian-11-python-3-i386 GitHub Actions
test-fedora-39-python-3 Azure
test-ubuntu-20.04-python-3 Azure
test-ubuntu-22.04-python-3 GitHub Actions
wheel-macos-big-sur-cp310-arm64 GitHub Actions
wheel-macos-big-sur-cp311-arm64 GitHub Actions
wheel-macos-big-sur-cp312-arm64 GitHub Actions
wheel-macos-big-sur-cp38-arm64 GitHub Actions
wheel-macos-big-sur-cp39-arm64 GitHub Actions
wheel-macos-catalina-cp310-amd64 GitHub Actions
wheel-macos-catalina-cp311-amd64 GitHub Actions
wheel-macos-catalina-cp312-amd64 GitHub Actions
wheel-macos-catalina-cp38-amd64 GitHub Actions
wheel-macos-catalina-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-arm64 GitHub Actions
wheel-manylinux-2-28-cp311-amd64 GitHub Actions
wheel-manylinux-2-28-cp311-arm64 GitHub Actions
wheel-manylinux-2-28-cp312-amd64 GitHub Actions
wheel-manylinux-2-28-cp312-arm64 GitHub Actions
wheel-manylinux-2-28-cp38-amd64 GitHub Actions
wheel-manylinux-2-28-cp38-arm64 GitHub Actions
wheel-manylinux-2-28-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp39-arm64 GitHub Actions
wheel-manylinux-2014-cp310-amd64 GitHub Actions
wheel-manylinux-2014-cp310-arm64 GitHub Actions
wheel-manylinux-2014-cp311-amd64 GitHub Actions
wheel-manylinux-2014-cp311-arm64 GitHub Actions
wheel-manylinux-2014-cp312-amd64 GitHub Actions
wheel-manylinux-2014-cp312-arm64 GitHub Actions
wheel-manylinux-2014-cp38-amd64 GitHub Actions
wheel-manylinux-2014-cp38-arm64 GitHub Actions
wheel-manylinux-2014-cp39-amd64 GitHub Actions
wheel-manylinux-2014-cp39-arm64 GitHub Actions
wheel-windows-cp310-amd64 GitHub Actions
wheel-windows-cp311-amd64 GitHub Actions
wheel-windows-cp312-amd64 GitHub Actions
wheel-windows-cp38-amd64 GitHub Actions
wheel-windows-cp39-amd64 GitHub Actions

@jonkeane
Copy link
Member Author

3 of the 4 failing PR checks are due to missing Python version bumps, can you take a look @jonkeane ?

Yup, I can keep bumping versions if we're all good with doing that here. I'll also change the title to be more "bump python, and also the tiny archery fix too"

@jonkeane
Copy link
Member Author

jonkeane commented Feb 28, 2024

For the C++ / ARM64 Ubuntu 20.04 C++, ARM64 Debian 11 Go 1.19/1.20 jobs, IIUC those run on the self-hosted runners managed by Voltron Data, yeah?

I see:

https://github.com/apache/arrow/blob/d6b9051fa05b230ce288eb0b7ab6b596200c45b1/.github/workflows/cpp.yml#L137-L140

Which is pulling from apt (rather than using the setup-python action). Would it be preferred to switch to the setup-python action or add a ppa that we can fetch a newer python from?

The other C++ / AMD64 macOS 12 C++ is a timeout that I'm seeing on main on main too

I have not yet, but still will later go through the extended crossbow tests

@kou
Copy link
Member

kou commented Feb 28, 2024

ARM64 Ubuntu 20.04 C++

Let's use Ubuntu 22.04:

diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml
index e9409f1cd6..b5461e2f56 100644
--- a/.github/workflows/cpp.yml
+++ b/.github/workflows/cpp.yml
@@ -94,12 +94,12 @@ jobs:
             cat <<JSON >> "$GITHUB_OUTPUT"
           {
             "arch": "arm64v8",
-            "clang-tools": "10",
+            "clang-tools": "14",
             "image": "ubuntu-cpp",
-            "llvm": "10",
+            "llvm": "14",
             "runs-on": ["self-hosted", "arm", "linux"],
-            "title": "ARM64 Ubuntu 20.04 C++",
-            "ubuntu": "20.04"
+            "title": "ARM64 Ubuntu 22.04 C++",
+            "ubuntu": "22.04"
           }
           JSON
           fi

ARM64 Debian 11 Go 1.19/1.20

Let's use Debian 12:

diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index bbffab6704..5d6918aa60 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -90,7 +90,7 @@ jobs:
           echo "JSON" >> "$GITHUB_OUTPUT"
 
   docker:
-    name: ${{ matrix.arch-label }} Debian 11 Go ${{ matrix.go }}
+    name: ${{ matrix.arch-label }} Debian 12 Go ${{ matrix.go }}
     needs: docker-targets
     runs-on: ${{ matrix.runs-on }}
     if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
@@ -101,6 +101,7 @@ jobs:
         include: ${{ fromJson(needs.docker-targets.outputs.targets) }}
     env:
       ARCH: ${{ matrix.arch }}
+      DEBIAN: "12"
       GO: ${{ matrix.go }}
     steps:
       - name: Checkout Arrow
``

@jonkeane
Copy link
Member Author

Bumping to 22.04 still resulted in python 3.8 for C++ / ARM64 Ubuntu 22.04 C++: https://github.com/apache/arrow/actions/runs/8088510588/job/22102700260?pr=40269#step:5:55

But in looking at the extended builds, there are a number of old OSes that we won't have 3.8 by default (and some we even want 3.8 in order to build on). (e.g. wheel-macos-big-sur-cp38-arm64, test-fedora-39-python-3, test-ubuntu-20.04-python-3).

IMO, we should not bundle all of these changes up because of this one archery issue. We should:

  1. pin pygit2 to one version older (with a note to update it when 3.8 is fully gone),
  2. accept that archery[crossbow] requires 3.9, but others do not. Document that and then we can bump python to 3.12 for the places that call archery crossbow and leave many of these (especially the older ones which generally are using only archery[docker] on older python versions).

I think I would prefer the second of those, but am also happy to do the first.

@pitrou
Copy link
Member

pitrou commented Feb 29, 2024

I'm ok with option 2 as well.

@jonkeane
Copy link
Member Author

jonkeane commented Mar 1, 2024

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

Copy link

github-actions bot commented Mar 1, 2024

Revision: a7b63dd

Submitted crossbow builds: ursacomputing/crossbow @ actions-a2c3e955e0

Task Status
test-conda-python-3.10 GitHub Actions
test-conda-python-3.10-cython2 GitHub Actions
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions
test-conda-python-3.10-pandas-latest GitHub Actions
test-conda-python-3.10-pandas-nightly GitHub Actions
test-conda-python-3.10-spark-v3.5.0 GitHub Actions
test-conda-python-3.10-substrait GitHub Actions
test-conda-python-3.11 GitHub Actions
test-conda-python-3.11-dask-latest GitHub Actions
test-conda-python-3.11-dask-upstream_devel GitHub Actions
test-conda-python-3.11-hypothesis GitHub Actions
test-conda-python-3.11-pandas-upstream_devel GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.12 GitHub Actions
test-conda-python-3.8 GitHub Actions
test-conda-python-3.8-pandas-1.0 GitHub Actions
test-conda-python-3.8-spark-v3.5.0 GitHub Actions
test-conda-python-3.9 GitHub Actions
test-conda-python-3.9-pandas-latest GitHub Actions
test-cuda-python GitHub Actions
test-debian-11-python-3-amd64 Azure
test-debian-11-python-3-i386 GitHub Actions
test-fedora-39-python-3 Azure
test-ubuntu-20.04-python-3 Azure
test-ubuntu-22.04-python-3 GitHub Actions
wheel-macos-big-sur-cp310-arm64 GitHub Actions
wheel-macos-big-sur-cp311-arm64 GitHub Actions
wheel-macos-big-sur-cp312-arm64 GitHub Actions
wheel-macos-big-sur-cp38-arm64 GitHub Actions
wheel-macos-big-sur-cp39-arm64 GitHub Actions
wheel-macos-catalina-cp310-amd64 GitHub Actions
wheel-macos-catalina-cp311-amd64 GitHub Actions
wheel-macos-catalina-cp312-amd64 GitHub Actions
wheel-macos-catalina-cp38-amd64 GitHub Actions
wheel-macos-catalina-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-arm64 GitHub Actions
wheel-manylinux-2-28-cp311-amd64 GitHub Actions
wheel-manylinux-2-28-cp311-arm64 GitHub Actions
wheel-manylinux-2-28-cp312-amd64 GitHub Actions
wheel-manylinux-2-28-cp312-arm64 GitHub Actions
wheel-manylinux-2-28-cp38-amd64 GitHub Actions
wheel-manylinux-2-28-cp38-arm64 GitHub Actions
wheel-manylinux-2-28-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp39-arm64 GitHub Actions
wheel-manylinux-2014-cp310-amd64 GitHub Actions
wheel-manylinux-2014-cp310-arm64 GitHub Actions
wheel-manylinux-2014-cp311-amd64 GitHub Actions
wheel-manylinux-2014-cp311-arm64 GitHub Actions
wheel-manylinux-2014-cp312-amd64 GitHub Actions
wheel-manylinux-2014-cp312-arm64 GitHub Actions
wheel-manylinux-2014-cp38-amd64 GitHub Actions
wheel-manylinux-2014-cp38-arm64 GitHub Actions
wheel-manylinux-2014-cp39-amd64 GitHub Actions
wheel-manylinux-2014-cp39-arm64 GitHub Actions
wheel-windows-cp310-amd64 GitHub Actions
wheel-windows-cp311-amd64 GitHub Actions
wheel-windows-cp312-amd64 GitHub Actions
wheel-windows-cp38-amd64 GitHub Actions
wheel-windows-cp39-amd64 GitHub Actions

@jonkeane
Copy link
Member Author

jonkeane commented Mar 1, 2024

Ok, I took approach (2). I did slightly more than that e.g. .github/workflows/comment_bot.yml which isn't strictly necessary, but while I'm in there it seemed a good idea to pull it up.

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.

Current CI failures are unrelated

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 1, 2024
@jonkeane jonkeane merged commit 30e6d72 into apache:main Mar 1, 2024
9 checks passed
@jonkeane jonkeane removed the awaiting merge Awaiting merge label Mar 1, 2024
Copy link

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

There was 1 benchmark result indicating a performance regression:

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

thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Mar 8, 2024
…anges (apache#40269)

### Rationale for this change

`archery crossbow submit ...` fails with newer versions of pygit2

### What changes are included in this PR?

Adapt away from deprecated [sic] APIs in pygit2 to ones that work with current versions, bump the pin

### Are these changes tested?

Manually, yes, I can use `archery crossbow submit ...` again. CI will run using archery in a bunch of places on this PR too.

### Are there any user-facing changes?

No
* GitHub Issue: apache#40268

Authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
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.

4 participants