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

Federation 2 supported? #982

Closed
hchockarprasad opened this issue Jul 14, 2022 · 15 comments
Closed

Federation 2 supported? #982

hchockarprasad opened this issue Jul 14, 2022 · 15 comments
Labels
question Further information is requested

Comments

@hchockarprasad
Copy link

Do async-graphql currently support federation 2 schemas?? For using it with new apollo router built with rust instead of nodejs apollo gateway..

@hchockarprasad hchockarprasad added the question Further information is requested label Jul 14, 2022
@peacechen
Copy link

peacechen commented Aug 2, 2022

We're deciding between Rust and Go to implement GraphQL sub-graphs. The Apollo page lists Rust as having very little Fed2 support:
https://www.apollographql.com/docs/federation/supported-subgraphs/#rust

Will this library be updated regularly? Is it production ready? Are there other (preferably official) Rust GraphQL libraries?

Related discussion: #922

@sunli829
Copy link
Collaborator

sunli829 commented Aug 5, 2022

I'll support it when I'm free. 🙂

@peacechen
Copy link

Fair enough. What that means is this project isn't ready for production use.

@leebenson
Copy link
Contributor

Will this library be updated regularly?

I suggest you take a look at the releases, commits and issue board. @sunli829 is generously donating an enormous amount of time to supporting this project, and I for one am very grateful.

Is it production ready?

https://github.com/async-graphql/async-graphql#whos-using-async-graphql-in-production

Fair enough. What that means is this project isn't ready for production use.

That's terribly unfair. Many companies and projects are using it successfully in production. If your particular use-case isn't answered, perhaps you could consider sponsoring the project or contributing to it?

@raptros
Copy link
Contributor

raptros commented Aug 18, 2022

I am not very familiar with the async-graphql codebase, but I do want to contribute to the community in any way I can here. I think it'd be helpful if we at least have ideas written down about how to implement each of the remaining pieces.

looking over the apollo compat list, i think the things we need still are:

  • @link directive
  • @tag directive
  • @override directive
  • plus ftv1 might be a "nice-to-have"

the override directive should be pretty straightforward since it only applies to fields; the changes that have been made so far should provide a good example for how to proceed.

I managed to find a definition of the tag directive - it's documented here: https://specs.apollo.dev/tag/v0.2/; i think it should be pretty similar to existing directive support as well.

ftv1 is almost straightforward as per the apollo documentation: https://www.apollographql.com/docs/federation/metrics/#how-tracing-data-is-exposed-from-a-subgraph
it just requires looking for a header in the http request, and then computing the value to put in the response extensions. does anyone have any thoughts about how to do this, given the various web frameworks async-graphql integrates with?

the really tricky one is @link. I think there are several challenges here based on what I've seen from Apollo about its use, which seem to require totally new features in async-graphql.

  1. we need a way to have schema-level directives.
  2. the really tricky part is that it can import totally new schema directive definitions into the subgraph. so we need a way to add custom schema directives all over the place.
  3. there's also this extend schema syntax associated with it (as seen here https://specs.apollo.dev/link/v1.0/). the subgraph spec documentation for it is maybe easier to understand: https://www.apollographql.com/docs/federation/federation-spec/#link

@dariuszkuc
Copy link

re: @link directive support - while the full functionality is pretty complex as it allows for namespacing and renaming of the imported definitions, due to the flexible type merging that is available in Federation v2, router should be able to compose subgraph schemas even if subset of functionality is provided.

extend schema syntax is not required as directive can be applied directly on the schema type. At a minimum you just need to import the spec (but as a side effect all federation directives will have to be namespaced with federation__ prefix).

schema @link(url : "https://www.apollographql.com/docs/federation/federation-spec/"){
  query: Query
}

