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

Add Dynamic Prefix To Cache Key #6428

Conversation

jorgechacons
Copy link

@jorgechacons jorgechacons commented May 12, 2022

Adding hook to pass a function with access to request context to return a dynamic prefix for each key.
This feature allows the developers to pass identifiers to the keys, so it makes it searchable for redis-cli or memcached that use key-based searches.

Adding hook to pass a function with acess to requestContext to return a dynamic prefix for each key
@apollo-cla
Copy link

@jorgechacon04: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@netlify
Copy link

netlify bot commented May 12, 2022

Deploy Preview for apollo-server-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit c29bec4
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/628e68091576fb0008d5b8e4

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 12, 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 c29bec4:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@glasser
Copy link
Member

glasser commented Jun 3, 2022

Would it make more sense to let you override cacheKeyString instead? (Note that this would also subsume the functionality of extraCacheKeyData which could be deprecated.)

@jorgechacons
Copy link
Author

jorgechacons commented Jun 3, 2022

@glasser That sounds good too, however at least for our case extraCacheKeyData is not used, because that will be hashed so it is not searchable for redis

I added a new hook, to be able to share the requestContext to extract data like operationName, etc

@glasser
Copy link
Member

glasser commented Jul 6, 2022

I think I might not have been clear here. I feel like the concept of "dynamic prefix" is too specific. Both "dynamic prefix" and "extra cache key data" are ways to influence the calculation of the cache key, and it would make more sense to just let you define a "calculate the cache key" hook instead of providing more ad hoc tweaks to the existing one. Happy to review and merge a PR that does this more general change (which would also let us deprecate extraCacheKeyData and drop it in AS4) rather than this more specific one.

@glasser
Copy link
Member

glasser commented Oct 19, 2022

We released a similar feature (generateCacheKey) in AS v3.10.0. Thanks for the inspiration!

@glasser glasser closed this Oct 19, 2022
@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.

3 participants