history: fix empty Exporters attribute #5017
Conversation
|
Ahhh nice catch! Looks like I got the results correctly added to the history record, but for some reason didn't think to get the right exporters attr. Thanks! 🙏 🎉 |
crazy-max
force-pushed
the
fix-history-exporters
branch
from
f1b94e4
to
fa2a8b9
Compare
crazy-max
force-pushed
the
fix-history-exporters
branch
from
fa2a8b9
to
f11d945
Compare
exporter/containerimage/export.go
Outdated
| @@ -187,6 +190,14 @@ func (e *imageExporterInstance) ID() int { | |||
| } | |||
|
|
|||
| func (e *imageExporterInstance) Name() string { | |||
There was a problem hiding this comment.
Maybe Type() would be better?
There was a problem hiding this comment.
Potentially - note this also mirrors the structures of the cache exporter here.
Type and Name SGTM - then we keep the old field.
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
crazy-max
force-pushed
the
fix-history-exporters
branch
from
f11d945
to
6b6fa9d
Compare
| @@ -22,6 +22,8 @@ type ExporterInstance interface { | |||
| ID() int | |||
| Name() string | |||
| Config() *Config | |||
| Type() string | |||
| Attrs() map[string]string | |||
There was a problem hiding this comment.
(possible follow-up) Maybe the Attrs and Config() can be combined somehow in the future. Atm not obvious why there are two similar sounding things.
There was a problem hiding this comment.
I think maybe a better approach is to change the solver.ExporterRequest to keep array of objects that keep type/attrs instead of raw ExporterInstance. Like it was previously with the keys that are now removed.
There was a problem hiding this comment.
Let's get this in as seems functionally correct. #5017 (comment) to be addressed in follow-up
Since multiple exporters support #4134 in BuildKit 0.13,
Record.Exportersis not set anymore.This is related to the
TypeandAttrsfields not being set in thellbsolver.ExporterRequest: https://github.com/moby/buildkit/pull/4134/files#diff-0397d42d692ad676389ccf4b5a1126a3976ef69d4342b8423a6bbb51d21a9181L458-L459But this is used when we want to set the
Exportersfor the build history:buildkit/solver/llbsolver/solver.go
Line 176 in eed17a4
To fix this, I have updated the
exporter.ExporterInstanceinterface to expose attributes and name of the exporter. Also removesTypeandAttrsfieldsExporterRequestthat don't seem used anymore.