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

Add OpenDAL as another kind of storage adapter #948

Open
Xuanwo opened this issue Nov 2, 2024 · 4 comments · May be fixed by #978
Open

Add OpenDAL as another kind of storage adapter #948

Xuanwo opened this issue Nov 2, 2024 · 4 comments · May be fixed by #978

Comments

@Xuanwo
Copy link

Xuanwo commented Nov 2, 2024

Description

Hi, OpenDAL is another widely used storage abstraction in the Rust ecosystem and offers many more storage integrations.

image

I'm willing to add an OpendalAdapter so that users can choose based on their own needs or existing dependencies.

Do you think it's a good idea?

@jondot
Copy link
Contributor

jondot commented Nov 2, 2024

We currently use object store and kind of wrapped it.

Do you think we should replace object store with this? Will that work better? Im not against replacing as long as we pick a leading store abstraction and keep with it.

@Xuanwo
Copy link
Author

Xuanwo commented Nov 3, 2024

Hi @jondot, thank you so much for your swift response!

As an OpenDAL maintainer and object_store contributor, I must acknowledge that my perspective might be somewhat biased.

However, I believe OpenDAL is particularly well-suited for Loco, not just because it offers extensive storage support, but also because it seamlessly integrates with the Rust ecosystem. This includes native logging, tracing, metrics, retry mechanisms, concurrent limits, and speed limit support, all of which are crucial for a robust framework like Loco. All of those features can be enabled by feature flags which compiling.

I'm happy to contribute to this part of the project and take responsibility for further maintenance of the storage component.

@Xuanwo
Copy link
Author

Xuanwo commented Nov 8, 2024

Hi, @jondot. I would be happy to implement this change to replace it with opendal. OpenDAL hasn't reached version 1.0 yet (we are working on it: apache/opendal#5189). I will take responsibility for helping upgrade OpenDAL as breaking changes occur, if you want.

I also plan to completely hide opendal and make it purely an implementation detail. This will address the following concern and allow us to switch from opendal easily in the worst-case scenario.

// TODO: need to properly abstract the object_store type in order to not
// strongly depend on it
pub type GetResponse = object_store::GetResult;

@jondot
Copy link
Contributor

jondot commented Nov 9, 2024

@Xuanwo please feel free to submit PR, it sounds good!

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

Successfully merging a pull request may close this issue.

2 participants