Subscriptions and Union inline fragments #23
Conversation
…tation Allow the __as__ operator to be used on Unions
There was a problem hiding this comment.
hi, I couldn't test yet, but the PR looks good. Could you take a look at my comments and update the commit (fixup/amend)?
As for the subscriptions, I do want to support, but I did not invest time as I'm not using it at this moment. If you wish to contribute that back I can help to get that in. Maybe we could add the aiohttp to provide the async/await http requests and with that also bring the https://docs.aiohttp.org/en/stable/client_quickstart.html#websockets ?
| @staticmethod | ||
| def supports_fields_query(type): | ||
| return issubclass(type, ContainerType)\ | ||
| or issubclass(type, Union) |
There was a problem hiding this comment.
there is no need to create a class to provide a static method. Also you can do in a single issubclass(type, (ContainerType, Union)).
Other than that, check the flake8, it should complain about the type being a reserved keyword, so use type_ or t or typ_ or something like that :-)
| @@ -1223,7 +1228,7 @@ class SelectionList: | |||
| __slots__ = ('__type', '__selectors', '__selections', '__casts') | |||
|
|
|||
| def __init__(self, typ): | |||
| assert issubclass(typ, ContainerType), str(typ) + ': not a container' | |||
| assert SelectionHelpers.supports_fields_query(typ), str(typ) + ': not a container' | |||
There was a problem hiding this comment.
maybe change not a container to not selectable?
| @@ -1446,18 +1451,24 @@ def __init__(self, typ=None, name=None, **args): | |||
| name = typ.__name__ | |||
|
|
|||
| self.__type = typ | |||
| self.__kind = self.__resolve_kind(type, kind) | |||
There was a problem hiding this comment.
is this correct? I believe there is a typo, typ is the parameter, while type should be the global function.
it happens to work because you don't use this parameter, rather use self.__type.__name__ inside the function __resolve_kind()
| return cls.__meta_fields[key] | ||
|
|
||
| @property | ||
| def __meta_fields(cls): |
There was a problem hiding this comment.
could you follow with the same code snippet as with ContainerTypeMeta, with __populate_meta_fields()? Maybe make it a shared metaclass on top of BaseMeta where both ContainerTypeMeta and UnionMeta inherit, providing that meta fields
Hi,
I am using this project in a scope that requires subscriptions (I use https://github.com/tdip/gql-subscriptions/tree/develop as the client in case it is of any interest to make it part of this repo as well) therefore I added support for the subscription keyword.
Also, did minor modifications so the "as" method can be used with unions.
Not sure if any of these approaches is the right one, so let me know any feedback you may have and I am happy to improve the code.
Cheers