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

feat: edge gateway extend cdn resolution with r2 #61

Merged
merged 2 commits into from May 20, 2022

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Apr 26, 2022

Adds R2 as a first layer resolution (in CDN together with cache API). It also adds better header handling for previous convs

image

Closes #60

@vasco-santos vasco-santos force-pushed the feat/edge-gateway-cdn-resolution-layer-with-r2 branch from 5c1280c to 3e95587 Compare April 26, 2022 12:40
@cloudflare-pages
Copy link

cloudflare-pages bot commented Apr 26, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5fd3f6d
Status: ✅  Deploy successful!
Preview URL: https://0b0af933.nftstorage-link.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the feat/edge-gateway-cdn-resolution-layer-with-r2 branch 11 times, most recently from 93a9bb7 to 311fb83 Compare April 29, 2022 19:38
@vasco-santos vasco-santos marked this pull request as ready for review April 29, 2022 19:46
@vasco-santos vasco-santos force-pushed the feat/edge-gateway-cdn-resolution-layer-with-r2 branch from 311fb83 to 9b4bada Compare May 19, 2022 12:56
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

As we discussed, this is fine for now but the gateway should be asking the perma-cache API for the content, not reaching round behind it and grabbing the content from it's bucket.

#101

@@ -231,6 +232,36 @@ async function settleGatewayRequests(
])
}

/*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/*
/**

Comment on lines 241 to 247
* @param {string} cacheControl
*/
async function cdnResolution(request, env, cache, cacheControl) {
// Should skip cache if instructed by headers
if (cacheControl.includes('no-cache')) {
return undefined
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param {string} cacheControl
*/
async function cdnResolution(request, env, cache, cacheControl) {
// Should skip cache if instructed by headers
if (cacheControl.includes('no-cache')) {
return undefined
}
*/
async function cdnResolution(request, env, cache) {
// Should skip cache if instructed by headers
if ((request.headers.get('Cache-Control') || '').includes('no-cache')) {
return undefined
}

@vasco-santos vasco-santos merged commit fb78202 into main May 20, 2022
@vasco-santos vasco-santos deleted the feat/edge-gateway-cdn-resolution-layer-with-r2 branch May 20, 2022 14:13
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.

Extend edge gateway CDN layer resolution with R2
2 participants