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: api get from bucket #140

Merged
merged 2 commits into from Jul 6, 2022
Merged

feat: api get from bucket #140

merged 2 commits into from Jul 6, 2022

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Jun 3, 2022

This PR adds:

  • GET /perma-cache/:url endpoint added to api.nftstorage.link to be used by worker binding from gateway to API, in order to read from perma cache
  • worker bindings from Gateway to API

Other notes:

  • Also small utilities to handle URLs were moved to their own file for better code organization
  • Previously perma-cache/get.js had handler for GET perma cache list, but given we actually are adding a GET endpoint to get perma-cache content, it does not make sense anymore. Code there moved to list.js file and new handler code is in get.js

Needs:

Closes #101

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 3, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: c8819bb
Status: ✅  Deploy successful!
Preview URL: https://74336d64.nftstorage-link.pages.dev
Branch Preview URL: https://feat-api-get-from-bucket.nftstorage-link.pages.dev

View logs

@vasco-santos vasco-santos force-pushed the feat/api-get-from-bucket branch 5 times, most recently from 55dd335 to 442f822 Compare June 3, 2022 16:31
@vasco-santos vasco-santos mentioned this pull request Jun 3, 2022
2 tasks
@vasco-santos vasco-santos force-pushed the feat/api-get-from-bucket branch 2 times, most recently from ecbffd3 to 77fe24b Compare June 30, 2022 14:16
@vasco-santos vasco-santos marked this pull request as ready for review June 30, 2022 14:19
@vasco-santos vasco-santos force-pushed the feat/api-get-from-bucket branch 4 times, most recently from 5c62b34 to d098a81 Compare July 1, 2022 08:33
Copy link
Contributor

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

Things to look at

packages/edge-gateway/src/gateway.js Outdated Show resolved Hide resolved
packages/edge-gateway/src/gateway.js Outdated Show resolved Hide resolved
@@ -64,6 +67,7 @@ kv_namespaces = [{ binding = "DENYLIST", id = "f4eb0eca32e14e28b643604a82e00cb3"
[env.staging.vars]
IPFS_GATEWAYS = "[\"https://ipfs.io\", \"https://cf.nftstorage.link\", \"https://pinata.nftstorage.link\"]"
GATEWAY_HOSTNAME = 'ipfs-staging.nftstorage.link'
EDGE_GATEWAY_API_URL = 'https://api.nftstorage.link'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be staging?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get rid of this? It would be nice not to need this URL to match up with the binding config as it causes an invariant that we humans must ensure we configure correctly. The binding config should be enough.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sadly we cannot :( Fetch fails if we do not provide a full URL. We can provide something random like example.com, but feeling like keeping the real URL is better (and also allows future move out of bindings by just changing code from env.API.fetch to fetch)

Co-authored-by: Oli Evans <oli.evans@gmail.com>
Copy link
Contributor

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

🚀

@vasco-santos vasco-santos merged commit 7a83045 into main Jul 6, 2022
@vasco-santos vasco-santos deleted the feat/api-get-from-bucket branch July 6, 2022 12:01
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.

GET https://api.nftstorage.link/perma-cache/:url
2 participants