-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feature: Support AWS SDK JS v3 #22
base: main
Are you sure you want to change the base?
Conversation
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.
Hey, first of all thanks for your work and your PR.
Unfortunately I can not merge it in the current state. Seems like the whole functionality of using R2 with your own domain is getting removed with this PR (config.cloudflarePublicAccessUrl
). This is a crucial feature and I think for many people the reason they are using this provider instead of the default strapi aws provider (which can be used with R2 as well). Please add this feature to your PR before we can merge it into main.
Thanks a lot and let me know if you have any further questions.
Hey 👋🏼 |
🆙 @tilman |
@anthlasserre the endpoint is the actual S3 endpoint of the bucket. |
Thanks for your answer @JannikZed. I'll give a try with a public bucket and I'll let you know |
@JannikZed Can you please let me know if it's fine for you now ? |
); | ||
}); | ||
}, | ||
file.url = getFileURL(file, config); |
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 should not assign back into file.url
, the getFileURL is already doing that and causes this to overwrite and make it null, causing rendering issues for the file in the dashboard.
Context
I have been looking for an active Cloudflare R2 plugin for Strapi but I couldn't. So I have decided to re-work it from scratch. Here are the references I have been using to generate this new
index.js
file.Implementation
Issue related
Here is an issue related to this PR.