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

Rename property for RESTDataSource #6834

Merged
merged 3 commits into from Aug 19, 2022

Conversation

smyrick
Copy link
Member

@smyrick smyrick commented Aug 19, 2022

Rename property for RESTDataSource from requestCacheEnabled to memoizeGetRequests

From the initial PR, the name of the new property was a little confusing since there are actually two caches, but what is being cached is the response data in the end

Original PR #6650

@netlify
Copy link

netlify bot commented Aug 19, 2022

Deploy Preview for apollo-server-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 4671b57
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/63001907a08cda0009d5955d

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 19, 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 4671b57:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

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

revert CHANGELOG and I'll merge.

Your previous PR was a great step in the right direction of actually explaining what RESTDataSource does in its doc page. I know @rkoron007 will be looking at that page soon and perhaps that can be an opportunity to add an overall explanation of the caching behavior and expected lifecycle on the page (the general concept of “create a RESTDataSource once per request; all GET requests are memoized inside the data source regardless of HTTP cache control headers in the response; also the shared cache is used to cache anything with HTTP cache control headers in a way that honors the headers”). That the page doesn't mention anywhere that each RDS object is per request is pretty silly.

CHANGELOG.md Outdated Show resolved Hide resolved
@glasser glasser merged commit 8cc8437 into apollographql:main Aug 19, 2022
@smyrick smyrick deleted the rename-rest-datasource-property branch August 22, 2022 19:19
trevor-scheer pushed a commit to apollographql/datasource-rest that referenced this pull request Aug 22, 2022
Rename property for RESTDataSource from requestCacheEnabled to memoizeGetRequests

From the initial PR, the name of the new property was a little confusing since there are actually 
two caches, but what is being cached is the response data in the end.

Original PR #3
Back-port of apollographql/apollo-server#6834
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants