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
feat: gate target-cpu=native
with portable
feature
#4489
Conversation
We believe this can cause some problems with docker images.
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.
Thanks for submitting this Brice!
Related PR: #4460
The PR linked above explicitly defines the CPU target when an image is being built via GHA. But when this Docker image is being built locally (which I've had to do numerous times) intended for cloud deployments, it'll be helpful to have this feature flag in place to ensure the image is being built the same way it's always been, which has been compatible with most cloud infra.
I would suggest getting @wileyj's approval in this before merging as well.
So maybe it would be good to have that "portable" feature enabled conditionally, with an environment variable for example? |
@obycode I actually like the way you have it right now. In the scenario I painted above when I need to build the image locally, I use the |
Ah, got it. Thanks for clarifying! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #4489 +/- ##
===========================================
- Coverage 83.17% 55.28% -27.90%
===========================================
Files 451 451
Lines 324497 324497
Branches 318 318
===========================================
- Hits 269916 179409 -90507
- Misses 54573 145080 +90507
Partials 8 8 see 327 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
We believe this flag can cause some problems with docker images.