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

Added support for Alibaba Cloud storage adapter #95

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nidhi-singh02
Copy link

@nidhi-singh02 nidhi-singh02 commented Oct 14, 2023

Added support for Alibaba Cloud storage adapter

@vermakhushboo
Copy link
Member

Hey @nidhi-singh02, thank you for your contribution! ✨
Please run composer format to fix linter errors and also take a look at the failing tests. Also, you need to implement all abstract methods of the parent class i.e. Device class.


protected function setUp(): void
{
$this->alibaba = new Alibaba('accessKey', 'secretKey', 'bucket', 'endpoint');
Copy link
Member

Choose a reason for hiding this comment

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

You need to pass actual values of accessKey, secretKey etc. Those values can either be added to environment variables and read from there or you can take a reference from the S3 adapter.


public function testRead()
{
$this->expectException(OssException::class);
Copy link
Member

Choose a reason for hiding this comment

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

The tests need to ensure that the method works as expected. You can use a test Alibaba Cloud account, read values from there and assert that the value is being read properly. Similarly for other tests.


public function testExists()
{
$this->expectException(OssException::class);
Copy link
Member

Choose a reason for hiding this comment

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

Add tests for all cases like when the value exists and when it doesn't exist.

@vermakhushboo
Copy link
Member

@nidhi-singh02 have you made any progress? If yes, please update the same.

@gewenyu99
Copy link

Hey,

Due to time constraints, I'm going to mark this PR hacktoberfest-accepted for now so you get DO's Hacktoberfest rewards. We'll continue to work with you on this issue for review and merge.

When it is merged, we'll contact you for Appwrite-specific Hacktoberfest swag.

Thanks for helping us improve Appwrite!

@gewenyu99
Copy link

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@nidhi-singh02
Copy link
Author

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

That's great to hear. My discord user name is "nidzi."

@gewenyu99
Copy link

Will reach out ASAP, adding everyone's names to a spread sheet to reach out together. Thank you for your patience :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants