Skip to content

Always respect value of evaluate field in the gateway api#2947

Closed
jedevc wants to merge 1 commit into
moby:masterfrom
jedevc:solve-evaluate-frontend
Closed

Always respect value of evaluate field in the gateway api#2947
jedevc wants to merge 1 commit into
moby:masterfrom
jedevc:solve-evaluate-frontend

Conversation

@jedevc
Copy link
Copy Markdown
Member

@jedevc jedevc commented Jul 5, 2022

Always use the value of the evaluate field to force result generation, which previously was not performed for frontends. This improves API consistency, and ensures the value is used regardless of whether the solver uses a frontend, or a raw definition.

Always use the value of the evaluate field to force result generation,
which previously was not performed for frontends. This improves API
consistency, and ensures the value is used regardless of whether
the solver uses a frontend, or a raw definition.

Signed-off-by: Justin Chadwell <me@jedevc.com>
@tonistiigi
Copy link
Copy Markdown
Member

What's the use case for this? Usually, a frontend itself should define if solve is lazy or not.

@jedevc
Copy link
Copy Markdown
Member Author

jedevc commented Jul 18, 2022

A couple reasons:

  • Something like this would be required to implement Proposal: add new --sync-output flag to bake docker/buildx#1197 in the way it's currently done, though that might easily change.
  • API consistency - a user requesting an eager solve by setting the Evaluate field to true will expect that the solve should not be lazy. I agree that a frontend should determine if a solve is lazy or not in the case that the Evaluate option isn't force set, but when a developer specifies it, I think that that the developer's choice should override the default. At the moment, without this patch, I don't think there's a way to force an eager solve for a generic frontend?

@tonistiigi
Copy link
Copy Markdown
Member

You could just call like a Statfile("/") https://pkg.go.dev/github.com/moby/buildkit@v0.10.3/frontend/gateway/client#Reference for that buildx issue.

API consistency

This API call already has two modes depending if Definition or Frontend is set. Eg. FrontendAttrs etc.

I have a slight worry that this could be abused somehow by a client who is just supposed to forward the request and doesn't know about the LLB doing a premature unlazy and messing up the performance.

@jedevc
Copy link
Copy Markdown
Member Author

jedevc commented Aug 4, 2022

You could just call like a Statfile("/")

That definitely works, but it would need to be called per-ref, so a result that has multiple platforms, would need a trip back-and-forth per StatRequest, which feels frustrating.

I wonder if we could potentially rename the field from Evaluate to make it's purpose more clear, for example, to EagerEvaluate, or even just have a field for Lazy as a boolean with default true, though that would require a backwards-incompatible change, which could reduce the risk of a client accidentally doing a premature unlazy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants