Skip to content
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

Closed
Lucretiel opened this issue Jan 27, 2018 · 18 comments
Closed

Document resolvers #654

Lucretiel opened this issue Jan 27, 2018 · 18 comments

Comments

@Lucretiel
Copy link

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:

class TestInstance:
    x = 1
    y = 2

class Test(graphene.ObjectType):
    x = graphene.Int()
    y = graphene.Int()

class Query(graphene.ObjectType):
    test = Test()
    def resolve_test(self, info):
        return TestInstance()

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

@altaurog
Copy link

altaurog commented May 3, 2018

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 test = Test() is incorrect).

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:
graphql.error.base.GraphQLError: Abstract type Node must resolve to an Object type at runtime for field Query.node with value "<test.graphql.testschema.TestInstance object at 0x7fa84c5b05c0>", received "None".

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.

@altaurog
Copy link

altaurog commented May 3, 2018

one of the most critically useful features of Graphene

In any event, I am curious what you are doing that makes this so useful.

@jkimbo
Copy link
Member

jkimbo commented May 3, 2018

@altaurog yep I've come across this issue before with the Node type. The way we worked around it was to create our own custom Relay.Node type and use that everywhere instead. That then gives you the opportunity to define a resolve_type method on the Node interface that can translate your underlying data structure into a Graphene type. E.g:

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)

@Lucretiel
Copy link
Author

Lucretiel commented May 3, 2018

one of the most critically useful features of Graphene

In any event, I am curious what you are doing that makes this so useful.

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 ObjectType instances.

In my case, it means that I can just pass around Django Model instances, rather than having to populate my ObjectType. This is especially important because Django does semi-lazy lookups and queries based on attribute access, so it's incredibly useful to have graphene look up only the desired attributes. My code is here: https://github.com/Lucretiel/SkillServe/blob/v2018/skillboards/graphql/query.py?ts=4

Notice, for example, in this snippet. I resolve_games returns a Django QuerySet, which is unresolved until graphql actually iterates over it to get the individual database rows. The objects of the QuerySet are, themselves, not graphql objects, but Model instances. GraphQL knows from the schema that Player.games is a list of schema.Game, and automatically applies that schema to the models.Game instance.

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:

NOTE: The resolvers on a ObjectType are always treated as staticmethods, so the first argument to the resolver method self (or root) need not be an actual instance of the ObjectType.

@altaurog
Copy link

altaurog commented May 13, 2018

@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 ConnectionField such that the resolver does not need the connection type:

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)

@jkimbo
Copy link
Member

jkimbo commented May 13, 2018

@altaurog I'm not sure what you are referring to when you say

helps break the dependency between the schema types and the resolvers and thereby allows more flexibility in the organization of the code

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.

@altaurog
Copy link

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 BlogEntry and Author, which we were able to set up using a lambda in the former. IMO it would be much nicer to specify just class name and resolve the type lazily, but that does not seem to be supported directly. So although I might ideally want to split my schema types up and define them in several different modules, I think it would be a bit awkward to do that. Is there a way?

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?)

@altaurog
Copy link

altaurog commented May 15, 2018

Ah, never mind. I just realized I could return the graphene type without referencing it directly using info.return_type.graphene_type.

@altaurog
Copy link

Another thing I want to do with the return type is cache additional information on it. For example, with the query:

{ blogEntries(first: 2) { edges { node { title author { name } } } } }

If the underlying data source were an RDBMS I would probably do a join like this:

SELECT blog_entry.blog_entry_id, blog_entry.title, author.author_id, author.name
FROM blog_entry JOIN author USING (author_id)
ORDER BY blog_entry.blog_entry_id
LIMIT 2

Then I could instantiate an Author object and attach it to each BlogEntry object in resolve_blog_entries. Then I can check info.root in resolve_blog_author to see if it already has the Author instance available and if so, return it. Now there’s no reason why I couldn’t do this with the graphene type, so:

author = Author(**author_data)
blog_entry = BlogEntry(**blog_entry_data)
blog_entry.author = author

But it would be a tad nicer IMO to be able to do the following, which isn’t allowed with the graphene types:

author = Author(**author_data)
blog_entry = BlogEntry(author=author, **blog_entry_data)

@ProjectCheshire
Copy link
Member

@altaurog actually graphene has a thing called lazy_import you can import.
Scroll down on #714

Circular refs dodged!

@Lucretiel
Copy link
Author

It's worth noting, too, that you don't even need lazy_import in most cases. It's not well documented, but you can just use field = lambda: Type instead of field = Type in most cases.

@cmmartti
Copy link

cmmartti commented Sep 24, 2018

@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.

@altaurog
Copy link

altaurog commented Oct 9, 2018

@ProjectCheshire thanks, that is useful. I wrote the following wrapper to allow relative imports when called as lazy_import('.site.Site', __name__):

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)

@jkimbo
Copy link
Member

jkimbo commented Dec 27, 2018

ObjectType documentation has been updated to be much more detailed in how to use resolvers.

@jkimbo jkimbo closed this as completed Dec 27, 2018
@knivets
Copy link

knivets commented May 6, 2021

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?

@jkimbo
Copy link
Member

jkimbo commented May 7, 2021

@knivets both will work. You can even return a dict with the right attributes.

@knivets
Copy link

knivets commented May 7, 2021

Got it, but what is the best practice? Is there a rule of thumb which one should I use?

@jkimbo
Copy link
Member

jkimbo commented May 7, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants