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

Changes to sdl field of _service object to make it compliant with the… #304

Merged
merged 4 commits into from
Aug 19, 2019

Conversation

tobias-f
Copy link
Contributor

@tobias-f tobias-f commented Aug 16, 2019

… federation specification

  • if the Query type is present, it should have the @extends directive, since it extends the query type
  • the default schema definition should not be includeded in the sdl. This causes problems for example with the @apollo/gateway, because if the default schema definition is present, no queries are picked up from the federated schema.

… federation specification

- if the Query type is present, it should have the @extends directive, since it extends the query type
- the default schema definition should not be includeded in the sdl
@codecov
Copy link

codecov bot commented Aug 16, 2019

Codecov Report

Merging #304 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #304      +/-   ##
============================================
+ Coverage     94.42%   94.44%   +0.01%     
  Complexity      256      256              
============================================
  Files            74       74              
  Lines           915      918       +3     
  Branches        169      169              
============================================
+ Hits            864      867       +3     
  Misses           31       31              
  Partials         20       20
Impacted Files Coverage Δ Complexity Δ
...raphql/federation/FederatedSchemaGeneratorHooks.kt 97.82% <100%> (+0.15%) 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 5812fa0...8e2b80c. Read the comment docs.

@dariuszkuc
Copy link
Collaborator

  • if the Query type is present, it should have the @extends directive, since it extends the query type

if all Query types are extended then which service defines the base?

  • the default schema definition should not be included in the sdl. This causes problems for example with the @apollo/gateway, because if the default schema definition is present, no queries are picked up from the federated schema.

Sounds like a bug in @apollo/gateway - @pcarrier can you take a look?

@dariuszkuc
Copy link
Collaborator

Documenting for completeness

  • if the Query type is present, it should have the @extends directive, since it extends the query type
    if all Query types are extended then which service defines the base?

Apollo gateway defines the base Query type. Each service that is part of the federated schema should be extending the Query type to signal that its queries are part of a larger federated schema.

  • the default schema definition should not be included in the sdl. This causes problems for example with the @apollo/gateway, because if the default schema definition is present, no queries are picked up from the federated schema.

Sounds like a bug in @apollo/gateway - @pcarrier can you take a look?

This still does sound like a bug but indeed there is no need to include those defaults.

- removed empty queries from SDL
- uses original query name for adding @extends directive
@dariuszkuc dariuszkuc merged commit 73b046f into ExpediaGroup:master Aug 19, 2019
@dariuszkuc
Copy link
Collaborator

Thanks for the fix!

@smyrick smyrick added changes: patch Changes require a patch version type: bug Something isn't working labels Aug 27, 2019
smyrick pushed a commit to smyrick/graphql-kotlin that referenced this pull request Sep 11, 2019
ExpediaGroup#304)

* Changes to sdl field of _service object to make it compliant with the federation specification

- if the Query type is present, it should have the @extends directive, since it extends the query type
- the default schema definition should not be included in the sdl
- empty queries should not be present in the SDL
dariuszkuc pushed a commit to dariuszkuc/graphql-kotlin that referenced this pull request Aug 5, 2022
ExpediaGroup#304)

* Changes to sdl field of _service object to make it compliant with the federation specification

- if the Query type is present, it should have the @extends directive, since it extends the query type
- the default schema definition should not be included in the sdl
- empty queries should not be present in the SDL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes: patch Changes require a patch version type: bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants