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

Fix _service { sdl } render to hide federated fields #21

Open
kdawgwilk opened this issue Sep 23, 2021 · 10 comments · Fixed by #22 or #42
Open

Fix _service { sdl } render to hide federated fields #21

kdawgwilk opened this issue Sep 23, 2021 · 10 comments · Fixed by #22 or #42
Assignees
Labels
bug Something isn't working

Comments

@kdawgwilk
Copy link
Collaborator

While implementing this PR apollographql/apollo-federation-subgraph-compatibility#35 I found we are not filtering federated schema fields from the SDL render like we should

@kdawgwilk kdawgwilk added the bug Something isn't working label Sep 23, 2021
@kdawgwilk kdawgwilk self-assigned this Sep 23, 2021
@kdawgwilk
Copy link
Collaborator Author

This was reverted in #29 and we need to look at taking a different approach

@kzlsakal
Copy link
Collaborator

Reopening this since #42 was reverted by #48

@kzlsakal kzlsakal reopened this Apr 14, 2022
@lorgio-jimenez
Copy link

lorgio-jimenez commented May 13, 2022

Is there a version of the library that does render this properly? We're switching over from using a mesh server to using this library, and this is a serious blocker for us to deploy with.

@kzlsakal
Copy link
Collaborator

Is there a version of the library that does render this properly?

The SDL rendered with v0.2.4 is compliant with Gateway 2 and it works without any problems. We currently use it in production.

The issue is about full compliance with Apollo Federation specs. Rendering these extra fields in the SDL doesn't affect the functionality as far as I can tell.

To answer your question though, unfortunately, we do not have a version where this issue is fixed without introducing bugs like #47 or #28.

@kzlsakal
Copy link
Collaborator

While looking at this, I noticed that there might have been an update on Apollo Federation docs. It mentions:

Unlike introspection, the sdl field's returned string includes federation-specific directives like @key.

https://www.apollographql.com/docs/federation/subgraphs/#query_service

This may be implying that we don't need to omit federation-specific fields from the SDL render after all. Any thoughts @kdawgwilk?

@kdawgwilk
Copy link
Collaborator Author

You are supposed to include the directives themselves on each field but it's the federated type definitions that are not recommended to be present eg union _Entity or directive definitions. Absinthe does a few optimizations like eagerly stripping out types that are not referenced in any accessible way in the schema which has been causing us problems. We have a pretty good trade off in place right now that should work great. What issues are you running into?

@kroehre
Copy link

kroehre commented May 27, 2022

_resolveReference fields are appearing in the schema also

@kdawgwilk
Copy link
Collaborator Author

@kzlsakal we could prob strip out those _resolveReference fields no matter what even if there is no _Entity type

@kdawgwilk
Copy link
Collaborator Author

This PR will strip out the _resolveReference fields from the SDL render #55 which helps to partially resolve this issue

@dariuszkuc
Copy link

FYI federation v1 required SDL modifications to strip out the fed definitions but fed v2 accepts full schemas -> there is no need to filter anything so you could just print out whole schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
5 participants