Dynamic platform detection for ImageSpec to support cross-platform development - #3315
Merged
Conversation
…velopment Signed-off-by: Barry Wu <a0987818905@gmail.com>
BarryWu0812
requested review from
cosmicBboy,
davidmirror-ops,
eapolinario,
kumare3,
pingsutw,
samhita-alla and
wild-endeavor
as code owners
August 15, 2025 05:28
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:
|
machichima
reviewed
Aug 15, 2025
Member
|
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 <a0987818905@gmail.com>
machichima
reviewed
Aug 18, 2025
Signed-off-by: Barry Wu <a0987818905@gmail.com>
machichima
previously approved these changes
Aug 18, 2025
machichima
left a comment
Member
There was a problem hiding this comment.
LGTM, thank you!
cc @pingsutw
pingsutw
reviewed
Aug 19, 2025
pingsutw
left a comment
Member
There was a problem hiding this comment.
LGTM, there is a lint error, could you take a look
pingsutw
approved these changes
Aug 27, 2025
|
Congrats on merging your first pull request! 🎉 |
Atharva1723
pushed a commit
to Atharva1723/flytekit
that referenced
this pull request
Oct 5, 2025
…velopment (flyteorg#3315) Signed-off-by: Barry Wu <a0987818905@gmail.com> Signed-off-by: Atharva <atharvakulkarni172003@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.