Skip to content

Add context parameter that passes through the API to resolvers. #82

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

Closed
wants to merge 1 commit into from

Conversation

augustoroman
Copy link
Contributor

This adds a net/context.Context parameter that is threaded through from
the calling API to any resolver functions. This allows an application
to provide custom, per-request handling when resolving queries.

For example, when working on App Engine, all interactions with the
datastore require a per-request context. Other examples include
authentication, logging, or auditing of graphql operations.

An alternative that was considered was to use an arbitrary, application-
provided interface{} value -- that is, the application could stick
anything in that field and it would be up to the app to handle it. This
is fairly reasonable, however using context.Context has a few other
advantages:

  • It provides a clean way for the graphql execution system to handle
    parallelizing and deadlining/cancelling requests. Doing so would
    provide a consistent API to developers to also hook into such
    operations.
  • It fits with a potentially upcoming trend of using context.Context
    for most HTTP handlers.

Going with an arbitrary interface{} now, but later using context.Context
for its other uses as well would result in redundant mechanisms to provide
external (application) metadata to requests.

Another potential alternative is to specifically provide just the
_http.Request pointer. Many libraries do this and use a global,
synchronized map[_http.Request]metadata lookup table. This would satisfy
the AppEngine requirements and provide a minimal mechanism to provide
additional metadata, but the global LUT is clumsy and, again, if
context.Context were later used to manage subprocessing it would provide
a redundant metadata mechanism.

This adds a net/context.Context parameter that is threaded through from
the calling API to any resolver functions.  This allows an application
to provide custom, per-request handling when resolving queries.

For example, when working on App Engine, all interactions with the
datastore require a per-request context.  Other examples include
authentication, logging, or auditing of graphql operations.

An alternative that was considered was to use an arbitrary, application-
provided interface{} value -- that is, the application could stick
anything in that field and it would be up to the app to handle it.  This
is fairly reasonable, however using context.Context has a few other
advantages:
  - It provides a clean way for the graphql execution system to handle
    parallelizing and deadlining/cancelling requests.  Doing so would
    provide a consistent API to developers to also hook into such
    operations.
  - It fits with a potentially upcoming trend of using context.Context
    for most HTTP handlers.

Going with an arbitrary interface{} now, but later using context.Context
for its other uses as well would result in redundant mechanisms to provide
external (application) metadata to requests.

Another potential alternative is to specifically provide just the
*http.Request pointer.  Many libraries do this and use a global,
synchronized map[*http.Request]metadata lookup table.  This would satisfy
the AppEngine requirements and provide a minimal mechanism to provide
additional metadata, but the global LUT is clumsy and, again, if
context.Context were later used to manage subprocessing it would provide
a redundant metadata mechanism.
@augustoroman
Copy link
Contributor Author

Sigh. For some reason, I just didn't see other pull request and ongoing discussion: #25. I'll close this now, but feel free to re-open and/or merge if you like the implementation.

@chris-ramon chris-ramon mentioned this pull request Jan 5, 2016
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant