-
Notifications
You must be signed in to change notification settings - Fork 7k
[runtime env] properly support apple silicon wheel urls #57745
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the wheel URL generation to better support M1 Macs by centralizing the OS/architecture string logic in get_wheel_filename. The previous complex and partially incorrect logic is replaced with a clearer and more robust implementation. The changes in conda.py correctly remove special-cased M1 logic, delegating the responsibility to the updated utility function. The test files are also updated to reflect these changes. Overall, this is a good improvement. I have one minor suggestion to improve the clarity of a comment in the test file.
| # Windows only has x86_64 wheels | ||
| if sys_platform == "win32" and arch != "x86_64": | ||
| continue | ||
| # MacOS only has arm64 wheels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on this line is a bit misleading. While it might be true that for these specific dev wheels only arm64 builds are produced for macOS, it's not true in general as macOS has x86_64 wheels. This could cause confusion for future maintainers. I'd suggest clarifying the comment to be more specific to dev wheels.
For example:
- # MacOS only has arm64 wheels
+ # For nightly dev wheels, we currently only build for arm64 on macOS.ac60876 to
bc89889
Compare
and update commits used in tests also removes special tests for "m1 wheels" Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
bc89889 to
43f23ea
Compare
…57745) and update commits used in tests also stops testing x86_64 osx wheels on "latest", those are deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
…57745) and update commits used in tests also stops testing x86_64 osx wheels on "latest", those are deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: xgui <xgui@anyscale.com>
and update commits used in tests also stops testing x86_64 osx wheels on "latest", those are deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
…57745) and update commits used in tests also stops testing x86_64 osx wheels on "latest", those are deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com>
…57745) and update commits used in tests also stops testing x86_64 osx wheels on "latest", those are deprecated. Signed-off-by: Lonnie Liu <lonnie@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
and update commits used in tests
also stops testing x86_64 osx wheels on "latest", those are deprecated.