search

datafusion-contrib / datafusion-objectstore-s3

S3 as an ObjectStore for DataFusion
Apache License 2.0
59 stars 13 forks source link

Added query test and readme example #18

Closed matthewmturner closed 2 years ago

seddonm1 commented 2 years ago

Looks good. My only comment would be something to the readme about using non-aws sources and credentials. 👍

matthewmturner commented 2 years ago

Looks good. My only comment would be something to the readme about using non-aws sources and credentials. 👍

yes sure will do. still have to fix the test i added too.

matthewmturner commented 2 years ago

@seddonm1 with that line of thinking - do you think we should generalize and remove references to "Amazon" from AmazonS3FileSystem and AmazonS3FileReader and the module could be s3 instead of aws?

seddonm1 commented 2 years ago

@matthewmturner I think Amazon probably 'owns' the standard and majority of people will use Amazon S3.

I hope the readme can just clarify to users they can use with alternatives (minio, ceph, etc)

matthewmturner commented 2 years ago

@seddonm1 ok sure sounds good.

seddonm1 commented 2 years ago

I think this is good to merge. I might make some changes to README once merged just to clarify how the credentials work.

matthewmturner commented 2 years ago

just want to fix my test, wanted to use assert_batches_eq macro like we do in datafusion. will have that done soon then will merge