Remove dependency of common for the storage crate #2076
Conversation
|
Hi @matthewmturner, @yjshen, @alamb, what do you think? |
| use std::{io, result}; | ||
|
|
||
| /// Result type for operations that could result in an io error | ||
| pub type Result<T> = result::Result<T, io::Error>; |
There was a problem hiding this comment.
Im wondering if any impact from using this instead of DataFusionError::IoError but i think its fine and of course it would require keeping datafusion-common dependency which would defeat the point of this PR.
There was a problem hiding this comment.
The goal of this PR is to remove the datafusion-common dependency.
When you need to use DataFusionError::IoError, it's simply to add .map_err(DataFusionError::IoError) like some places in this PR does.
|
I think this is quite nice. Can you just clarify how this will make it easier for datafusion core to depend on the object store extensions? Piggy backing on @alamb's idea from yesterday, and in light of this change, im wondering if new crate for all listing related code could be |
Suppose we hope to introduce optional features of object store extensions in the datafusion core. Both of the datafusion core and the object store extensions should depend on the same storage crate, either by version or rev rather than by path. Then if the storage crate depends on the common crate, it will be not good for the common crate to have the same version or rev as the one for the storage crate. Otherwise, if the storage crate does not depend on any datafusion crate, it will be possible to make it stable with some version. And then both of the datafusion core and object store extensions depend on that version.
I'm good with creating a new crate for the listing things. |
Actually, I prefer to regard the storage crate as a general object store interface and move it out from datafusion😂. And maybe we can manage the object store like Ballista and reorganize the whole project like follows: And have another PR for the reorganization. |
|
Hi @alamb, @matthewmturner, @yjshen, another PR is created for reorganizing the whole project folders #2081. What do you think? |
|
Thanks @yahoNanJing @yjshen and @matthewmturner |
Which issue does this PR close?
Closes #2075.
Rationale for this change
It's better to make the storage crate not dependent on any datafusion crates, which will make it easy for the object store extensions to depend on the storage crate. And it will also make it easy for the datafusion core to depend on both the storage crate and the object store extensions.
What's more, the storage crate should be relatively stable, since it defines the basic traits for accessing object stores. Later, maybe we can extract it to another repository. Then the repositories of the object store extensions and the datafusion repository won't depend on each other.
What changes are included in this PR?
Are there any user-facing changes?