Open edouardruiz opened 3 years ago
Thanks @edouardruiz! Just wanted to add that this is issue is not just for Django but also for any other integration.
At the moment when using unions you need to return a Strawberry Type. We should allow to return any type, by either allowing to pass a function to unions that finds the correct type or by finding the right type automatically (not sure I like this option though)
It feels like it should be possible to solve this in strawberry-Django. @la4de is there a way to find the strawberry type from a Django model?
@jkimbo are you suggesting that create a resolve_type
for unions in strawberry django?
@patrick91 no, I was thinking that you could implement some logic in the strawberry-django resolver to look up the right StrawberryType from the Django instance. Something like this:
class StrawberryDjangoField(StrawberryField):
def get_result(self, source, info, arguments):
if self.resolved_type is Union:
model_instance = super.get_result(source, info, arguments)
strawberry_type = get_type_from_instance(model_instance, options=self.resolved_type.types) # this function will need to be implemented
return strawberry_type.from_instance(model_instance)
...
What do you think?
@jkimbo that sounds a like a good plan, we would still need to do a generic solution in future I think (for when you return dicts or other instances) :)
Is a method like strawberry_type.from_instance(o)
actually available? I'm trying to figure out how to deal with this in my code right now.
Normally if I have a query or mutation that returns an object I can say:
@strawberry.field
def foo() -> GQLFoo:
return Foo()
and strawberry seems to deal with generating a GQLFoo from my Foo so that its fields can work. Now, when I introduce a union:
@strawberry.field
def foo() -> Union[GQLFoo, OtherThing]:
return Foo()
it blows up with that error in the issue description. Is there a way I could write GQLFoo.bind(Foo())
to explicitly invoke the logic that strawberry must have somewhere to do this conversion?
Edit: In other words, even if I don't want "implicit" detection of the type, how can I do it explicitly? (without having to write out GQLFoo(a=foo.a, b=foo.b, c=foo.c)
, which doesn't work in my case anyway because I need the self
in resolvers inside of GQLFoo
to actually be my Foo
instance)
I have been spelunking the code and I guess the issue with my assumption is that a GQLFoo is never actually instantiated when I return a Foo instance even without a union. So a method like GQLFoo.bind or strawberry_type.from_instance
can't be implemented, at least not trivially.
I guess some sort of concept of a "bound instance" would need to be created? so that the resolving machinery can know which graphql type to look up resolvers on for a given concrete type?
I am pretty sure that Graphene solved this somehow, so there's probably some inspiration there
I have been spelunking the code and I guess the issue with my assumption is that a GQLFoo is never actually instantiated when I return a Foo instance even without a union. So a method like GQLFoo.bind or
strawberry_type.from_instance
can't be implemented, at least not trivially.I guess some sort of concept of a "bound instance" would need to be created? so that the resolving machinery can know which graphql type to look up resolvers on for a given concrete type?
I am pretty sure that Graphene solved this somehow, so there's probably some inspiration there
@radix I think this can be used by using resolve type on the union: https://github.com/graphql-python/graphql-core/blob/c214c1d93a78d7d1a1a0643b1f92a8bf29f4ad14/src/graphql/type/definition.py#L1004-L1020
/cc @bellini666 as you might have done something similar in strawberry django plus
Hey @patrick91 and @radix ,
If I'm understanding the issue correctly, it is on how to tell which type of a union the object is if it isn't actually an instance of it (e.g. a django model)
In both strawberry-graphql-django and strawberry-django-plus this issue was solved by implementing a custom is_type_of
which basically checks for isinstance(obj, (cls, model))
, the cls being the type class itself and the model the django model. Since the strawberry union implementation checks for that for non strawberry objects, it resolves to the correct type.
Here are both implementations for reference (both are injected when processing the type):
ps. The implementation of checking for cls.__dict__
is better because otherwise inheritances of that given type would not override that check.
I am pretty sure that Graphene solved this somehow, so there's probably some inspiration there
It actually solved in a way that also causes other issues =P
Graphene django has a register in which it basically created a map of model
=> type
, which gets populated automatically when you create a model. This is fine for 99% of the cases, but what happens if you have more than one type created for a given model?
E.g. I faced this in a graphene project where I had 2 types for the User model, a UserType
and a PublicUserType
. The public one was the one I used to return in public places, which only displayed public and insensitive user information. What happened is that the last created one was the one graphene was going to map always. Also, the relay interface became sensitive to only one type, since it only know the model for one of the types (it could be fixed if the map was a model
=> [type
]), so the other one did not exist based on that map.
I'd just like to suggest that I think having a supported, explicit mechanism to choose the variant, unrelated to any Django support, would be useful as the first solution to this problem (I assume _type_definition
is a private member). If we can return an object that includes both the value and the chosen variant, then an automatic variant inference could be built on top of that.
Can we bump this? It's kind of a show stopper and Graphene does this brilliantly with https://docs.graphene-python.org/en/latest/types/interfaces/. See the resolve_type
method at the bottom.
Hi,
Return an union of Django models isn't supported at the moment (
strawberry-graphql==0.74.1
andstrawberry-graphql-django==0.2.5
).This example will fail:
with this error:
One workaround is to set
_type_definition
on the retuned type like this:It would be great if we could return the Django objects directly.
Upvote & Fund