-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: Add support for Cloudflare R2 Storage #3352
Comments
If it's S3 compatible, can it be supported rn by using the configurable ENV Vars? (I'm genuinely curious as a sanity check). |
The way utopia storage works right now, a new Device needs to be implemented to change some parameters: The region constants for each provider are also a little nice to make sure you're not using the wrong region. Then, the Appwrite code needs to be updated to use the right device based on the That said, there is an issue for creating a generic S3 compatible device. And, I've seen a PR for a generic S3, but it doesn't look like it implements 1 S3 device that you can use for all S3 compatible providers. |
Cool, I always learn a lot from talking to you :P |
@gewenyu99 I think we had PR to do that at some point before 0.14. Not sure why it was not used. |
I have the same wish! |
Hello, I'd like to have a go at implementing this! |
@doublevcodes, thanks for your interest! 🙏 Happy hacking! |
So as I understood at it, I need to create 2 PRs? One to utopia-php/storage and one to this repo |
@doublevcodes That is correct 😇 You can take a look at this Hacktoberfest issue that we created, it's on the same topic but different adapter. You can use it's description to learn more: #3991 |
Hi, I'm a bit confused on how to implement this because of how Cloudflare R2 bucket URLs are structured. So far I have something like this: public function __construct(string $root, string $accountId, string $accessKey, string $secretKey, string $bucket, string $region = self::AUTO, string $acl = self::ACL_PRIVATE)
{
parent::__construct($root, $accessKey, $secretKey, $bucket, $region, $acl);
$this->headers['host'] = $accountId . '.' . 'r2' . '.' . 'cloudflarestorage' . '.' . 'com' . '/'. $bucket;
} However I'm sure that this wouldn't form a valid |
@doublevcodes, oye that's unfortunate Cloudflare does it differently...It looks like the implementation for Cloudflare won't be the same as the other S3 compatible providers like Linode. You'll need to figure out what else needs to be overridden in the base S3 class and implement it in the Cloudflare class. |
Ah. Thanks, I'll have a look at what I can do. |
@doublevcodes, FYI, virtual-hosted style paths should also be supported in Cloudflare |
Hello there, I was looking around for R2 support... You are not forced to keep the s3 = boto3.client('s3',
aws_access_key_id=key,
aws_secret_access_key=secret,
endpoint_url="https://blahblahblahblahblahblah.r2.cloudflarestorage.com")
with open("some_file_name.png", "rb") as f:
s3.upload_fileobj(f, "bucket-name-here", "some_file_name.png") hope this helps a bit |
Thank you everyone for celebrating Hacktoberfest 22 with us! This issue will now be closed as we're getting ready to celebrate Hacktoberfest 23. |
Dear @eldadfux I'm not sure this issue is related to an Hacktoberfest. It is a general request for the product's improvement |
Our team opened this task for Hacktoberfest :P @stnguyen90 is a part of our team. AFAIK, he doesn't actually want this feature :D |
Well, utopia-php/storage#76 is a thing and @stnguyen90 requested a review 3 weeks ago so it looks like to be almost ready to be merged? |
Yep, that will be a generic adaptor, too. So this will be supported in some future version along with all S3 compatible storage services 🤞 |
Could we re-open this? Cloudflare R2 provides much better costs than AWS and it's S3 compliant. It already use it in other projects by simply entering the Cloudflare R2 storage details where the AWS S3 information is usually entered. |
please re-open this |
@stnguyen90 @gewenyu99 any updates on my pr? |
@Tushar98644 the PR is still awaiting your follow up before we can merge |
any update? I think those 3 days are kind of over |
@tessamero Is this implemented yet? |
@docimin I closed the issue as the PR did not provide any proof it works as they wrote the code without Cloudflare access. They need to provide proof of tests passing for it to be merged and they are unable to. We are going to close the PR. |
@tessamero I'd like to reopen this issue because here is the implementation that can solve also this issue: #7172 |
@schneidermr , reopened the issue, thank you for submitting a PR :) |
@pietrodicaprio, thanks for the insight. Does this work too?
|
@stnguyen90 It does not. |
I forgot I actually researched this..According to this, it is supported. |
Is there still anything that blocks the related PR from being merged? R2 support would be awesome 🚀 |
Heyo, |
They will let me know if that's the case, but if you could understand the result of the unit test, you would see that there is also some problem with the local storage adapter test case. So my changes absolutely couldn't break any unit test with that extra function param what is optional and not required. I'm happy that you need this changes, but in the future please don't write these type of comments if you aren't senior enough to understand the changes in the code and the result of the unit tests. 🙂 |
No need to be passive aggressive 😕. I'm just trying to help. The answer is based on the last comment on there. I also wrote without assurance, which you most likely didn't read based on your answer. Just because you got mentioned doesn't automatically mean everyone is waiting for you and it's your fault. 😊 |
Closing this issue. Thank you so much for participating in Hacktoberfest 2023! We can't wait to welcome you all during HF 2024! Stay tuned for a lot more amazing issues from the Appwrite team! |
Isn't utopia-php/storage#103 solving utopia-php/storage#28 that allows any S3-like storage solving also the mentioned utopia-php/storage#76 ? |
76 and 28 will not be merged, as 103 already does this I think, however due to test issues in 103, utopia-php/storage#106 got created. So I would say keep checking 106. |
Correct, keep in mind that 106 is needed due to a repository configuration issue. I expect to be 103 the mentioned issue as reference (e.g. in release notes) |
Hey there 👋 |
Hello, It seems that the utopia-php/storage now supports custom S3 endpoints, and the feature has been released. To enable support for Cloudflare R2 Storage in Appwrite, I believe we just need to add a configuration option for custom S3 endpoints. There is a related PR (#7172) that addresses this by adding support for custom endpoint URLs for S3. Could this PR be reviewed and merged soon to enable this functionality? Thank you for your time and attention! |
🔖 Feature description
Cloudflare R2 Storage is an S3-compatible distributed object storage with generous pricing and a free tier. It would be great if Appwrite could make use of this storage provider.
🎤 Pitch
A storage provider with a free tier would make switching to and testing a cloud storage provider easier.
Related issue:
👀 Have you spent some time to check if this issue has been raised before?
🏢 Have you read the Code of Conduct?
The text was updated successfully, but these errors were encountered: