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

passing @federation__shareable to type PageInfo #207

Open
matoni109 opened this issue Aug 24, 2022 · 17 comments
Open

passing @federation__shareable to type PageInfo #207

matoni109 opened this issue Aug 24, 2022 · 17 comments

Comments

@matoni109
Copy link

matoni109 commented Aug 24, 2022

Hello Everyone 😀,

We are using type PageInfo in two subgraphs using federation 2.0

  • Currently we are getting the below error when trying to use connection_type
  • I've tried passing shareable: true also.
  • Any ideas ?
# query_type.rb
#... 
  field :users, UserType.connection_type, 'List all users.'  #, shareable: true

    def users
      User.all
    end
INVALID_FIELD_SHARING

Non-shareable field "PageInfo.endCursor" is resolved from multiple subgraphs: it is resolved from subgraphs "channels" and "users" and defined as non-shareable in all of them

INVALID_FIELD_SHARING

Non-shareable field "PageInfo.hasNextPage" is resolved from multiple subgraphs: it is resolved from subgraphs "channels" and "users" and defined as non-shareable in all of them
# base_connection.rb
module Types
  class BaseConnection < Types::BaseObject
    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors
  end
end
@abachman
Copy link

We faced this issue too and it was tricky to solve. We ended up monkeypatching GraphQL::Types::Relay::PageInfo like this:

# extending the Graphql Relay PageInfo class with federation abilities
class GraphQL::Types::Relay::PageInfo
  include ApolloFederation::Object
  include ApolloFederation::Field

  shareable if federation_2_condition?
end

and then requiring that file manually from our schema.rb file.

We had additional logic to decide if shareable should be called based on what federation version this subgraph was running. We're supporting something like 25 subgraphs, so not all of them could go to Federation 2 at the same time.

@geshwho
Copy link
Collaborator

geshwho commented Aug 24, 2022

Hi @matoni109, Thank you for submitting this. I'll take a closer look at it within the next day or so.

@matoni109
Copy link
Author

Thanks @abachman & @geshwho,

If we get something up and running here I'll paste it into this thread

@geshwho
Copy link
Collaborator

geshwho commented Aug 30, 2022

Sorry for the delay.. To me, this feels like something that the library should handle for you since connection_type is a graphql-ruby feature. I'm going to chat with some folks and see how we want to handle it.

@matoni109
Copy link
Author

@geshwho our team came up with below in the mean time if that helps anyone else :

# app/graphql/types/base_connection.rb
module Types
  class BaseConnection < Types::BaseObject

    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors

    own_fields.delete("pageInfo")

    field :page_info, Types::PageInfo, 'Information to aid in pagination.', null: false
  end
end
# app/graphql/types/page_info.rb

module Types
  class PageInfo < Types::BaseObject
    shareable
    include GraphQL::Types::Relay::DefaultRelay

    field :has_next_page, Boolean, null: false,
      description: "When paginating forwards, are there more items?"

    field :has_previous_page, Boolean, null: false,
      description: "When paginating backwards, are there more items?"

    field :start_cursor, String, null: true,
      description: "When paginating backwards, the cursor to continue."

    field :end_cursor, String, null: true,
      description: "When paginating forwards, the cursor to continue."
  end
end

@smyrick
Copy link

smyrick commented Sep 13, 2022

Hello, Shane from Apollo here. I am not familiar with the internals of this library but the original error is an issue where you might be running a combination of Fed 1 + Fed 2 or you are not using Fed 2 with the correct directives.

The short explanation is that in Fed 1 everything was considered "shareable" by default, but in Fed 2 it is now an opt-in model with the new @shareable directive.

https://www.apollographql.com/docs/federation/federation-2/new-in-federation-2#value-types

The quick fix is to add the @shareable directive on all subgraphs which will resolve the error, but this would require that all the subgraphs can also use Fed 2.

Because of this change and to remain backward compatible, if you are running Apollo Gateway v2, any Fed 1 subgraphs are auto-updated to add @shareable to their types, which will allow you to incremental update to Fed 2. There was a small bug around this so you might want to make sure you are running the latest versions of the gateway if you want to run in this mixed mode.

@eliocapelati
Copy link

Hello 👋 ,

We are also having this same problem. We have tested the approaches shared above and still can't find a workaround that works.

I'm commenting here to receive updates from this thread.

@matoni109
Copy link
Author

Hello 👋 ,

We are also having this same problem. We have tested the approaches shared above and still can't find a workaround that works.

I'm commenting here to receive updates from this thread.

@eliocapelati we have above working in staging happy to work through it if you have a sample repo to play with

@matoni109
Copy link
Author

@geshwho i'm also in the mix of working on a PR of a fed2 version of the libs example which is using fed1

@skukx
Copy link

skukx commented Oct 13, 2022

Many have already pointed out the workaround already. Here is a quick two liner for those curious. (Does the same thing as other solutions posted).

GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
GraphQL::Types::Relay::PageInfo.shareable

Note: This must be done in all subgraphs for it to fully work

@matoni109
Copy link
Author

thanks @skukx :) below for those wondering where to put the fix ( that might have just been me .. )

# app/graphql/types/base_connection.rb

module Types
  class BaseConnection < Types::BaseObject

    # add `nodes` and `pageInfo` fields, as well as `edge_type(...)` and `node_nullable(...)` overrides
    include GraphQL::Types::Relay::ConnectionBehaviors
    GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
    GraphQL::Types::Relay::PageInfo.shareable

  end
end

@skukx
Copy link

skukx commented Oct 13, 2022

You could technically place those lines anywhere. I chose to place it at the top of my graphql_schema.rb. You could also place it in an initializer as well.

@matoni109
Copy link
Author

You could technically place those lines anywhere. I chose to place it at the top of my graphql_schema.rb. You could also place it in an initializer as well.

Thanks @skukx I'll put them there :)

@rosscooperman
Copy link

@geshwho Any thoughts on a library-level fix and how to go about doing it? I'd be happy to contribute a PR but you mentioned giving it some thought awhile back so figured I should ask before I bumble on in 😄. Admittedly I haven't gone that deep at this point so maybe the answer will be self-evident if I do.

@grxy
Copy link
Collaborator

grxy commented Mar 29, 2023

Hey @rosscooperman sorry for that lack of response. A PR would be welcome if you have the space to put one together so we can take a look.

@ikuto0608
Copy link

Does anyone have the same bahavior that I have here?

GraphQL::Types::Relay::PageInfo.include ApolloFederation::Object
GraphQL::Types::Relay::PageInfo.shareable

when I define the pagination shareable directive in my schema file, this works great but if I change the file without restarting rails server, I got this error on my Apollo Gateway below

[1] Error: A valid schema couldn't be composed. The following composition errors were found:
[1] 	[SUBGRAPH NAME] The directive "@federation__shareable" can only be used once at this location.
[1]     at IntrospectAndCompose.createSupergraphFromSubgraphList (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:73:19)
[1]     at IntrospectAndCompose.updateSupergraphSdl (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:65:36)
[1]     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
[1]     at async IntrospectAndCompose.initialize (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/supergraphManagers/IntrospectAndCompose/index.js:28:36)
[1]     at async ApolloGateway.initializeSupergraphManager (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/index.js:299:28)
[1]     at async ApolloGateway.load (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/gateway/dist/index.js:236:13)
[1]     at async SchemaManager.start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/utils/schemaManager.js:37:28)
[1]     at async ApolloServer._start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/ApolloServer.js:163:30)
[1]     at async ApolloServer.start (/Users/ikuto.yata/Documents/smarthr/edp/node_modules/@apollo/server/dist/cjs/ApolloServer.js:141:16)

If I look at what my Apollo Gateway got the schema to compose,

"""
Information about pagination in a connection.
"""
type PageInfo @federation__shareable @federation__shareable {

Duplicated @federation__shareable causing this behavior. Although this is not gonna happen after I restart rails server.
I was wondering whether this behavior causes some other issue and also someone knows a work around 🤔

Thanks!

@smyrick
Copy link

smyrick commented Apr 13, 2023

@ikuto0608

There was a small bug around this so you might want to make sure you are running the latest versions of the gateway if you want to run in this mixed mode.

#207 (comment)

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

No branches or pull requests

9 participants