-
Notifications
You must be signed in to change notification settings - Fork 10
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
Conversation
Deploying with Cloudflare Pages
|
55dd335
to
442f822
Compare
442f822
to
342ebe6
Compare
342ebe6
to
d84e893
Compare
ecbffd3
to
77fe24b
Compare
5c62b34
to
d098a81
Compare
d098a81
to
1171068
Compare
There was a problem hiding this 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/wrangler.toml
Outdated
@@ -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' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be staging?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>
ed97be9
to
c8819bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This PR adds:
Other notes:
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.jsNeeds:
Closes #101