-
Notifications
You must be signed in to change notification settings - Fork 334
Dynamic platform detection for ImageSpec to support cross-platform development #3315
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
…velopment Signed-off-by: Barry Wu <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #3315 +/- ##
==========================================
- Coverage 85.26% 79.42% -5.84%
==========================================
Files 386 215 -171
Lines 30276 22520 -7756
Branches 2969 2951 -18
==========================================
- Hits 25814 17886 -7928
- Misses 3615 3806 +191
+ Partials 847 828 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I recommend using platform: str = field(default_factory=_get_default_platform)the get default platform function can be like: def _get_default_platform():
import platform
machine = platform.machine().lower()
if machine in ("arm64", "aarch64"):
return "linux/arm64"
return "linux/amd64" |
…registry on ARM64 machine Signed-off-by: Barry Wu <[email protected]>
Signed-off-by: Barry Wu <[email protected]>
machichima
left a comment
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.
LGTM, thank you!
cc @pingsutw
pingsutw
left a comment
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.
LGTM, there is a lint error, could you take a look
Signed-off-by: Barry Wu <[email protected]>
machichima
left a comment
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.
LGTM
|
Congrats on merging your first pull request! 🎉 |
…velopment (flyteorg#3315) Signed-off-by: Barry Wu <[email protected]> Signed-off-by: Atharva <[email protected]>
Tracking issue
Closes flyteorg/flyte#6565
Why are the changes needed?
The ImageSpec should automatically detect the current machine's architecture and set an appropriate default platform:
"linux/arm64" for Apple Silicon Macs (ARM64)
"linux/amd64" for Intel Macs (x86_64)
What changes were proposed in this pull request?
Use platform.machine() to detect the current machine's architecture
How was this patch tested?
Unit test
Check all the applicable boxes
Summary by Bito
This pull request enhances the ImageSpec class with dynamic platform detection, automatically configuring the default platform based on the machine's architecture for both Apple Silicon and Intel Macs. This improvement boosts cross-platform usability for developers in varied environments. Additionally, the platform parameter has been made optional to facilitate this functionality.