Graphene: Discussion - Pass context to resolve methods

[mjtamlyn]

The context story seems very confused in the newest version. My understanding from the graphql-js 0.5.0 release notes is that all resolving functions should now have the signature ([self,] args, context, info), as opposed to ([self,] args, info).

At the moment, if I update a resolve_NAME method to take context, then it throws argument errors.

Also, mutations don't take context and therefore they cannot access context data. It think a patch similar to this one on graphql-relay-js is necessary for the relay mutations at least, but probably for all of them.

[danielfarrell]

That PR seems to address this issue for me, and I've got it working locally. If there should be anything else on there, let me know.

[syrusakbary]

@mjtamlyn @danielfarrell thanks for opening the issue and the PR.

For achieve more simplicity, context and info args will be not mandatory in the resolver in the next stable version of graphene (see Roadmap).
So resolvers could access the context and info only if they requested explicitly by using the with_context and with_info decorators.

The current implementation only uses with_context, as step in the middle before the next stable release.

_PS: In JS this is a little bit easier as you could call a function with less arguments than required without triggering an error as it will complete the argument values with undefined, so they didn't have this problem_

Makes sense?

[mjtamlyn]

Right, I think that makes sense. Python actually wanting a proper set of arguments to a function does complicate the issue slightly.

By the way, there's no tagged release or changelog I can find for 0.9 which did make this slightly more complicated to work out! I'll give it a go using with_context.

[mjtamlyn]

Ok, I've got it working fine apart from relay mutations which need a bit of extra work I've done in the PR. It would also be very helpful to add some docs about with_context and how to use it.

[danielfarrell]

Dangit, how was I all over the codebase yesterday and didn't see with_context. Ha, I'll close the PR.

[Globegitter]

@syrusakbary is it possible to maybe have this as an option? We use the context all over the place and it would be quite a pain to have to add the with_context decorator to 90% of our resolve function. Also why then not just write resolve_bla(self, _, __) if you don't need the arguments. Or resolve_bla(self, *_) or resolve_bla(self, **_) depending on if they are being passed in as positional or keyword args.

Especially the * versions seems to me simpler than having to remember and import different decorators depending on what you need. I hope that decision is not fully set in stone yet.

[mjtamlyn]

I would agree that resolve_blah(self, args, **options) where options is now info=, context= but can be extended later would be more pythonic.

[syrusakbary]

I would like to discuss about that.
Somehow I like having the args exposed directly as keyword arguments in the resolver.

Like:

class Query(graphene.ObjectType):
    say = graphene.String(what=graphene.String())

    def resolve_say(self, what):
        return "Hey {}".format(say)

I think it's too verbose having info and context as required arguments... as are not going to be used always. Since we moved the context outside info, do we really use/need info in the general case? And therefore... it should be a required arg/kwarg?

Maybe using decorators (like with_context, with_info) could have some side effects, so I don't mind to revisit.

I would like to get into a solution that could solve both points of view, or at least get closer.

Merging args and context, info in one dict is another possible solution (like: dict(args, context=context, info=info)).
But then, we could have collision between argument names and context/info.

More proposals? Thoughts?

[Globegitter]

@syrusakbary yeah that is a quite difficult one. I mean the only other solution I could think of then is dict(args=args, context=context, info=info) to not have potential name conflict. I personally would prefer that all to the decorator.

[mjtamlyn]

Ok, so this is a tricky design decision in some ways because we want to mimic the "argument magic" that javascript can manage in python. We've also got a possible name clash issue.

Here are a few possible approaches:

• Use decorators to explicitly mark which resolvers need which arguments. This is the current proposal. It doesn't feel very pythonic. We could have a convention that context and info could be passed as _context and _info if there's a name clash, or always.
• Use the inspect module to find the signature. inspect.getargspec(resolver_func).args will return a list of names of arguments. If that includes context or info then we pass them. This is "fully" magical, but I don't find it any more distasteful than the _root/getattr pattern we have at the moment. I'm not sure if we can get access to which arguments might be relevant to deal with name clashes, but it would be easy to see if a user is expecting context AND _context and pass the arg as the former and the context as the latter.
• Define a more pythonic standard API for a resolver. Something like def resolve_foo(self, arg1, arg2, **options). Again we can deal with name clashes with _context if necessary, or just have a convention that you don't do that. I feel this is the route we would be taking if there wasn't the JS reference implementation.

[syrusakbary]

Hi @mjtamlyn, @Globegitter.

I've been researching and Jinja use the decorators for passing some extra args to the function.
http://jinja.pocoo.org/docs/dev/api/#jinja2.contextfunction
Implementation: https://github.com/pallets/jinja/blob/75685ec5e5079bcdbd443696c3d79d0cb86f7cf8/jinja2/utils.py#L41

Also, if this is the chosen path for graphene and context/info in functions could be completely possible to skip this behavior. (as it will be done via middleware).

I'm not very sure yet about what's the good choice here though.

Another option, is grouping context and info in other instance and pass something like:

def resolve_blablabla(self, context_with_info, **args):
    pass

[ekampf]

Maybe its coming from the Ruby world... but I kinda like the "magical" approach of automatically injecting values by argument name using inspect.getargspec(resolver_func).args (and inspect.signature() in 3.6+).
This way you can automatically not just context and info but args like in @syrusakbary's example too.

This automatic objection could be implemented as a decorator too... (which can help resolving possible name by providing some sort of mapping between default name and var name...)

[syrusakbary]

@ekampf @mjtamlyn @Globegitter I've been thinking in a good way to solve this for a long time.

If we use a magical approach using inspect, we could make the api's more obscure and handling things like middleware could become a problem (as the middleware functions have to be aware of the resolver context based on the inspection of the original function).

Also, the resolver in graphene should have exactly the same structure as is in graphql-core.

I'm thinking of merging back context into info, so you can have functions like:

def resolve_blablabla(self, args, info):
    context = info.context
    # ...

Thoughts?

[mjtamlyn]

I think args, info is the simplest option, I don't think we need extra arguments.

[Globegitter]

@syrusakbary Having thought about it a bit more I am leaning mostly towards the inspect.getargspec(resolver_func).args approach suggested by @ekampf. That would make it really straightforward and simple to use.

Also if the arguments passed in would ever change again it that approach would imo make it a bit simpler.

That all said though, I do think your suggested approach also works.

[syrusakbary]

I think I found a way to make the resolvers much more fun to write using type annotations in Python 3.

Would love to get some feedback! @Globegitter @mjtamlyn @ekampf
Code and examples of usage in the PR: #500

[FrankSalad]

According to the docs: http://docs.graphene-python.org/en/latest/types/objecttypes/

By default resolvers take the arguments args, context and info.

Followed by

    def resolve_reverse(self, info, word):
        return word[::-1]

As you can see, the example resolver method only has the self, info and word params. There was no usage of args or context at all, and no mention of what I should actually find passed in info. I'm still digging around to try to understand what I should expect in these params. This is confusing and unpythonic. The expectation around function params is that if they're positional, their actual names don't matter. I had to find this issue to understand what you guys were even trying to do here. If your intent was to create a function that ignores some of its arguments, adding **kwargs to the end to do so explicitly is the right way.

[vinigfer]

In case anyone is also having troubles on how to access the context, after an endless effort/search I found on this link:

http://docs.graphene-python.org/projects/django/en/latest/authorization/

that I can access context like this:

if not info.context.user.is_authenticated():

Hope this helps anyone

[jkimbo]

Hi @mjtamlyn . We're currently going through old issues that appear to have gone stale (ie. not updated in about the last 6 months) to try and clean up the issue tracker. If this is still important to you please comment and we'll re-open this.

Thanks!