directive @link(url: String!) repeatable on SCHEMA
directive @federation__key(fields: federation__FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar federation__FieldSet

NOTE: @tag and @inaccessible currently cannot be renamed/namespaced, attempting to do so will led to composition errors

Alternatively, if you import those directive definitions (even without supporting renaming) then you don't need to namespace them anymore, e.g.

schema @link(url : "https://www.apollographql.com/docs/federation/federation-spec/", import : ["key", "FieldSet"]){
  query: Query
}

directive @link(url: String!, import: [String]) repeatable on SCHEMA
directive @key(fields: FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar FieldSet

I am not familiar with async-graphql so unsure if this is possible but within graphql-java world you could use schema transformer to rename the definitions (so in the code there is only a single definition).

@peacechen
Copy link

@leebenson

That's terribly unfair. Many companies and projects are using it successfully in production. If your particular use-case isn't answered, perhaps you could consider sponsoring the project or contributing to it?

It was harsh I admit. I appreciate the time that @sunli829 has contributed to this project. I also maintain open source projects and it's a thankless job. Kudos to sunli, I meant no offense to him and others who've contributed to this project.

My response was directly to the comment "I'll support it when I'm free". Having used numerous open source libraries, it's painful when a bug is discovered, a fix provided, but the maintainer goes AWOL. There are open PRs that are months and even years old. Yes I could fork this, but that introduces all sorts of downstream problems.

@raptros
Copy link
Contributor

raptros commented Aug 19, 2022

I have taken a little time to try to implement the override directive ... currently trying to solve some mystifying build issues locally. If I can figure it out I might put up a change over the weekend.

ftv1 (subgraph) tracing ... should that be a separate crate, maybe? I've seen that there's already a separate crate for publishing traces to apollo studio directly, maybe the subgraph tracing support should be implemented similarly (if not in the same repo)?

@dariuszkuc if you have a chance - re

extend schema syntax is not required as directive can be applied directly on the schema type.

Would you mind elaborating on the distinction (if there is any)? How does federation treat the two differently?

I see that your example uses @federation__key, which worries me a little - async-graphql currently supports printing the @key directive when instructed in that form. This means supporting this link directive needs to be done to prevent this namespacing.

Either way, the issue is figuring out how to configure it for printing. It only needs to be output as part of the federation schema, right?

Maybe it could be added as an optional field on the Schema struct, so that it can be output when printing in federation mode.

within graphql-java world you could use schema transformer to rename the definitions (so in the code there is only a single definition).

i am pretty sure async-graphql does not have anything like this - it appears to be very straightforward in printing the definitions registered in the schema.

Overall I still consider link to be the biggest challenge - i.e.

router should be able to compose subgraph schemas even if subset of functionality is provided.

is not actually encouraging given that the subset of functionality that is needed provides some challenges.

@dariuszkuc
Copy link

dariuszkuc commented Aug 23, 2022

extend schema syntax is not required as directive can be applied directly on the schema type.

Would you mind elaborating on the distinction (if there is any)? How does federation treat the two differently?

From federation perspective there is no difference as either one (extend schema @link("foo") or (schema @link("foo")) is valid from composition perspective. There is a small difference from the subgraph perspective as if you apply directive directly on the schema type then you also need to include default Query types, e.g.

# using extension you can omit defaults
extend schema @link("foo")

# applying directive on schema directly requires specifying default types
schema @link("foo") {
  query: Query 
}

@dariuszkuc
Copy link

dariuszkuc commented Aug 23, 2022

In regards to implementing @link directive support, there are 4 use cases

1. Link to spec only

schema @link(url : "https://specs.apollo.dev/federation/v2.0"){
  query : Query
}

When we link to the target spec, we have to explicitly namespace* federation directives with federation__

directive @link(url: String!) repeatable on SCHEMA
directive @federation__key(fields: federation__FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar federation__FieldSet

Simplest way to support it, is to define your directives as federation__<directive> so there is no need for renames (you may want to have separate definitions for v1 which does not support namespacing and v2 that requires namespacing).

*NOTE: @tag and @inaccessible currently cannot be renamed/namespaced, attempting to do so will led to composition errors

2. Link to spec and specify custom namespace

schema @link(url : "https://specs.apollo.dev/federation/v2.0", as: "fed"){
  query : Query
}

When using custom namespace you have to explicitly namespace federation types (except for @tag and @inaccessible) with specified namespace.

directive @link(url: String!, as: String) repeatable on SCHEMA
directive @fed__key(fields: fed__FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar fed__FieldSet

IMHO this is somewhat complex to support as it requires dynamic renaming of the types based on the user input.

3. Explicitly import federated types

schema @link(url : "https://specs.apollo.dev/federation/v2.0", import : ["@key", "FieldSet"]) {
  query: Query
}

When explicitly importing types you don't need to rename/namespace any of the underlying types.

directive @link(url: String!, import: [String]) repeatable on SCHEMA
directive @key(fields: FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar FieldSet

IMHO this is arguably the easiest to implement as we don't need to rename/namespace any of the underlying types. There is a small drawback that there is a small chance that imported types could conflict with the types defined locally (guessing it generally shouldn't be common).

4. Explicitly import federated types and rename them to avoid collisions

schema @link(url : "https://specs.apollo.dev/federation/v2.0", import : [ { name: "@key", as: "@mykey" }, "FieldSet"]) {
  query: Query
}

directive @link(url: String!, import: [link_Import]) repeatable on SCHEMA
directive @mykey(fields: FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE
scalar FieldSet
# note that Import scalar is imported from different spec, hence the namespace, Import can be either a string or `name/as` value pair.
scalar link__Import

Similarly to custom namespacing, IMHO this is pretty complex to support as it requires dynamically renaming types based on user input.


Full link support would imply automatically applying namespacing (possibly custom) for non-imported types and providing ability to rename the types to avoid the collision. Minimal functionality to support is option 1 above or option 3.

@raptros
Copy link
Contributor

raptros commented Sep 19, 2022

just marked my PR ready for review - #1060

it's a simple approach to implementing the link directive that relies on the fact that async-graphql does not presently have any custom schema directive support. (if` in the future such support is added, that addition will need to entirely replace what i've done.)

@dariuszkuc
Copy link

Support was released in v4.0.14 🎉

@dbanty
Copy link
Member

dbanty commented Sep 27, 2022

I think this can be closed now that the fed2 compatibility is all "green" in the test matrix. I think federated tracing can be a separate feature in the future if folks are looking for that, specifically.

@dbanty dbanty closed this as completed Sep 27, 2022
@raptros
Copy link
Contributor

raptros commented Sep 27, 2022

that makes sense to me @dbanty, especially since federated tracing is probably something that should be implemented as a separate crate.

@dbanty
Copy link
Member

dbanty commented Sep 27, 2022

Thanks for pushing this over the finish line @raptros !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants