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

feat: Add support for including excepting SourceCharacter to printDescription #3765

Conversation

feuxfollets1013
Copy link

@feuxfollets1013 feuxfollets1013 commented Feb 7, 2020

This PR enable to print description which includes excepting SourceCharater(" or \ or LineTerminator).

According to GraphQL spec(3.3 Description and 2.9.4 StringValue), a single double quote description cannot include " or \ or LineTerminator, but it isn't considered.

It happened when I tried to combine Hasura to Apollo federation gateway.
The server construction is following.
[Front] Apollo gateway --> Hasura adopter(using apollo fedration)-->Hasura on Docker. [Backend]

When the Hasura adopter starts, it requests introspection to Hasura and executes buildFederatedSchema after Hasura returns the response.
The Hasura introspection response includes following descrption.
input type for inserting array relation for remote table "tableName"

So, I suppose this PR required to combine some existing GraphQL servers to federation gateway.

@apollo-cla
Copy link

@feuxfollets1013: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@jakeblaxon
Copy link
Contributor

jakeblaxon commented Feb 7, 2020

+1 for merging this PR. I ran into this same issue when integrating Federation with Hasura. As a workaround for now I just stripped all block string descriptions from the Hasura schema before federating it, but it would be nice to not have to do this.

For those curious, I managed to strip the block string descriptions in node using printSchema(schema).replace(/"""(.*?(\s)*?(\n)?)*?"""/g, "") and then passing this string in as a typeDef to the buildFederatedSchema method.

@abernix abernix changed the title [Federation] Add support for including excepting SourceCharacter to printDescription feat(federation): Add support for including excepting SourceCharacter to printDescription Apr 22, 2020
@abernix abernix changed the title feat(federation): Add support for including excepting SourceCharacter to printDescription feat: Add support for including excepting SourceCharacter to printDescription Apr 22, 2020
@jbaxleyiii jbaxleyiii requested a review from trevor-scheer June 17, 2020 13:39
@abernix abernix closed this Jun 24, 2020
@abernix abernix reopened this Jun 25, 2020
@abernix abernix changed the base branch from master to main June 25, 2020 09:03
@abernix
Copy link
Member

abernix commented Sep 17, 2020

Hi @feuxfollets1013!,

This is a semi-automated message. Apologies for the lack of personal detail. I’d encourage reading the rest of this message but, if you're busy, hopefully this is as easy for you — or someone who is willing to own the contribution — as pushing the button later in this message to re-create this PR, from an auto-migrated branch, on the new Federation repository. If you have time, read on!

We’re in the process of moving federation-related concerns out of this repository (where it has historically lived) into a new home specifically for Apollo Federation. This PR is affected by the transition since it touches code which has been moved.

I’ve done some preparations to make this as easy as possible, but we’ll need to close this PR, and I could use your help in re-opening it on the new Apollo Federation repository.

In apollographql/federation#134 (which has a lot more details, if you’re interested), we introduced the same code that was in this repository to the new Federation repository (maintaining its history) and removed the code from this repository in #4561.

While this PR still needs to get reviewed and approved to land, it should no longer live in this repository in its current state since it cannot merge in anymore. To facilitate the movement of this PR to the new home, I’ve automatically generated branches on the new repository using the same commit authorship and messages that you originally included on this PR.

Pull-requests can’t be moved on GitHub in the same way Issues can be. While I could just create the PRs from these new branches too, the contribution belongs to you!


Original PR author only: Click this button to open a new PR from the auto-created apollo-server-import/pr-3765 branch on the new Apollo Federation repository

Original PR author: Click here to re-create this PR on the Federation repository

(The code and your commits should be the same and you will have an opportunity to confirm, but this way you can continue to be the author and track its progress on the new Federation repository!)


There’s no easy way to bring over any existing comments on this PR, so I encourage you to copy and paste those onto the new issue if possible.

Overall, I hope this process is relatively easy for you while maintaining your commit contribution authorship. I apologize for any inconvenience caused by this shuffle, but please ping me if I can help in any way!

🚀

@abernix abernix closed this Sep 17, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants