Add a feature based object store provider #258
Conversation
|
Hi @andygrove, @tustvold, @matthewmturner, could you help review this PR? |
There was a problem hiding this comment.
I don't really have enough context on ballista to meaningfully review this, although its use of ObjectStoreProvider looks plausible.
apache/datafusion#3540 might also be related, a common way to register object stores based on environment variables seems sensible to me
|
|
||
| /// An object store detector based on which features are enable for different kinds of object stores | ||
| pub struct FeatureBasedObjectStoreProvider; |
There was a problem hiding this comment.
Is this just a placeholder, it doesn't appear to be doing anything?
There was a problem hiding this comment.
There is a follow-on PR #260 to implement this for HDFS but it is waiting on another PR to be merged
There was a problem hiding this comment.
Yes, based on this PR, we can easily add feature-based object stores like #260 does. And we have one verified example without manual object store registration in https://github.com/datafusion-contrib/datafusion-objectstore-hdfs/blob/master/hdfs-examples/src/bin/ballista-sql.rs
|
cc @avantgardnerio who has been working on supporting custom object stores in DataFusion/Ballista |
|
The approach here LGTM but I have not worked with the object store yet |
|
Let's merge it first. Will update the ObjectStoreProvider interface after upgrading datafusion to the latest version. |
|
It's unclear from skimming this: what is the intended usage? Should a statement like: Work now? |
Which issue does this PR close?
Closes #257, #6.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?