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

provide rhai surrogate cache key example #1136

Merged
merged 9 commits into from May 25, 2022
Merged

Conversation

garypen
Copy link
Contributor

@garypen garypen commented May 24, 2022

This provides an example of reading subgraph header responses and creating a composite surrogate key.

The approach taken is to override:

  • subgraph service to read the responses for each subgraph and store them in a shared object map within the response context.
  • router_service to check the context for a surrogate key and add to response headers if present

This approach makes use of the new Context::upsert() function which is added to the rhai implementation.

Note: The rhai dependency is bound to a specific git rev since we require a serialization fix in rhai (provided by @Geal ) and which hasn't been released yet.

fixes: #648

What is causing the de-serialization error?
To pull in Geoffroy's rhai contribution for serde.

Clean up the example and think about the best way to expose upsert to
rhai.
Fix the implementation of upsert() and update the example to use the
new upsert().
Until the next release of rhai. Needed to pull in the fix for map
serialization.
@garypen garypen self-assigned this May 24, 2022
@netlify
Copy link

netlify bot commented May 24, 2022

Deploy Preview for apollo-router-docs ready!

Name Link
🔨 Latest commit ac1cc18
🔍 Latest deploy log https://app.netlify.com/sites/apollo-router-docs/deploys/628d029a79d4e1000847ffa3
😎 Deploy Preview https://deploy-preview-1136--apollo-router-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions

This comment has been minimized.

 - Added feature line to changelog
 - (clean up dead commented out line in Cargo.toml)
 - Document new Context::upsert() support in rhai
 - Document new surrogate cache key example
let map_callback = |response| {
try {
// IMPORTANT: Take a copy of the cache-key here
// to avoid deadlock in the closure.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the documentation should have a section about that deadlock risk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a general rhai risk rather than anything specific about how we use it. I will think about the best way to incorporate something that references this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a section to the docs: 3f08c90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Geal Could you take a look at the changes I added?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

Try to call out some of the dangers of deadlock with shared values.
@garypen garypen requested a review from Geal May 25, 2022 07:10
@garypen garypen merged commit a2c4123 into main May 25, 2022
@garypen garypen deleted the garypen/648-surrogate branch May 25, 2022 14:27
@garypen garypen mentioned this pull request May 31, 2022
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.

examples(rhai): Building surrogate cache keys
3 participants