Skip to content
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

add apollo graphql to graphql branch (#159) #173

Merged
merged 36 commits into from
Mar 5, 2020
Merged

add apollo graphql to graphql branch (#159) #173

merged 36 commits into from
Mar 5, 2020

Conversation

yingwang-us
Copy link
Contributor

@yingwang-us yingwang-us commented Feb 21, 2020

Following changes are made based on Mike's code review comments.

  1. Add ENABLE_GRAPHQL environment variable , default is false. see here and here

  2. Add req_id to graphql context, which is then used by logger to print debug information with the req_id. see here for setting and here for example how to use. Note, for web socket connections, the req_id on the context is the original http upgrade request req.id.

  3. Add apollo's bunyan config here . This is because graphql has common url and we need to turn on body log, so we could know what graphql api is called. Also include some optimization to disable debug logs for playground ping requests.

  4. Add identifier in user schema and make email as optional. It is because some tokens might not be associated with any email, e.g. a token generated from service-id api key.

  5. other minor changes mentioned in Mike's code review under
    graphql (see the latest in graphql branch or this PR #173) #142

yingwang-us and others added 3 commits February 13, 2020 17:31
* graphql

* update

* update

* update

* update

* update

* update per review comments

* update

* update

* update apollo and graphql package vesions

* update

* update

* update

* update

* update

* update
@yingwang-us yingwang-us changed the title add graphql to graphql branch (#159) add apollo graphql to graphql branch (#159) Feb 21, 2020
package.json Show resolved Hide resolved
@yingwang-us yingwang-us requested a review from mckaymic February 26, 2020 21:21
@yingwang-us
Copy link
Contributor Author

temporarily turned off vulnerability 1217 for audit since it does not impact prod image. Still waiting kevva/decompress#71 to be fixed.

Dockerfile Outdated Show resolved Hide resolved
@yingwang-us yingwang-us requested a review from mckaymic March 4, 2020 17:48
@yingwang-us yingwang-us merged commit 0206ebd into master Mar 5, 2020
@mckaymic mckaymic deleted the graphql branch April 2, 2020 00:10
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.

2 participants