-
Notifications
You must be signed in to change notification settings - Fork 828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document resolvers #654
Comments
This doesn’t seem to be supported with use of schema interfaces. There may be a way to do it, but a direct adaptation of the above doesn’t work (note that Take the following schema: import graphene
def resolve_test(*_):
return TestInstance()
class TestInstance:
id = 1
x = 1
y = 2
class Test(graphene.ObjectType):
x = graphene.Int()
y = graphene.Int()
class Meta:
interfaces = (graphene.relay.Node,)
@classmethod
def get_node(*_, **__):
return resolve_test()
class Query(graphene.ObjectType):
node = graphene.relay.Node.Field()
test = graphene.Field(Test, resolver=resolve_test)
schema = graphene.Schema(query=Query) Now we can execute the following query: { test { id x y } } And get this result: {
"data": {
"test": {
"id": "VGVzdDox",
"x": 1,
"y": 2
}
}
} But if you try: { node(id: "VGVzdDox") { ... on Test { id x y } } } You will get the following error: Presumably the limitations placed on the resolved object are to ensure that the executor can determine its type, since it does not know the type a priori in this case. |
In any event, I am curious what you are doing that makes this so useful. |
@altaurog yep I've come across this issue before with the class RelayNode(relay.Node):
class Meta:
name = 'Node'
description = 'An object with an ID'
@classmethod
def resolve_type(cls, instance, info):
if isinstance(instance, TestInstance):
return Test
return cls.resolve_type(instance, info) |
The answer here is twofold: one for my particular use case, and one for why in general it's useful. The reason I think it's useful in general is that it means that you can continue to reuse whatever your data model type is, instead of having to reassign useful data to your graphene In my case, it means that I can just pass around Django Notice, for example, in this snippet. I It sounds complicated but it really makes sense once you're used to it. In my original bug posting, I mentioned that the documents hint at this functionality. That hinting happens here:
|
@jkimbo thanks. The value I see in this is it helps break the dependency between the schema types and the resolvers and thereby allows more flexibility in the organization of the code. It also allows me to cache additional data on the objects without having to play tricks to allow that. I am adapting your code as below. Also I figured out how to make a from graphene import relay
class Node(relay.Node):
"""
Custom Node interface type
The purpose of this type is to allow resolvers to return objects
which are not instances of graphene.ObjectType, but which has the
same name as a graphene type in the schema. The graphene type will
be inferred from the class name.
"""
class Meta:
"""meta class"""
name = 'Node'
description = 'An object with an ID'
@classmethod
def resolve_type(cls, instance, info):
"""Assume the class name is the same"""
return type(instance).__name__
class ConnectionField(relay.ConnectionField):
"""
Custom connection field interface type
The purpose of this type is to allow relay connection resolvers to return a dict
which can be used to instantiate the connection object.
"""
@classmethod
def resolve_connection(cls, connection_type, args, resolved):
if isinstance(resolved, dict):
return connection_type(**resolved)
return super().resolve_connection(connection_type, args, resolved) |
@altaurog I'm not sure what you are referring to when you say
Could you expand? Also I dont think assuming the name of the data source instance will be the same as the graphene type is a good idea. Your schema will likely (and should) diverge from your underlying data structure so it's unlikely that assumption will hold for very long. |
Let’s say I have the following in my schema: class BlogEntry(graphene.ObjectType):
datePublished = graphene.String()
title = graphene.String()
author = graphene.Field(lambda: Author, resolver=resolve_blog_author)
content = graphene.String()
class Meta:
interfaces = (relay.Node,)
class BlogEntryConnection(graphene.Connection):
class Meta:
node = BlogEntry
class Author(graphene.ObjectType):
name = graphene.String()
blogEntries = relay.ConnectionField(
BlogEntryConnection,
resolver=resolve_author_blog_entries
) Unfortunately, there’s a bit of a circular reference between But this conversation is about resolvers. Now if the resolvers had to return the actual schema types, we’d have more interdependencies. The schema classes depend on the resolvers and the resolvers depend on other classes in the schema. Which again, it seems to me, makes it difficult to organize my resolvers in their own modules. So everything goes in one big module with all these circular references. But since the resolver can return any kind of object, it does not have to depend on the schema and I can move it elsewhere. Regarding your second point, that the schema is likely to diverge from the underlying data structure, I agree with you 100%, which is why I would never return an ORM object or queryset from a reducer. I created a separate set of intermediate plain-old-data classes which I instantiate in my resolver with the data it loads. Those classes serve no purpose other than to hold the data and specify the type, or in other words to look like the schema types without actually being the schema types, so there is no risk in relying on the name. (It seems to me that there are plenty of devs who are willing to couple their schema quite tightly to the underlying data structure, though. Isn’t that what graphene-django does?) |
Ah, never mind. I just realized I could return the graphene type without referencing it directly using |
Another thing I want to do with the return type is cache additional information on it. For example, with the query:
If the underlying data source were an RDBMS I would probably do a join like this:
Then I could instantiate an
But it would be a tad nicer IMO to be able to do the following, which isn’t allowed with the graphene types:
|
It's worth noting, too, that you don't even need |
@Lucretiel That works if all your types are in one file, but as soon as you have types that reference one another (for example, Student has a Teacher, and Teacher teaches Students), which is extremely common, you'll run into circular import errors. |
@ProjectCheshire thanks, that is useful. I wrote the following wrapper to allow relative imports when called as def lazy_import(path, relative_to_path=''):
if path.startswith('.'):
return graphene.lazy_import(relative_to_path.rsplit('.', 1)[0] + path)
return graphene.lazy_import(path) |
ObjectType documentation has been updated to be much more detailed in how to use resolvers. |
Unfortunately, it is still not clear what the return type of the ObjectType field resolver should be. Should it be an instance of ObjectType or any object that has the required fields? |
@knivets both will work. You can even return a dict with the right attributes. |
Got it, but what is the best practice? Is there a rule of thumb which one should I use? |
Depends on how your data is modelled really. Using the ObjectType as a container for your data doesn't provide any benefits really. |
I just learned, completely by accident, one of the most critically useful features of Graphene: resolvers can return anything, even for object resolvers! They don't have to return instances of graphene.ObjectType. So, this works:
While this is hinted at in the ObjectType reference docs, there are no code examples of the pattern, nor documentation of any kind about the behavior of resolver functions. Documentation about this feature, and guidance about nested Objects, should be added.
I'm trying to draft something but I don't have a lot of time these days so I wanted to file an issue just in case
The text was updated successfully, but these errors were encountered: