Skip to content
This repository has been archived by the owner on Oct 22, 2021. It is now read-only.

Basic Implement of OneDrive Storager #17

Merged
merged 8 commits into from
Sep 28, 2021
Merged

Conversation

zxy-lgtm
Copy link
Member

Sorry for the late pull request.
The past few days I study the onedrive documents and beyondstorage specs, so it takes time to finish a basic one drive client and some adjust for the latest behavior.
For now, I have implement the basic storager for one drive, but the newest GSP-751 and one-drive conflict with the design of the empty file, the latter doesn't allow empty file exist, I didn’t found ways to avoid that, so I think this could to be discuss later.
ref: https://stackoverflow.com/questions/64772346/uploading-empty-0-byte-to-sharepoint-online (since the ms voice has shut down …)

TL;DR:
This storage has pass the integration-test except the part of zero size file.

@zxy-lgtm
Copy link
Member Author

2021-09-27 22-39-21 的屏幕截图
2021-09-27 22-38-28 的屏幕截图

@xxchan
Copy link

xxchan commented Sep 27, 2021

Great work! Give me some time to review the details..

@xxchan
Copy link

xxchan commented Sep 27, 2021

I think we can create an error code like below and returns it when size=0, but this doesn't pass integration test.

var ErrWriteFileEmpty = services.NewErrorCode("onedrive doesn't support upload empty file")

I think GSP-751 just considered the API flavor, but doesn't consider the case where writing empty file is not supported... Ping @abyss-w and @JinnyYi

Makefile.env Outdated Show resolved Hide resolved
utils.go Outdated Show resolved Hide resolved
utils.go Outdated Show resolved Hide resolved
utils.go Outdated Show resolved Hide resolved
utils_drives.go Outdated Show resolved Hide resolved
utils_drives.go Outdated Show resolved Hide resolved
utils_drives.go Outdated Show resolved Hide resolved
utils_drives.go Outdated Show resolved Hide resolved
@xxchan
Copy link

xxchan commented Sep 27, 2021

The build CI fails, and you can see it's format check: https://github.com/beyondstorage/go-service-onedrive/runs/3722658754

You can run make build before pushing your code to ensure it's formatted :)

@JinnyYi
Copy link
Contributor

JinnyYi commented Sep 28, 2021

I think we can create an error code like below and returns it when size=0, but this doesn't pass integration test.

var ErrWriteFileEmpty = services.NewErrorCode("onedrive doesn't support upload empty file")

I think GSP-751 just considered the API flavor, but doesn't consider the case where writing empty file is not supported... Ping @abyss-w and @JinnyYi

We need to specify the expected behavior when writing an empty file is not supported.

For integration test, how about splitting integration test cases like what we did for Move/Copy?

@Xuanwo
Copy link
Contributor

Xuanwo commented Sep 28, 2021

I think we need to come up with a solution like feature flags or something. Can you just pin the go-integration-test to the older version and leave a comment on this comment? We expect to solve this problem in October.

go.sum Outdated Show resolved Hide resolved
@zxy-lgtm
Copy link
Member Author

zxy-lgtm commented Sep 28, 2021 via email

@zxy-lgtm
Copy link
Member Author

zxy-lgtm commented Sep 28, 2021 via email

@xxchan xxchan merged commit f1138af into beyondstorage:master Sep 28, 2021
@xxchan
Copy link

xxchan commented Sep 28, 2021

Nice work!

Next, let's add docs like:

At the same time, don't forget the deadline of the final report of OSPP!

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

Successfully merging this pull request may close these issues.

4 participants