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

feat(services/aliyun-drive): support AliyunDrive #4585

Merged
merged 1 commit into from
May 17, 2024
Merged

feat(services/aliyun-drive): support AliyunDrive #4585

merged 1 commit into from
May 17, 2024

Conversation

yuchanns
Copy link
Member

@yuchanns yuchanns commented May 8, 2024

Closing #3415. Ready for review.

API documentation reference: https://www.yuque.com/aliyundrive/zpfszx/gogo34oi2gy98w5d

Required Environment Variables:

export OPENDAL_ALIYUN_DRIVE_CLIENT_ID=
export OPENDAL_ALIYUN_DRIVE_CLIENT_SECRET=
export OPENDAL_ALIYUN_DRIVE_REFRESH_TOKEN=
export OPENDAL_ALIYUN_DRIVE_RAPID_UPLOAD=true
export OPENDAL_TEST=aliyun_drive

To run behavior tests, use the following command:

cd core && cargo test behavior --features tests,services-aliyun-drive -- --test-threads=1

Note that the test threads should be set to 1 as it may fail part of the test due to rate limit.

AliyunDrive has a feature called Rapid Upload, but it is difficult to implement completely because the MultipartWriter trait design isn't well compatible with this feature at present. Therefore, I have only implemented it on the write_once method.

Let's discuss proposing a new RapiduploadWriter for this kind of rapid-upload featured service together in the future if you would.

@yuchanns yuchanns marked this pull request as ready for review May 16, 2024 03:48
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Only some small changes.

core/src/services/aliyun_drive/backend.rs Show resolved Hide resolved
core/src/services/aliyun_drive/backend.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/backend.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/core.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/core.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/core.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/core.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/lister.rs Outdated Show resolved Hide resolved
core/src/services/aliyun_drive/mod.rs Outdated Show resolved Hide resolved
core/src/services/mod.rs Outdated Show resolved Hide resolved
@yuchanns
Copy link
Member Author

yuchanns commented May 16, 2024

Done.


Sure, I'll fix them later today.

@yuchanns yuchanns changed the title feat(services/aliyundrive): support AliyunDrive feat(services/aliyun-drive): support AliyunDrive May 17, 2024
@Xuanwo
Copy link
Member

Xuanwo commented May 17, 2024

AliyunDrive has a feature called Rapid Upload, but it is difficult to implement completely because the MultipartWriter trait design isn't well compatible with this feature at present. Therefore, I have only implemented it on the write_once method.

Hi, it's not a requirement to implement MultipartWriter. Feel free to implement oio::Write on you own.

@yuchanns
Copy link
Member Author

yuchanns commented May 17, 2024

it's not a requirement to implement MultipartWriter

Well. that's fine. I will raise another PR to improve it. Let's make it work at first.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

.github/services/aliyun_drive/aliyun_drive/action.yml Outdated Show resolved Hide resolved
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks, bravo work!

@Xuanwo Xuanwo merged commit 08b2a96 into apache:main May 17, 2024
212 checks passed
@yuchanns yuchanns deleted the support-aliyun-drive branch May 17, 2024 13:52
@suyanhanx
Copy link
Member

Great work! Thank you!

@suyanhanx
Copy link
Member

I'll set up the test credentials for this service.

@imWildCat
Copy link
Contributor

thank you!

George-Miao pushed a commit to George-Miao/opendal that referenced this pull request Jun 5, 2024
feat(services/aliyun-drive): add support for AliyunDrive
@imWildCat
Copy link
Contributor

imWildCat commented Jun 13, 2024

Trying to use this in production but this implementation is too different from that of OneDrive and Google Drive.

The major question is that: Why this OpenDAL implement requires refresh_token and client_id?

The AliyunDrive API supports OAuth. IMHO, we can just send an access_token to the operator and let the host app handle the auth part.

Not all users would have the refresh_token. By design, AliyunDrive does not generate refresh_token for non-server applications https://www.yuque.com/aliyundrive/zpfszx/eam8ls1lmawwwksv

Sample token redeem response:

{
  "token_type": "Bearer",
  "access_token": "ey...",
  "refresh_token": null,
  "expires_in": 2592000
}

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.

4 participants