Uh oh!
There was an error while loading. Please reload this page.
Added Oracle Object Storage adapter - #91
Conversation
stnguyen90
left a comment
There was a problem hiding this comment.
Great PR! 🤯 We left some comments during the review, please check them out.
Please also make sure to:
- to run the formatter to fix the lint errors
- attach a screenshot of successful tests
| * @var string | ||
| */ | ||
| protected string $root = 'oracle-object'; | ||
There was a problem hiding this comment.
Shouldn't there be region constants?
| public function __construct(string $root = '') | ||
| { | ||
| $this->root = $root; | ||
| } |
There was a problem hiding this comment.
There should definitely be more parameters than this like some sort of key and bucket.
There was a problem hiding this comment.
Thanks!
Will solve the mentioned things and push the changes.
SaketKaswa20
commented
Oct 14, 2023
I am facing problem in phpunit test. It is showing could not read phpunit.xml when using |
gewenyu99
commented
Oct 18, 2023
@stnguyen90 take a peek at the comment here |
stnguyen90
left a comment
There was a problem hiding this comment.
Changes still pending.
Discord thread: https://discord.com/channels/564160730845151244/1162704045937541140
stnguyen90
left a comment
There was a problem hiding this comment.
Please see my comments and make sure to run the formatter to fix the lint problems.
| public function __construct(string $root = '', array $config = []) | ||
| { | ||
| $this->root = $root; | ||
| $this->config = $config; |
There was a problem hiding this comment.
Let's not use a config array like this. Please take a look at how similar adapters are implemented in this repo to see how the configuration should be implemented.
| use Utopia\Storage\Device; | ||
| use Utopia\Storage\Storage; | ||
| class OracleObject extends Device |
There was a problem hiding this comment.
This should probably extend the S3 adapter because Oracle Object is an S3-compatible storage provider.
| public function uploadFileToOracleObjectStorage(string $localFilePath, string $remoteFilePath): bool | ||
| { | ||
| // Implement logic to upload a file to Oracle Object Storage | ||
| // Return true on success, false on failure | ||
| // You should use the Oracle Object Storage SDK or API for this operation | ||
| return true; // Example: always return true for the sake of testing | ||
| } | ||
| public function downloadFileFromOracleObjectStorage(string $remoteFilePath, string $localFilePath): bool | ||
| { | ||
| // Implement logic to download a file from Oracle Object Storage | ||
| // Return true on success, false on failure | ||
| // You should use the Oracle Object Storage SDK or API for this operation | ||
| return true; // Example: always return true for the sake of testing | ||
| } | ||
| public function deleteFileFromOracleObjectStorage(string $remoteFilePath): bool | ||
| { | ||
| // Implement logic to delete a file from Oracle Object Storage | ||
| // Return true on success, false on failure | ||
| // You should use the Oracle Object Storage SDK or API for this operation | ||
| return true; // Example: always return true for the sake of testing | ||
| } |
| const DEVICE_LINODE = 'linode'; | ||
| const DEVICE_ORACLEOBJECT = 'oracleobject'; |
There was a problem hiding this comment.
This should be DEVICE_ORACLE_OBJECT just like DEVICE_DO_SPACES:
| constDEVICE_ORACLEOBJECT = 'oracleobject'; | |
| constDEVICE_ORACLE_OBJECT = 'oracleobject'; |
gewenyu99
commented
Oct 31, 2023
Hey, Due to time constraints, I'm going to mark this PR When it is merged, we'll contact you for Appwrite-specific Hacktoberfest swag. Thanks for helping us improve Appwrite! |
stnguyen90
commented
Oct 31, 2023
Unfortunately, I'm going to need to remove the hacktoberfest-accepted label as what's done so far doesn't actually implement anything. |
SaketKaswa20
commented
Nov 1, 2023
No worries! It was great working with you all! |
iSuslov
commented
Dec 1, 2023
Well, kind of sad. Are there any plans to support it afterall? Oracle Cloud is undervalued for sure. |
gewenyu99
commented
Dec 18, 2023
@iSuslov We'd still love a valid implementation of the Oracle Object Storage adapter. I just think the existing PR wasn't quite ready for merge. We could also see a generic S3 adapter in the future, which would also solve the same issue. TL;DR: We'd love to support it, this PR just wasn't ready for merge :) |
Added Oracle Object Storage Adapter
Solving Issue #6415