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

[apollo-datasource-rest] Add option to disable GET cache #6650



Copy link

@smyrick smyrick commented Jul 7, 2022

Fixes #6603

By default, RESTDataSource caches all outgoing GET requests in a separate memoized cache from the regular response cache. It makes the assumption that all responses from HTTP GET calls are cacheable by their URL.
If a request is made with the same cache key (URL by default) but with an HTTP method other than GET, the cached request is then cleared.

This change adds a new class property requestCacheEnabled (which defaults to true to match current behavior) which allows users to disable the cache. You might want to do this if your API is not actually cacheable or your data changes over time.

Separately it documents this feature exists and adds more info about how to set a TTL for the entire HTTP cache

Copy link

changeset-bot bot commented Jul 7, 2022

🦋 Changeset detected

Latest commit: 7bc65a7

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Jul 7, 2022

Deploy Preview for apollo-server-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 0134a13
🔍 Latest deploy log

Copy link

codesandbox-ci bot commented Jul 7, 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.

Latest deployment of this branch, based on commit 0134a13:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@smyrick smyrick force-pushed the datasource-rest-get-cache-disable branch from 1850f12 to 8562674 Compare July 15, 2022 17:35
Copy link
Member Author

smyrick commented Jul 15, 2022

Part of the issue was also that this feature was undocumented so I have added some basic info about the RESTDatasource properties and methods. I would prefer to review the code first and get iterative changes into the docs as needed.

@smyrick smyrick marked this pull request as ready for review July 15, 2022 18:38
Copy link
Member Author

smyrick commented Jul 15, 2022

cc @Jaxolotl

@smyrick smyrick changed the title Add option to disable RESTDatasource GET cache [apollo-datasource-rest] Add option to disable GET cache Jul 15, 2022
Copy link
Member Author

smyrick commented Aug 19, 2022

cc @trevor-scheer and @glasser.

I can port my changes over to the new package, but this change is still relevant to anyone using ASv3 today or whoever will in the future with respect to the doc changes. Can I get a review for this one?

Copy link

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

Can you please add why someone would want this behavior to the PR description? Totally fine if it's the same blurb that I asked to be added to the docs, just want people to understand why they'd disable it.

We can delete the changeset, and add an entry to CHANGELOG - sorry, we had the bot enabled for v4 development (glasser's since disabled it) which made it confusing for dev on main since we're not using it for v3 things.

@smyrick smyrick force-pushed the datasource-rest-get-cache-disable branch from c73b57f to 6768111 Compare August 19, 2022 21:42
@trevor-scheer trevor-scheer merged commit 3b017c6 into apollographql:main Aug 19, 2022
17 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
None yet

Successfully merging this pull request may close these issues.

Undocumented and non-configurable memoization feature on apollo-datasource-rest
2 participants