Closed
Description
In my server, I have the following lines: https://github.com/ezyang/ghstack/blob/973ef7b25a71afa8f813cd8107f227938b3413f1/ghstack/github_fake.py#L288
GITHUB_SCHEMA.get_type('Repository').is_type_of = lambda obj, info: isinstance(obj, Repository) # type: ignore
GITHUB_SCHEMA.get_type('PullRequest').is_type_of = lambda obj, info: isinstance(obj, PullRequest) # type: ignore
I don't know how to put these on the Repository/PullRequest classes, in the same way resolvers can be put on the appropriate object class and then automatically called. The obvious things (e.g., adding an is_type_of
method) do not work.
Activity
Cito commentedon Mar 6, 2019
HI @ezyang, thanks for reporting this issue.
is_type_of
is actually a method of the GraphQL type which checks whether the given object is of that type. What you're probably looking for is the__typename
attribute which you can set on objects to specify their GraphQL type as a string and which is checked bydefault_resolve_type_fn
. This is the way it's done also in GraphQL.js which graphql-core-next copies.However, I see that that graphql-core-next currently checks
__typename
only on objects that are returned as dicts, not on (data)class instances like in your case.It would certainly be a good idea and very simple to extend graphql-core-next to check for
__typename
attributes on non-dict objects as well - you would simply set__typename = 'Repository'
on your Repository class. Unfortunately, there is an issue here in that Python applies name mangling to attributes starting with two underscores (a feature intended to be used for fake "private" attributes that is frowned upon but unfortunately still exists in Python 3).So probably we should use the special name
__typename__
instead (these names do not underly name mangling). It's not nice, since these special names are actually reserved for Python, and we deviate a bit from the GraphQL API, but it's the only obvious solution that comes to my mind just now. What do you think?ezyang commentedon Mar 7, 2019
A special name like
__typename__
seems fine to me, as long as it's prominently documented.Note that it's not impossible to demangle it, if you really wanted to. But I think it's probably better here to just hew to Pythonic conventions.
Cito commentedon Mar 7, 2019
I also considered demangling and immediately dismissed the idea. But now after sleeping over it, I am thinking maybe it is not so bad an idea after all. The rule for the mangling - prefixing with the class name- is well documened and not implementation specific, and the class name can be easily inspected. That would allow us to be completely compatible to GraphQL.js and consistent between dicts and class based objects.
Another option would be check both attributes (
__typename
and__typename__
), but that would go against the Zen of Python.Check for `__typename` of objects (solves #25)
Cito commentedon Mar 7, 2019
So I decided to go with the demangling. This will be available in the next version (released soon).
Please reopen if you have any better ideas.
Cito commentedon Mar 10, 2019
Version 1.0,2 with this fix has now been released on PyPI.
Hellzed commentedon Feb 20, 2020
Hi @Cito , we might need to reopen this!
Unfortunately, in cases such as using an inherited
__typename
property, basic demangling is not enough.Let's imagine I have the following (Relay inspired) Node and subclass:
Obviously
_A__typename
will not be found, and we will ignore_Node__typename
.Something like this (instead of just using a simple
getattr()
) would help instead:What do you think?
Cito commentedon Feb 20, 2020
Good point @Hellzed. Will add your
A(Node)
example and maybe and a more complicated one with multiple inheritance to the test suite. Your suggested code will probably solve this.Support inheritance of __typename attribute (#25)
Cito commentedon Feb 21, 2020
This is implemented now in the master branch and will be available in the next beta release.
2 remaining items