Skip to content
This repository was archived by the owner on Oct 4, 2022. It is now read-only.

[WIP] Go SDK: Query Builder#174

Closed
aluzzardi wants to merge 2 commits into
dagger:mainfrom
aluzzardi:query-builder-test
Closed

[WIP] Go SDK: Query Builder#174
aluzzardi wants to merge 2 commits into
dagger:mainfrom
aluzzardi:query-builder-test

Conversation

@aluzzardi
Copy link
Copy Markdown
Contributor

No description provided.

@netlify
Copy link
Copy Markdown

netlify Bot commented Sep 6, 2022

Deploy Preview for cloak-docs ready!

Name Link
🔨 Latest commit 187773f
🔍 Latest deploy log https://app.netlify.com/sites/cloak-docs/deploys/631b1822f15c4000085085e1
😎 Deploy Preview https://deploy-preview-174--cloak-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Copy Markdown
Contributor

@sipsma sipsma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, love the direction this is going. I think it has the potential to mesh really nicely with the builder pattern API proposed here. Just leaving high level comments for since the details seem to be WIP.

I guess this will bring back some of the questions we've managed to ignore for a while around lazy vs. synchronous behavior. For instance, if you have to pass an FSID as an arg somewhere, should calling .ID() result in synchronous evaluation? If not, then how can users force a synchronous evaluation when they want to trigger a side effect?

  • This is actually somewhat overlapping with our other discussions about query vs. mutation. Maybe mutations are synchronously evaluated by default and queries are lazy by default? I don't know, that is probably too magical, but might be something there.
  • Maybe the simplest solution would be for .ID() to synchronously evaluate (same behavior as today). And then we can have an advanced method like .LazyRef() that returns ID but without evaluating. Need a better name than LazyRef but you get the idea.
    • This could be implemented by actually having our low-level graphql API support both.

Another thing to think about: there was feedback recently that the need to return Filesystem objects but use FSID as input is kind of weird. It only makes sense if you know the underlying graphql model. I don't personally think it's a huge deal, but I do see how that is surprising to the uninitiated. I wonder if there's any way to hide the difference between "the object" and "the ID of the object" on this sort of query builder layer here.

  • Not a blocker at all, just curious if you have any thoughts on it.

Comment on lines +11 to +17
func alpine(packages ...string) *Filesystem {
fs := core.Image("alpine")
for _, pkg := range packages {
fs = fs.Exec("apk", "add", pkg).FS()
}
return fs
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩

@aluzzardi
Copy link
Copy Markdown
Contributor Author

I guess this will bring back some of the questions we've managed to ignore for a while around lazy vs. synchronous behavior. For instance, if you have to pass an FSID as an arg somewhere, should calling .ID() result in synchronous evaluation? If not, then how can users force a synchronous evaluation when they want to trigger a side effect?

Unsure about that. Spent most of the efforts in querybuilder so far, the core example is basically supposed to be code generated on top of querybuilder.

I was thinking of this:

  • Scalars always evaluate: there is no way not to evaluate them anyway I think? e.g. I don't know what a hypothetical LazyRef() would do. For non-scalars: there's no way to "stitch" them anyway (can't pass a scalar as a reference) so there's no point evaluating them (also you can't evaluate them in GraphQL -- at the end of the day we need to pass a scalar in the query or it will complain)

  • Go DX: Lazy can be chained. Not lazy takes a context.Context and returns an error. At least it's somehow obvious

  • Unknowns: if you do .ID(), .Stdout(), .Stderr() how to avoid firing up the same query N times? Maybe we don't care because of caching? Alternatively, it's the Exec() that's synchronous and "pre-queries" ID, Stdout, Stderr (in which case my previous 2 points don't make sense anymore)

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
@aluzzardi
Copy link
Copy Markdown
Contributor Author

Moved to dagger/dagger#3125

@aluzzardi aluzzardi closed this Sep 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants