Implement required artifact resolution in buildpacks builder #4962
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4962 +/- ##
==========================================
- Coverage 72.34% 72.29% -0.06%
==========================================
Files 360 361 +1
Lines 12564 12665 +101
==========================================
+ Hits 9090 9156 +66
- Misses 2808 2829 +21
- Partials 666 680 +14
Continue to review full report at Codecov.
|
| if builderImage == d.Alias { | ||
| builderImage, found = r.GetImageTag(d.ImageName) | ||
| if !found { | ||
| logrus.Fatalf("failed to resolve build result for required artifact %q", d.ImageName) | ||
| } | ||
| builderImageLocal = true | ||
| } | ||
| if runImage == d.Alias { | ||
| runImage, found = r.GetImageTag(d.ImageName) | ||
| if !found { | ||
| logrus.Fatalf("failed to resolve build result for required artifact %q", d.ImageName) | ||
| } | ||
| runImageLocal = true | ||
| } |
There was a problem hiding this comment.
can these two code paths be combined into one? And then do the check at the end to either set builderImageLocal = true or runImageLocal = true
There was a problem hiding this comment.
not quite sure how though, can you suggest the code change you're thinking?
There was a problem hiding this comment.
On second look, maybe they can't, my bad 😅
Do you think the log messages should be differentiated to say either builder image or run image? Or do you think the user will have enough info from the ImageName?
There was a problem hiding this comment.
this is actually an impossible scenario. It'll only happen if we ever introduce a bug into the code.
Related: #4713, #4922
Description (Part 4 of several PRs.)
This PR implements resolving required artifacts as either the builder image or the run image in buildpacks builder as described in the design for supporting dependencies between build artifacts.