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

Queries do not support multiple inheritance #55

Closed
adamcharnock opened this issue Nov 30, 2015 · 4 comments
Closed

Queries do not support multiple inheritance #55

adamcharnock opened this issue Nov 30, 2015 · 4 comments

Comments

@adamcharnock
Copy link
Contributor

Use Case: splitting out query definition logic into smaller and more manageable files (e.g. in Django app directories)

Something like this will not currently work:

schema = graphene.Schema(name='Swiftwind Schema')


class Query1(ObjectType):
    all_accounts = relay.ConnectionField(accounts_nodes.Account)
    account = relay.NodeField(accounts_nodes.Account)


class Query2(ObjectType):
    all_transactions = relay.ConnectionField(accounts_nodes.Transaction)
    transaction = relay.NodeField(accounts_nodes.Transaction)


class Query3(ObjectType):
    all_users = relay.ConnectionField(accounts_nodes.User)
    resolve_all_users = ConnectionResolver(accounts_nodes.User)


class Query(Query1, Query2, Query3):
    pass


schema.query = Query

It dies with the following error:

  File "/Users/adam/Envs/swiftwind/src/graphql-core/graphql/core/type/schema.py", line 30, in __init__
    assert isinstance(query, GraphQLObjectType), 'Schema query must be Object Type but got: {}.'.format(query)

I have also tried:

schema = graphene.Schema(name='Swiftwind Schema')


class Query1(object):
    all_accounts = relay.ConnectionField(accounts_nodes.Account)
    account = relay.NodeField(accounts_nodes.Account)


class Query2(object):
    all_transactions = relay.ConnectionField(accounts_nodes.Transaction)
    transaction = relay.NodeField(accounts_nodes.Transaction)


class Query3(object):
    all_users = relay.ConnectionField(accounts_nodes.User)
    resolve_all_users = ConnectionResolver(accounts_nodes.User)


class Query(Query1, Query2, Query3, ObjectType):
    pass


schema.query = Query

Which dies with the error:

  File "/Users/adam/Envs/swiftwind/src/graphql-core/graphql/core/type/definition.py", line 211, in define_field_map
    ).format(type)
AssertionError: Query fields must be a mapping (dict / OrderedDict) with field names as keys or a function which returns such a mapping.

Presumably because the fields never get initialised?

Anyway, if this is possible I'd love to know how.

@syrusakbary
Copy link
Member

The main reason is:

  • In the first example, the Query object, as inherits more than one ObjectType is automatically mapped to a Union type. Some how graphql doesn't allow union types for the root query object.
  • In the second example, the objects are not inheriting for ObjectType, so when Query inherit those objects is not copying their fields.

However I certainly believe we should be able to do this in a good way.
I'm thinking about create the concept of abstract ObjectTypes so you can extend from them without converting in a UnionType.
Something like:

class Query1(ObjectType):
    class Meta:
        abstract = True
    all_accounts = relay.ConnectionField(accounts_nodes.Account)
    account = relay.NodeField(accounts_nodes.Account)


class Query2(ObjectType):
    class Meta:
        abstract = True
    all_transactions = relay.ConnectionField(accounts_nodes.Transaction)
    transaction = relay.NodeField(accounts_nodes.Transaction)


class Query3(ObjectType):
    class Meta:
        abstract = True
    all_users = relay.ConnectionField(accounts_nodes.User)
    resolve_all_users = ConnectionResolver(accounts_nodes.User)


class Query(Query1, Query2, Query3):
    pass

Thoughts?

@adamcharnock
Copy link
Contributor Author

I think that looks like a good solution. It like that it has some parity with Django's models. Ideally we wouldn't have to specify that a Query is abstract and things would just work, but given that this is not possible I think this is a good solution. 👍

@ustun
Copy link
Contributor

ustun commented Dec 15, 2015

This is an interesting topic. Should each django app have its own schema? Then, how do we prevent name collisions? Should we be prefixing the resources then or should each app be serving its own graphql schema at a different URL?

Some of these are subjective, but it would be better if we could give some recommended best practices to the community there.

@jcouser
Copy link

jcouser commented Aug 16, 2017

Hi,

I'm attempting to create this level of abstraction as well. I'm using graphql-sqlalchemy and when I do something like:

class UserGroupMember(SQLAlchemyObjectType):
    class Meta:
        abstract = True
        model = UserGroupMemberModel
        interfaces = (CustomNode, )

I'm getting the following error:

TypeError: Invalid attributes: abstract

p0123n pushed a commit to p0123n/graphene that referenced this issue Aug 26, 2017
don't assume property is not nullable when no "nullable" field on property
ronachong pushed a commit to ronachong/graphene that referenced this issue Nov 3, 2017
…tions

Add docs about how to build docs, and add sphinx as docs requirement.
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

4 participants