-
Notifications
You must be signed in to change notification settings - Fork 485
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
build: fix build details link in experimental mode #2722
Conversation
052caaa
to
a51e219
Compare
controller/errdefs/errdefs.proto
Outdated
@@ -6,4 +6,5 @@ option go_package = "github.com/docker/buildx/controller/errdefs"; | |||
|
|||
message Build { | |||
string Ref = 1; | |||
string BuildRef = 2; |
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.
what's the difference?
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.
seems to handle the attached session when monitoring:
Line 466 in d353f5f
ref = be.Ref |
will be "local" for local builds:
buildx/controller/local/controller.go
Line 24 in d353f5f
ref: "local", |
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.
Can we rename this "kind" or "type" then?
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.
Looking a bit more at it, for local builds with controller it always set local
but for remote it sets a random id that lives across the controller session:
buildx/controller/remote/client.go
Line 120 in 746eadd
ref := identity.NewID() |
So ref
is still good but confusing as this is not a build ref. It's even more confusing as we already have BuildOptions.Ref
in
buildx/controller/pb/controller.proto
Line 80 in 746eadd
string Ref = 29; |
I renamed it to sessionID
.
a51e219
to
52cddf9
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
52cddf9
to
eb15c66
Compare
fixes #2382
Temp workaround while we are working on cleaning up the controller logic.