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.

Sign up for GitHub

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

Jump to bottom

feat: Implement huaweicloud obs service read support #540

Merged
merged 9 commits into from
Aug 18, 2022
Merged

feat: Implement huaweicloud obs service read support #540

merged 9 commits into from
Aug 18, 2022

Conversation

teckick
Copy link
Contributor

@teckick teckick commented Aug 17, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

This PR implements Huaweicloud OBS service.

Related to #512

@Xuanwo Xuanwo marked this pull request as draft August 17, 2022 10:28
@Xuanwo Xuanwo changed the title [WIP] Implement huaweicloud obs service feat: Implement huaweicloud obs service Aug 17, 2022
Xuanwo
Xuanwo reviewed Aug 17, 2022
View reviewed changes
src/scheme.rs Outdated Show resolved Hide resolved
src/services/util/builder.rs Outdated Show resolved Hide resolved
src/services/util/mod.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
@teckick
Copy link
Contributor Author

teckick commented Aug 17, 2022

Thanks for reviewing, I'll fix it later.

Xuanwo
Xuanwo reviewed Aug 18, 2022
View reviewed changes
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.

Mostly LGTM!

src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
src/services/obs/backend.rs Outdated Show resolved Hide resolved
Xuanwo
Xuanwo approved these changes Aug 18, 2022
View reviewed changes
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.

Great work!

@Xuanwo
Copy link
Member

Xuanwo commented Aug 18, 2022

Looks so nice!

It's OK to me to merge current PR as-is, and we can continue the job in following PRs.

And in this way, you will become a contributor of OpenDAL and don't need approve to get CI run.

What do you think? @eastfisher

@teckick
Copy link
Contributor Author

teckick commented Aug 18, 2022 •
edited
Loading

Looks so nice!

It's OK to me to merge current PR as-is, and we can continue the job in following PRs.

And in this way, you will become a contributor of OpenDAL and don't need approve to get CI run.

What do you think? @eastfisher

Other methods in Backend (create, write, list, ...) are not implemented.

Besides, integration tests for OBS are not added.

Should I finish the above in this PR or create another PR?

@Xuanwo
Copy link
Member

Xuanwo commented Aug 18, 2022

Other methods in Backend (create, write, list, ...) have not been implemented.

It's better to split into different PRs. We can create a tracking issue for them.

Besides, integration tests for OBS are not added.

This is the latest part to be added. Itself is simple to add but need more steps on credentials and so on. We can discuss them later on.

Should I finish the above in this PR or create another PR?

I prefer to create other PRs~

@teckick
Copy link
Contributor Author

teckick commented Aug 18, 2022 •
edited
Loading

Great！

@Xuanwo
Copy link
Member

Xuanwo commented Aug 18, 2022

For this PR, we only need to make CI happy.

@Xuanwo Xuanwo marked this pull request as ready for review August 18, 2022 11:07
@Xuanwo Xuanwo changed the title feat: Implement huaweicloud obs service feat: Implement huaweicloud obs service read support Aug 18, 2022
@Xuanwo Xuanwo mentioned this pull request Aug 18, 2022
Closed
6 tasks
@Xuanwo Xuanwo merged commit e0bf2ee into apache:main Aug 18, 2022
@Xuanwo
Copy link
Member

Xuanwo commented Aug 18, 2022

Thanks for contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@teckick @Xuanwo