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-45156: [Python][Packaging] Refactor Python Windows wheel images to use newer base image #45442

Merged

Conversation

amoeba
Copy link
Member

@amoeba amoeba commented Feb 6, 2025

Rationale for this change

See #45156. Refactors our previous Windows Docker CI images from an unofficial image to the official Microsoft one based on Windows Server 2022 and adds VS2022 BuildTools.

What changes are included in this PR?

  • New Windows base Dockerfiles
  • Updated Dockerfiles which build on top of that base
  • Updated Docker Compose services to use new images

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the awaiting review Awaiting review label Feb 6, 2025
@amoeba
Copy link
Member Author

amoeba commented Feb 7, 2025

This is failing when the image is built with a thrift checksum mismatch from vcpkg but otherwise what's here in the PR is working.

@amoeba
Copy link
Member Author

amoeba commented Feb 7, 2025

I let a build of the base image run overnight and it ran out of disk space on my small hard disk but it looks like it built. So next steps here are:

  • Find a way to get the base image to build
  • Hook it into updated final Dockerfiles and start testing

@amoeba amoeba force-pushed the feature/GH-45156--python-windows-wheel-docker-image branch from 9c311c3 to 0c18bee Compare February 8, 2025 02:14
@amoeba amoeba force-pushed the feature/GH-45156--python-windows-wheel-docker-image branch from 0c18bee to ed31241 Compare February 8, 2025 02:15
@amoeba
Copy link
Member Author

amoeba commented Feb 8, 2025

Good progress today. I was able to build the base image with a combination of increasing the memory limit to 4GB and tweaking the storage limit. I propagated the changes to the other dockerfiles and elsewhere in the repo.

Now this just needs testing.

@amoeba
Copy link
Member Author

amoeba commented Feb 23, 2025

Working on this today. I have the vs2022 base image building cleanly now so this runs:

archery docker build python-wheel-windows-vs2022

(other related builds also run since they use the same base now).

@amoeba
Copy link
Member Author

amoeba commented Feb 23, 2025

@github-actions crossbow submit wheel-windows-*

Copy link

Revision: ffa8d79

Submitted crossbow builds: ursacomputing/crossbow @ actions-ad4009d4a3

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@amoeba
Copy link
Member Author

amoeba commented Feb 23, 2025

Testing this on crossbow now.

@amoeba
Copy link
Member Author

amoeba commented Feb 23, 2025

@github-actions crossbow submit wheel-windows-*

Copy link

Revision: bb517c4

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

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@amoeba
Copy link
Member Author

amoeba commented Feb 23, 2025

@github-actions crossbow submit wheel-windows-*

Copy link

Revision: b420014

Submitted crossbow builds: ursacomputing/crossbow @ actions-1b8ee8dbc2

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@amoeba amoeba marked this pull request as ready for review February 23, 2025 20:24
@amoeba amoeba requested a review from pitrou February 23, 2025 20:25
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

RUN curl https://dl.min.io/server/minio/release/windows-amd64/archive/minio.RELEASE.2024-09-13T20-26-02Z `
--output "C:\Windows\Minio.exe"

# Install archiver to extract xz archives (for timezone database).
Copy link
Member

Choose a reason for hiding this comment

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

We use a package in PyPI for timezone database now.
Can we remove using archiver?

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Feb 24, 2025
@pitrou
Copy link
Member

pitrou commented Feb 24, 2025

This is excellent @amoeba !

Out of curiosity, what is the size of the new Docker images?

@amoeba
Copy link
Member Author

amoeba commented Feb 24, 2025

@pitrou The new images are a bit smaller which is nice. For example,

  • Before, python-3.9-wheel-windows-vs2019-vcpkg-f7423ee180c4b7f40d43402c2feb3859161ef625-2025-02-03 was 26.7GB
  • After, python-wheel-windows-vs2022-base-vcpkg-f7423ee180c4b7f40d43402c2feb3859161ef625-ltsc2022-KB5049983 is 21.7GB

@amoeba
Copy link
Member Author

amoeba commented Feb 24, 2025

I built all the images and it looks like we generally shave off about 5GiB off each,

Docker Image Before (GB) After (GB) Change (GB)
python-x-free-threaded-wheel-windows-test 17.6 12.3 -5.3
python-x-wheel-windows-test 17.6 12.2 -5.4
python-wheel-windows-test 16.4 11.3 -5.1
python-x-wheel-windows-x-vcpkg 26.7 21.7 -5.0
python-wheel-windows-x-base-vcpkg 25.5 20.7 -4.8
Total 103.8 78.2 -25.6

@amoeba
Copy link
Member Author

amoeba commented Feb 24, 2025

@github-actions crossbow submit wheel-windows-*

@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 24, 2025
Copy link

Revision: 9651440

Submitted crossbow builds: ursacomputing/crossbow @ actions-ef7fa8e459

Task Status
wheel-windows-cp310-cp310-amd64 GitHub Actions
wheel-windows-cp311-cp311-amd64 GitHub Actions
wheel-windows-cp312-cp312-amd64 GitHub Actions
wheel-windows-cp313-cp313-amd64 GitHub Actions
wheel-windows-cp313-cp313t-amd64 GitHub Actions
wheel-windows-cp39-cp39-amd64 GitHub Actions

@kou
Copy link
Member

kou commented Feb 25, 2025

It worked.

Can we merge this?

@amoeba
Copy link
Member Author

amoeba commented Feb 25, 2025

Yes, I'll merge it now.

@amoeba amoeba merged commit a94f860 into apache:main Feb 25, 2025
34 checks passed
@amoeba amoeba removed the awaiting change review Awaiting change review label Feb 25, 2025
kou added a commit to kou/arrow that referenced this pull request Feb 25, 2025
…ges to use newer base image (apache#45442)

### Rationale for this change

See apache#45156. Refactors our previous Windows Docker CI images from an unofficial image to the official Microsoft one based on Windows Server 2022 and adds VS2022 BuildTools.

### What changes are included in this PR?

- New Windows base Dockerfiles
- Updated Dockerfiles which build on top of that base
- Updated Docker Compose services to use new images

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.

* GitHub Issue: apache#45156

Lead-authored-by: Bryce Mecum <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Bryce Mecum <[email protected]>
@pitrou
Copy link
Member

pitrou commented Feb 25, 2025

Thanks a lot @amoeba for taking this up, this is really great!

Copy link

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

There were no benchmark performance regressions. 🎉

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

arashandishgar pushed a commit to arashandishgar/arrow that referenced this pull request Feb 25, 2025
…ges to use newer base image (apache#45442)

### Rationale for this change

See apache#45156. Refactors our previous Windows Docker CI images from an unofficial image to the official Microsoft one based on Windows Server 2022 and adds VS2022 BuildTools.

### What changes are included in this PR?

- New Windows base Dockerfiles
- Updated Dockerfiles which build on top of that base
- Updated Docker Compose services to use new images

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.

* GitHub Issue: apache#45156

Lead-authored-by: Bryce Mecum <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Bryce Mecum <[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.

3 participants