#Generic ID

1 messages · Page 1 of 1 (latest)

dusk anvil
#

@mental reef says:

i found myself wanting this lol 🙂

mental reef
#

🎉

dusk anvil
mental reef
#

okay, so i've been working on interactive debugging (demo asciicast incoming soon)
essentially, hover over a span in the tui, hit T, and open a terminal on that thing

#

but i want this to work with everything that has a .terminal method

dusk anvil
#

Interface maybe?

mental reef
#

atm, i'm just hardcoding support for Directory/File/Container/Service - it would be really cool to support it generically with generic ids

dusk anvil
#

We talked about converting our core types to interfaces before

mental reef
#

we did indeed 😄

#

we will

dusk anvil
#

The reason I need it is to implement Env.withBinding and accept any object ID

mental reef
#

(one day, when i get angry enough with the git api, i'll do it)

mental reef
#

okay, um, i would need a moment to page that in

dusk anvil
#

What's funny is that the feature already works... Just with a string as ID argument, so clients won't generate useful bindings to it

mental reef
#

i think that i would probably require that users do some manual casting from a generic ID. so doing a ContainerID(myGenericID)

#

like, being explicit about it feels like a good thing from the SDK POV

dusk anvil
#

But that''s for getting the object right? My issue is with setting

#

Getting is already solved, because the Binding type already has converters

mental reef
#

ohhh, you mean for the other way round?

dusk anvil
#

yes, I want to replace Env.withXXXBinding with just Env.withBinding

#

well, I already did (in that PR). Just need the codegen for it to work cleanly

mental reef
#

cool, but withBinding(ID(myContainerID)) is too terrible

dusk anvil
mental reef
#

uhhh

#

yeah, sdk work needed for everyone i think

#

i do not think it can be done automagically

dusk anvil
#

We could make it take an interface that all objects satisfy (e.g. in Go every object implements XXX_GraphQLID and XXX_GraphQLType
[...]
In Go it would probably look like this:

type Object interface {
    XXX_GraphQLType() string
    XXX_GraphQLID(context.Context) (string, error)
}

The downside to this approach is that it's another box for every SDK maintainer to check. They'll have to special-case an ID argument to take an Object interface (analogous to how ContainerID actually takes a Container), or come up with some other language-specific tactic. Or just leave users to do the boilerplate themselves, but that's no fun.

mental reef
#

yeahhh. that works, but then, you're needing to automatically fill that Object in to everything that takes/returns a generic ID
but because Object is specific per-sdk, you need to do this conversion per sdk

#

i like the approach though

#

it's just gonna be work

dusk anvil
mental reef
#

unless we could make this work with interfaces 🤔

#

i wonder if we have all the plumbing

dusk anvil
#

Well this would be an interface right?

mental reef
#

yeaaaaa. it's the empty interface.

#

but we already have the plumbing to make those conversions work automatically i think

dusk anvil
#

@tame oxide gut check. You're an SDK maintainer. We ask you to add support for a generic Object interface that matches all Dagger objects. Is it a PITA?

mental reef
#

you need to do .AsMyModuleFooer

#

so you'd essentially need to do dag.Container().Whatever().AsObject() (if Object was the generic interface type)

#

if that's a reasonable compromise, we've already got the plumbing there! just need to add core interfaces

#

(which has been on the backlog forever)

dusk anvil
#

But if all SDKs added a special case for generic Object, then the DX would be simpler for users right?

mental reef
#

it probably would be. i wonder if it would be possible to do it on top of interfaces

#

it would be good to avoid two different interface mechanisms

dusk anvil
#

Well, I'm assuming end-users would only ever see one mechanism

dusk anvil
#

@mental reef what's funny is that my current implementation actually works in dagger shell 🙂

tame oxide
dusk anvil
#

I guess that doesn't make sense... 🤔 since all the engine needs is a scalar ID

--> Basically I don't know the answer because I don't understand in enough detail how it would work

dusk anvil
#

Generic ID

dusk anvil
#

@digital silo I would like to take a stab at implementing the generic ID in the way that you suggested. Could you give me a few pointers in how to start?

digital silo
dusk anvil
#

I already have that. It actually works fine in shell

#

Oh you mentioned my ID type is not wired somewhere though

#

Just to make sure I understand: say my module exposes a function foo(source: ID!): ID. What would the generated binding look like in each SDK?

  • Go: func (c Client) Foo(source Object) Object with type Object interface { ID() } ?
#

should I be able to do this?

ctr := dag.Container()
newCtr := dag.Foo(ctr)
#

Or would I need a bunch of explicit conversions, like:

ctr := dag.Container()
newCtr := dag.Foo(ctr.AsObject()).AsContainer()
digital silo
#

since that's what all objects already implement - we even have that interface defined somewhere, i think, but it has a different name

digital silo
#

this gets a bit tricky because it seems like we would want a non-scalar representation of 'any object' too

#

seems like we'd want to call that Object too - but then is it a GraphQL interface? or is it a Dagger-style interface? and how does it relate to our type Object interface { ... } implementation?

dusk anvil
digital silo
#

no, those are withExec: Container!

#

that's how you're able to nest/chain

dusk anvil
#

Oh right. Brainfart sorry

digital silo
#

the tragic thing is we're so close to being able to do the 'official' GraphQL thing:

interface Node {
  id: ID!
}

except all of our objects have an id that returns their own ID type

maybe we just need to add an ID!-returning version?

interface Object {
  objectID: ID!
}

and then automatically install that accessor on every Object type in the schema?

(oh, but also, GraphQL also requires explicit implements Foo when defining types... i guess DagQL could do that automatically, like we already do for Dagger interfaces)

dusk anvil
#

OK, so for my use case I only need to pass generic objects as arguments, I don't need to receive them back

digital silo
dusk anvil
digital silo
#

ah ok

dusk anvil
#

which has asFoo() converters in the Dagger API

#

Although you just opened a window in my mind towards maybe, in the future, moving the "generate a converter for every type in the schema" heavy lifting out of the Binding type, and into the SDKs and their magic Object/ID type??

digital silo
#

yeah

dusk anvil
#

which would make the engine schema static again (no more middleware)

#

MASA - Make the API Static Again 😛 (OK no more political jokes)

digital silo
#

like a type Object { id: ID!, typeName: String!, asX: X, ... }?

#

avoiding graphql interfaces there would at least be consistent with our current interface mechanics

#

which are more Go-style (implicit implements)

#

oh, you're saying outside of the schema entirely

dusk anvil
dusk anvil
#

anyway that's definitely not needed to unblock me right now 🙂

digital silo
#

you don't really need it at the schema layer anyway because you can always just pass the ID into load<DesiredType>ByID

dusk anvil
#

So now I'm stuck on the very very basics: which files do I edit to prototype this 🙂

#

I tried digging through go/sdk looking for the generated bindings, and the code that generates them, but it's a lot

digital silo
#

indeed

dusk anvil
#

breaking it down with explicit conversions

var (
  ctrIn, ctrOut *dagger.Container
  objectIn, objectOut dagger.Object
)

ctrIn = dag.Container() // concrete -> concrete 👍
objectIn = ctrIn // concrete -> interface  👍
objectOut = dag.Foo(objectIn) // interface -> interface 👍
ctrOut = objectOut // interface -> concrete 🤔🤔🤔🤔
digital silo
#

there could be a NewObject(id ID) (Object, error) that does object(id: ID!) { typeName } and uses that to determine what struct to create

#

tangent, we could get rid of the loadFooByID APIs and do object(id: ID!) { asContainer { ... } } - at the expense of re-adding schema dynamic-ness. (arguably just as dynamic as loadFooByIDs tbh)

dusk anvil
#

Oh wait I myself said I don't need to get objects back out... 🙈

digital silo
#

yeah that too 😛

#

but worth a thought

dusk anvil
#

So for my narrower scope:

ctr := dag.Container()
env = env.WithBinding("container", ctr)
#

What about Python and Typescript? They don't have interfaces

digital silo
#

i think they do? but yeah this is going to be a very language-specific thing. (typescript at least has interfaces)

#

that Go suggestion was specifically because i know we already have those two methods that all objects implement

dusk anvil
digital silo
#

for Python you can maybe just get away with accepting Type since it's the superclass of every object type, and has the _ctx (I think that's where the query gets chained as you call methods)

dusk anvil
#

OK this is definitely going to be a learning curve for me... But good! Forcing function to ramp up on this part of the code

#

{{- $convertID := $field | ConvertID }}
[...]

{{- range $arg := $field.Args }}
    {{- if not (IsArgOptional $arg) }}
    q = q.Arg("{{ $arg.Name }}", {{ $arg.Name }})
    {{- end }}
    {{- end }}
    {{- $typeName := $field.TypeRef | FormatOutputType }}
    {{ if and $supportsVoid $field.TypeRef.IsVoid }}
        return q.Execute(ctx)
    {{- else if $convertID }}
    var id {{ $typeName }}
    if err := q.Bind(&id).Execute(ctx); err != nil {
        return nil, err
    }
    return &{{ $field.ParentObject.Name }} {
        query: q.Root().Select("load{{ $field.ParentObject.Name }}FromID").Arg("id", id),
    }, nil

    {{- else if $field.TypeRef.IsObject }}
    return &{{ $typeName }} {
        query: q,
        {{- if eq $typeName "Client" }}
        client: r.client,
        {{ end }}
    }

🤔
🤔
🤔
🤔

#

I'm looking for the part that says "if the argument is of type <FOO>ID then your binding argument should be <FOO>"

digital silo
#

yeaaaahhh this code is not my favorite, though to be fair it's hard to imagine an ideal form of it, all things considered. we use a mix of Go text/template, and also something called jennifer, which is for programmatic codegen instead of templates. they both have trade-offs

#

what you're looking for might be in a template func instead, if you don't see it directly in the template

#

like FormatOutputType for example

dusk anvil
#

My understanding is that I need to:

  1. Add the definition of the Object interface that you gave me
  2. Insert logic so that ID arguments are correctly exposed as Object arguments in the binding
  3. Maybe some plumbing to make the above work???
dusk anvil
#

just blindly poking around for now

digital silo
#

client.go.tmpl might be a good spot for the type Object definition

dusk anvil
#

does _dagger.gen.go/*.go.tmpl get smooshed together in a single output file?

#

in alphabetical order I guess?

digital silo
#

they're conditionally applied by dagger.gen.go.tmpl

#

ok actually client.go.tmpl might not be right. hmm maybe type Object just goes in dagger.gen.go.tmpl

dusk anvil
# digital silo like `FormatOutputType` for example

Now I understand what you meant 🙂 Led me to this:

// formatType loops through the type reference to transform it into its SDK language.
func (c *CommonFunctions) formatType(r *introspection.TypeRef, scope string, input bool) (representation string, err error) {
    ff := c.formatTypeFuncs.WithScope(scope)

    for ref := r; ref != nil; ref = ref.OfType {
        switch ref.Kind {
        case introspection.TypeKindList:
            // Handle this special case with defer to format array at the end of
            // the loop.
            // Since an SDK needs to insert it at the end, other at the beginning.
            defer func() {
                representation = ff.FormatKindList(representation)
            }()
        case introspection.TypeKindScalar:
            switch introspection.Scalar(ref.Name) {
            case introspection.ScalarString:
                return ff.FormatKindScalarString(representation), nil
            case introspection.ScalarInt:
                return ff.FormatKindScalarInt(representation), nil
            case introspection.ScalarFloat:
                return ff.FormatKindScalarFloat(representation), nil
            case introspection.ScalarBoolean:
                return ff.FormatKindScalarBoolean(representation), nil
            default:
                return ff.FormatKindScalarDefault(representation, ref.Name, input), nil
            }
        case introspection.TypeKindObject:
            return ff.FormatKindObject(representation, ref.Name, input), nil
        case introspection.TypeKindInputObject:
            return ff.FormatKindInputObject(representation, ref.Name, input), nil
        case introspection.TypeKindEnum:
            return ff.FormatKindEnum(representation, ref.Name), nil
        }
    }

    return "", fmt.Errorf("unexpected type kind %s", r.Kind)
}
#

would an ID type eg. ContainerID match introspection.TypeKindScalar or introspection.TypeKindInputObject?

digital silo
#

Scalar

dusk anvil
#

So ->

default:
                return ff.FormatKindScalarDefault(representation, ref.Name, input), nil
#

Aha!

func (f *FormatTypeFunc) FormatKindScalarDefault(representation string, refName string, input bool) string {
    if obj, ok := strings.CutSuffix(refName, "ID"); input && ok {
        representation += "*" + f.scope + formatName(obj)
    } else {
        representation += f.scope + formatName(refName)
    }

    return representation
}
#

OK let's see if this works!

#

@digital silo in the PR you said:

I didn't notice it was an alias defined only in core/ and not exposed as ID through the schema yet, which explains why it's just a string here. Not sure if that's intentional or just missing a dagql tweak to expose it

What's the missing tweak?

#

nevermind I see it - there's a dagql.ID[]

#

Oh but wait - dagql.ID[<what goes here>]

digital silo
#

yeah those are the typed ones, yours is different

dusk anvil
#

I did this:


type ID dagql.String

func (id ID) Load(ctx context.Context, srv *dagql.Server) (dagql.AnyResult, error) {
    var callID = new(call.ID)
    err := callID.Decode(dagql.String(id).String())
    if err != nil {
        return nil, err
    }
    return srv.Load(ctx, callID)
}

func (ID) Type() *ast.Type {
    return &ast.Type{
        NamedType: "Object", // FIXME: ID?
        NonNull:   true,
    }
}
digital silo
#

you'll want NamedType: "ID", and then srv.InstallScalar(ID("")) in *querySchema.Install

dusk anvil
#

That NamedType: "Object" is me blindly trying stuff...

dusk anvil
#

@digital silo currently implementing ID.DecodeInput to fit the dagql.Scalar interface

#

mostly I'm copy-pasting from ID[x] and trying to get it to build 🙂

#

OK this is harder than I expected... (making my ID implement dagql.Scalar)

#

Maybe I'm overthinking it... type ID dagql.Scalar and done?

dusk anvil
dusk anvil
# 
Error: generate code: template: module.go.tmpl:86:3: executing "_dagger.gen.go/module.go.tmpl" at <ModuleMainSrc>: error calling ModuleMainSrc: missing method MarshalJSON from DaggerObject interface, which must be embedded in interfaces used in Functions and Objects

😭

dusk anvil
#

it worked!!