refactor: Use opendal to replace object_store #978
Conversation
Signed-off-by: Xuanwo <github@xuanwo.io>
Xuanwo
force-pushed
the
storage-refactor
branch
from
7d04759 to
cfad6c8
Compare
Signed-off-by: Xuanwo <github@xuanwo.io>
…nto storage-refactor
Signed-off-by: Xuanwo <github@xuanwo.io>
|
Some tests are failing. I'm working on it. |
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
|
@Xuanwo did you look at the latest CI by a chance? |
Signed-off-by: Xuanwo <github@xuanwo.io>
|
Hi, the only thing left is |
|
@Xuanwo we dont have any S3 bucket used for testing, in these specific tests it seems the old implementation would not actually connect to S3 (because the test is testing for failure). |
Thank you for the information, I will take an another look. |
Signed-off-by: Xuanwo <github@xuanwo.io>
Signed-off-by: Xuanwo <github@xuanwo.io>
|
Hi, @jondot sorry for the long waiting. All tests should pass now. Please take another look, thank you! |
| let fs = Fs::default().root("/"); | ||
| Box::new(OpendalAdapter::new( | ||
| Operator::new(fs) | ||
| .expect("fs service should build with success") |
There was a problem hiding this comment.
Can we handle this without panic?
Would it make more sense to return a Result instead?
There was a problem hiding this comment.
Hi, I'm willing to return an error if changing the signature is allowed. This PR tries it's best to not change the exposed public API.
There was a problem hiding this comment.
I think it is better than I don't expect.
What do you think? it should't affect the users, right? Only framework changes
| Box::new(ObjectStoreAdapter::new(Box::new(InMemory::new()))) | ||
| Box::new(OpendalAdapter::new( | ||
| Operator::new(Memory::default()) | ||
| .expect("memory service must build with success") |
Close #948
This PR will use opendal to replace object_store.
The most significant change is the addition of the
GetResponsestruct. After this PR,GetResponsewill no longer expose any external types; instead, it will provide its own public API that users can rely on.cc @jondot to take a review, thank you!
There are many things we can consider, such as enabling logging, tracing, and metrics layers to make our storage driver more production-ready. Perhaps we can discuss this later in a separate issue.