Skip to content

Remove custom directives from Federated GraphQL schema _service{sdl} #374

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 27, 2019
Merged

Remove custom directives from Federated GraphQL schema _service{sdl} #374

merged 6 commits into from
Sep 27, 2019

Conversation

aarestu
Copy link
Contributor

@aarestu aarestu commented Sep 24, 2019 •
edited by dariuszkuc
Loading

📝 Description

Remove custom directive from federated GraphQL schema _service { sdl }.

🔗 Related Issues

#373

@codecov-io
Copy link

codecov-io commented Sep 24, 2019 •
edited
Loading

Codecov Report

Merging #374 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #374      +/-   ##
============================================
+ Coverage     97.61%   97.61%   +<.01%     
  Complexity      318      318              
============================================
  Files           104      104              
  Lines          1215     1217       +2     
  Branches        198      198              
============================================
+ Hits           1186     1188       +2     
  Misses            9        9              
  Partials         20       20
Impacted Files Coverage Δ Complexity Δ
...raphql/federation/FederatedSchemaGeneratorHooks.kt 100% <100%> (ø) 7 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b02bc91...99ab2d5. Read the comment docs.

@dariuszkuc dariuszkuc mentioned this pull request Sep 24, 2019
Closed
@dariuszkuc
Copy link
Collaborator

While we certainly could follow the pattern with manually modifying the federated SDL I personally feel that this is not the best solution. I opened up an issue against apollo-server to see whether they could support proper SDL so we can avoid those transformations.

Lets keep this open for the time being and see whats the proper long term fix.

@aarestu
Copy link
Contributor Author

aarestu commented Sep 24, 2019 •
edited
Loading

I agree.. I personally curious why apollo gateway not support _service{sdl} with all definition information

@aarestu
Copy link
Contributor Author

aarestu commented Sep 25, 2019 •
edited
Loading

[question] what if we use this kind of manually transformation for this version.. while we wait for proper solution.. so we can use federation for current apollo version.
Is any other features will be impacted?

Currently query _service{sdl} being used for gateway only, right?
And not use for internal app / front-end

@aarestu
don't remove deprecated directive from SDL
872bdf6 
update name variables regex
@dariuszkuc dariuszkuc changed the title Remove Declared Directive from _service{sdl} Remove custom directives from _service{sdl} Sep 26, 2019
@dariuszkuc dariuszkuc changed the title Remove custom directives from _service{sdl} Remove custom directives from Federated GraphQL schema _service{sdl} Sep 26, 2019
@smyrick smyrick added changes: patch Changes require a patch version type: bug Something isn't working labels Sep 26, 2019
@smyrick smyrick merged commit af202f0 into ExpediaGroup:master Sep 27, 2019
@smyrick
Copy link
Contributor

smyrick commented Sep 27, 2019

Thank you for the contribution!

@dariuszkuc dariuszkuc mentioned this pull request Sep 27, 2019
Closed
@aarestu aarestu deleted the bug-fix/remove-declared-derective-from-sdl-service branch September 27, 2019 14:34
This was referenced Sep 27, 2019
Merged
Closed
dariuszkuc pushed a commit to dariuszkuc/graphql-kotlin that referenced this pull request Aug 5, 2022
@aarestu @smyrick
Remove custom directives from Federated GraphQL schema _service{sdl} (E…
164b75a 
…xpediaGroup#374)

* Remove Declared Directive from _service{sdl}

* don't remove `deprecated` directive from SDL
update name variables regex

* add todo to simplify code, once this issue is solved: apollographql/apollo-server#3334
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smyrick smyrick smyrick approved these changes

@dariuszkuc dariuszkuc dariuszkuc approved these changes

Assignees
No one assigned
Labels
changes: patch Changes require a patch version type: bug Something isn't working
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aarestu @codecov-io @dariuszkuc @smyrick