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

gateway: remove dependency on apollo-server-caching #2029

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented Aug 2, 2022

The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by @apollo/utils.keyvaluecache), so let's do
it.

While we're at it, we make a few other improvements:

  • Ever since chore(gateway): Simplify startup code paths #440, the queryPlanStore field is always set, so we can
    remove some conditionals around it.
  • Instead of using the old lru-cache@6 wrapped by the
    apollo-server-caching package, we use the newer lru-cache@7 (which
    improves the algorithm internally and changes the names of methods a
    bit).
  • The get and set methods on InMemoryLRUCache were only async because
    they implement the abstract KeyValueCache interface: the
    implementations didn't actually do anything async. So we no longer
    need to await them or include a giant comment about how we're not
    awaiting them.

The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by `@apollo/utils.keyvaluecache`), so let's do
it.

While we're at it, we make a few other improvements:
- Ever since #440, the queryPlanStore field is always set, so we can
  remove some conditionals around it.
- Instead of using the old lru-cache@6 wrapped by the
  apollo-server-caching package, we use the newer lru-cache@7 (which
  improves the algorithm internally and changes the names of methods a
  bit).
- The get and set methods on InMemoryLRUCache were only async because
  they implement the abstract KeyValueCache interface: the
  implementations didn't actually do anything async. So we no longer
  need to await them or include a giant comment about how we're not
  awaiting them.
@netlify
Copy link

netlify bot commented Aug 2, 2022

Deploy Preview for apollo-federation-docs canceled.

Name Link
🔨 Latest commit 6d6e533
🔍 Latest deploy log https://app.netlify.com/sites/apollo-federation-docs/deploys/62e8801bd027b30008ccefc9

@glasser
Copy link
Member Author

glasser commented Aug 2, 2022

cc @trevor-scheer

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 2, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

@benweatherman benweatherman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐐

@glasser glasser merged commit 6bfefb7 into main Aug 2, 2022
@glasser glasser deleted the glasser/no-apollo-server-caching branch August 2, 2022 04:40
glasser added a commit that referenced this pull request Aug 2, 2022
The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by `@apollo/utils.keyvaluecache`), so let's do
it.

While we're at it, we make a few other improvements:
- Ever since #440, the queryPlanStore field is always set, so we can
  remove some conditionals around it.
- Instead of using the old lru-cache@6 wrapped by the
  apollo-server-caching package, we use the newer lru-cache@7 (which
  improves the algorithm internally and changes the names of methods a
  bit).
- The get and set methods on InMemoryLRUCache were only async because
  they implement the abstract KeyValueCache interface: the
  implementations didn't actually do anything async. So we no longer
  need to await them or include a giant comment about how we're not
  awaiting them.
glasser added a commit that referenced this pull request Aug 2, 2022
* gateway: remove dependency on apollo-server-caching (#2029)

The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by `@apollo/utils.keyvaluecache`), so let's do
it.

While we're at it, we make a few other improvements:
- Ever since #440, the queryPlanStore field is always set, so we can
  remove some conditionals around it.
- Instead of using the old lru-cache@6 wrapped by the
  apollo-server-caching package, we use the newer lru-cache@7 (which
  improves the algorithm internally and changes the names of methods a
  bit).
- The get and set methods on InMemoryLRUCache were only async because
  they implement the abstract KeyValueCache interface: the
  implementations didn't actually do anything async. So we no longer
  need to await them or include a giant comment about how we're not
  awaiting them.

* gateway RemoteGraphQLDataSource: throw GraphQLError, not ApolloError (#2028)

This is part of
apollographql/apollo-server#6057 (which is
itself part of
apollographql/apollo-server#6719). We are
trying to break the dependency of Gateway on Server so that (among other
things) it is easier to have a single version of Gateway that works with
both the current AS3 and the upcoming AS4.

In AS4, we are removing the ApolloError class and its subclasses.
Instead, we will just use GraphQLError directly. See:

https://www.apollographql.com/docs/apollo-server/v4/migration#apolloerror
https://www.apollographql.com/docs/apollo-server/v4/migration#built-in-error-classes
apollographql/apollo-server#6355
apollographql/apollo-server#6705

This commit changes RemoteGraphQLDataSource to throw GraphQLError
instead of ApolloError. The `code` extension will still be the same.
(The `name` field of the thrown Error will no longer be eg
`AuthenticationError`, though; this does not affect the error as
serialized in GraphQL.)

This is technically slightly backwards-incompatible (eg, the method
errorFromResponse is public and now returns GraphQLError instead of the
tighter ApolloError) but this doesn't seem likely to affect many users.
We can adjust based on feedback if necessary.

* Adjust #2028 for graphql@15.8 compatibility
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

Successfully merging this pull request may close these issues.

2 participants