#FunctionArg

1 messages · Page 1 of 1 (latest)

shrewd dagger
#

Oh, so what about the TypeDef.Kind() problem?

eternal jackal
#

it's definitely related

shrewd dagger
#

You're saying that you need to make it idable as well?

dusk scaffold
#

TypeDef is IDable right? I think that's the problem, when you get a list of IDable objects and then try to use them, you get a panic

#

IIRC

eternal jackal
#

basically the TypeDef field isn't marked as one that can be pre-fetched (wherever that happens) - so when you call it, it's trying to run a query, but that object doesn't even have the GraphQL client on it

shrewd dagger
#

I did funcArg.ID() and got no error from Vim, only on :e 😅

eternal jackal
#

it'll be blowing up on the bottom line at the moment

image.png
#

i think the general fix could be that for all ID-able objects, we pre-populate a r.q that is basically { functionArg(id: "...") { ... } }

#

something like that anyway

eternal jackal
#

Proposing a new convention for how ID-able objects are constructed, especially types like FunctionArg that have required fields:

scalar FunctionArgID

type FunctionArg {
  id: FunctionArgID!
  name: String!
  typeDef: TypeDef!
}

# goal: ability to construct a FunctionArg from ID OR optional/required args,
#       such that a FunctionArg is never "partially instantiated"
# solution: two constructors, one by required ID, another with required args (if any)

type Query {
  getFunctionArg(id: FunctionArgID!): FunctionArg!
  functionArg(name: String!, typeDef: TypeDefID!): FunctionArg!
}

### rationale below

type Query {
  functionArg(id: FunctionArgID!): FunctionArg!
  functionArgWith(name: String!, typeDef: TypeDefID!): FunctionArg!
}

# problem: what if it has no required args? functionArgWith() is weird
# solution: newFunctionArg

type Query {
  functionArg(id: FunctionArgID!): FunctionArg!
  newFunctionArg(name: String!, typeDef: TypeDefID!): FunctionArg!
}

# problem: "new" feels weird. we want it to feel like a query, not like state.
# plus if you call the same query multiple times you get objects with the same ID,
# which is not how "new" works anywhere else.
# solution: functionArg constructs from values, functionArgFromID constructs from ID

type Query {
  functionArgFromID(id: FunctionArgID!): FunctionArg!
  functionArg(name: String!, typeDef: TypeDefID!): FunctionArg!
}

# problem: fights with your tab completion
# solution: functionArgFromID -> getFunctionArg

type Query {
  getFunctionArg(id: FunctionArgID!): FunctionArg!
  functionArg(name: String!, typeDef: TypeDefID!): FunctionArg!
}

# this feels OK? optimizes for the regular constructor, since getFoo tends to be used internally.
#
# getFoo has precedence from https://dgraph.io/docs/graphql/schema/types/#the-id-type
#
# this could/should be added for all existing ID-able objects (Container etc)

cc @dusk scaffold @sullen wyvern

I basically want to find a consistent pattern we can use to make any module object ID-able, and eventually adjust our own API to match. So far we've skirted around this issue by either having a valid 'zero value' for the major types (Directory = empty directory, Container = scratch container) or making it so you can only construct by FooID! or through other APIs (File). When the 'zero value' is valid you can just add an optional id param to the constructor, but it doesn't seem right to force every object to support a partial/empty state.

So this approach implies two constructors for every object: getFoo(id: FooID!): Foo! (ID always required), and foo(...): Foo! (which takes whatever required/optional args you want). The getFoo field would be more of a plumbing method available to SDKs or folks doing fun meta-things (Daggerverse?). For this thread, it would solve the issue by having a predictable way of going from an ID returned in a list of objects to a query selecting from that single object.

#

bikeshed welcome - I only went with "getFoo" because dgraph (which I've never used) went with it. Could also consider "loadFoo" or "_foo" or something.

dusk scaffold
#

I would probably rank your suggestion at the very end as my favorite, though I'd advocate going even more verbose/explicit with it:

type Query {
  loadFunctionArgFromID(id: FunctionArgID!): FunctionArg!
  functionArg(name: String!, typeDef: TypeDefID!): FunctionArg!
}

load*FromID is just very specific to what it's doing relative to get*, which is so generic that it in theory could make sense for either method. If I was autocompleting in my IDE, typed functionArg and saw getFunctionArg and functionArg available as the two closest matches, I'd have to look at the signatures+docs to know which one I should use.

Whereas if my IDE showed me loadFunctionArgFromID and functionArg, my brain would probably rule out loadFunctionArgFromID more quickly.

#

This is very much splitting already split hairs though 🙂 Realistically get* is totally fine too.

eternal jackal
#

Makes sense! I agree, we don't want it looking super prominent, and that fixes the tab-completion issue too. I like the 'FromXXX' pattern in general too, it felt like a shame to drop it

#

(IMO Container.import would be better as Container.fromOCITarball or something, for example)

shrewd dagger
#

Any progress? 🙂 Is there any way to get the type of a function arg in the mean time? I need it to support Directory and Secret arguments in the CLI, however not for today.

eternal jackal
shrewd dagger
#

I'd need to dynamically build a query string which is what the querybuilder already does but it's internal. Another option is the ast way like erik's done before.

#

Your raw query is easier if you have a known set of arguments.

#

It's going to be needed for the chaining in a next iteration anyway.

#

make querybuilder public or ast? 🙂

#

ast doesn't look too bad, but I remember @dusk scaffold complaining about it.

dusk scaffold
#

I was probably just whining 😄

eternal jackal
#

lol - FWIW I'm OK with just making querybuilder public; we already expose (*Client).Do, we might as well expose the thing that makes it easier to use, and it's not like it's a moving target with regard to API stability

dusk scaffold
eternal jackal
#

some day we'll probably reinvent our whole GraphQL stack, and at that point it can just become part of that package 😛

dusk scaffold
#

At least for Go

eternal jackal
#

Yeah there's a bit of a sprawl. Wonder if anyone has something worth extracting/collaborating on instead...

#

(another topic for another day)

dusk scaffold
shrewd dagger
#

So I guess it would be dagger.io/dagger/querybuilder?

#

What's the best way to get the graphql.Client instance from dagger.Client? Ok to expose a method for it or something else?

dusk scaffold
shrewd dagger
#

The querybuilder's Execute calls on internal functions for s.build() and s.unpack().

#

Execute is doing what Do does, but with those internal functions.

dusk scaffold
shrewd dagger
#

Yep, that'd be nice 🙂

#

I'll do it tomorrow 🙂