Translatable descriptions

[ktosiek]

graphene-django 2 passes a lazy translated string as description if that's what's used on the model. This way the descriptions can be translated to the language current at the introspection time.

But graphql-core 3 asserts the description is an str, which forces choosing the translation at the definition time.
I couldn't find such type check in GraphQL.js - they just declare descriptions as strings, but don't check that (at least not in definition.js).

Would it make sense to loosen that restriction a bit? Or even drop the type check, and add an str() around descriptions in introspection?

I see similar issue would apply to deprecation_reason - they are both human-readable text.

[Cito]

Sounds reasonable. Actually it already works when you set the description manually. Here is an example:

class LazyText:
    def __init__(self, text):
        self.text = text
    def __str__(self):
        return self.text + ' (lazily evaluated)'

from graphql import *

queryType = GraphQLObjectType('Query', {
    'someField': GraphQLField(GraphQLString)},
    description='test description')

queryType.description = LazyText(queryType.description)
schema = GraphQLSchema(query=queryType)
query = get_introspection_query(descriptions=True)
result = graphql_sync(schema, query)
print(result.data['__schema']['types'][0]['description'])

We would need to remove the restriction for descriptions to be strings in the constructors of all the GraphQL types. Anything that can be converted to a string with str() (which is literally anything in Python) would then qualify as a description.

But removing the type hints and runtime type checks has also disadvantages; these can prevent real errors when you pass wrong arguments.

One solution would be to have the lazy texts masquerade as strings, then everything would just work:

class LazyText(str):
    def __init__(self, text):
        self.text = text
    def __str__(self):
        return self.text + ' (lazily evaluated)'

from graphql import *

queryType = GraphQLObjectType('Query', {
    'someField': GraphQLField(GraphQLString)},
    description=LazyText('test description'))
schema = GraphQLSchema(query=queryType)
query = get_introspection_query(descriptions=True)
result = graphql_sync(schema, query)
print(result.data['__schema']['types'][0]['description'])

Unfortunately, the Django lazy translations don't do that:

>>> from django.utils.translation import gettext_lazy
>>> isinstance(gettext_lazy('text'), str))
False

One solution would be to put a second wrapper around the Django translation:

from django.utils.translation import gettext

class GetTextLazy(str):
    def __init__(self, msg):
        self.msg = msg
    def __str__(self):
        return gettext(self.msg)

from graphql import *

queryType = GraphQLObjectType('Query', {
    'someField': GraphQLField(GraphQLString)},
    description=GetTextLazy('test description'))
schema = GraphQLSchema(query=queryType)
query = get_introspection_query(descriptions=True)
result = graphql_sync(schema, query)
print(result.data['__schema']['types'][0]['description'])

I''let this set for a while to see if we can come up with other/better ideas before starting to remove all the checks. Maybe it could be made configurable somehow. E.g. there could be a global DescriptionType = str which could be monkeypatched to another class or tuple of classes or to None to completely remove type checks for descriptions.

[ktosiek]

Another idea would be to use an ABC for DescriptionType, to let library users register their own types. But it seems mypy doesn't support ABCMeta.register(), which would be a problem when typing the uses of graphql-core.

[Cito]

Interesting idea. But as you already mentioned, it would only work for the runtime checks. And you would need to know the base class for the lazy strings which can be somewhat obscure (it's called "Promise" in Django, for instance) and might not even be publicly accessible. And you can't completely switch off the type checks that way - DecriptionType.register(object) does not work.

[Cito]

I think a global variable would be simpler and more straightforward, though a bit "unclean". E.g. we can have a module pyutils.types with a global variable Description = str, and types.Description would be used both as type alias for the type hint description: Optional[types.Description] and for the runtime check isinstance(description, types.Description). You could then set types.Description = (str, LazyString) while building the schema, or even pyutils.Description = object to switch the runtime check off.

[Cito]

Btw, the Django docs mention this problem here, it also exists with other libraries. The recommended solution to cast to str() would not work in this case though. It really should happen only later in the resolvers.

[Cito]

I have now implemented a different solution that allows registering additional classes that can be used as descriptions. Please reopen if you see any problems with this solution or have a better idea.