Improve buildkit node creation#10843
Conversation
…oad them for every service build. Signed-off-by: Silvin Lubecki <[email protected]>
961d232 to
092cd69
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## v2 #10843 +/- ##
==========================================
+ Coverage 58.30% 59.94% +1.63%
==========================================
Files 119 118 -1
Lines 10331 10076 -255
==========================================
+ Hits 6024 6040 +16
+ Misses 3710 3441 -269
+ Partials 597 595 -2
☔ View full report in Codecov by Sentry. |
milas
left a comment
milas
left a comment
There was a problem hiding this comment.
LGTM
Note to Compose team: we should make sure to mention that BUILDX_BUILDER is now supported for up --build as well in the release notes (see also my non-blocking review comment)
| builderName := options.Builder | ||
| if builderName == "" { | ||
| builderName = os.Getenv("BUILDX_BUILDER") | ||
| } |
There was a problem hiding this comment.
Hmm, so we already do a look up in cmd/build.go:
Lines 58 to 61 in 8318f66
But that won't apply for up --build (while this will).
@glours Do you think that --builder should become a global flag on Compose, and let the CLI/cmd part handle the env var lookup?
[I don't think we need to resolve this as part of this PR]
3 participants
What I did
Move builder and nodes initialization code up, avoiding to recreate/load them for every service build.
Added support of
BUILDX_BUILDERenv var.Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did
