Skip to content

Conversation

@pitrou
Copy link
Member

@pitrou pitrou commented Feb 11, 2020

Allow instantiating an S3 filesystem instance from a s3://user:pass@bucket/path?params... URI.

@pitrou pitrou force-pushed the ARROW-7761-s3-uris branch from d805f8a to 02229c6 Compare February 11, 2020 14:39
@github-actions
Copy link

@fsaintjacques fsaintjacques self-requested a review February 11, 2020 14:47
@pitrou pitrou force-pushed the ARROW-7761-s3-uris branch 3 times, most recently from 2294951 to ddcb28f Compare February 13, 2020 11:30
@fsaintjacques
Copy link
Contributor

Works great :D

01:build/ (pr/6403✗) $ time release/dataset-parquet-scan-example file:///home/fsaintjacques/datasets/nyc-tlc/parquet
Table size: 746

real    0m3.248s
user    0m5.832s
sys     0m0.203s
01:build/ (pr/6403✗) $ time release/dataset-parquet-scan-example 's3://123:12345678@nyc-tlc/parquet?scheme=http&endpoint_override=localhost:9000'
Table size: 746

real    0m5.600s
user    0m7.557s
sys     0m0.474s

@fsaintjacques
Copy link
Contributor

One thing that I noted in my local integration, I had to explicit the global init of S3. I think it might be worth providing a default or implicitly calling the init step in FileSystemFromUri.

Allow instantiating an S3 filesystem instance from a
`s3://user:pass@bucket/path?params...` URI.
@pitrou pitrou force-pushed the ARROW-7761-s3-uris branch from 89611f8 to 3c73b76 Compare February 17, 2020 13:55
@pitrou
Copy link
Member Author

pitrou commented Feb 17, 2020

@fsaintjacques Could you try again now?

@fsaintjacques
Copy link
Contributor

fsaintjacques commented Feb 17, 2020

I pushed a change to the dataset example, it seems to be lost did you push force before pulling? It was a small change, I can make it again.

@pitrou
Copy link
Member Author

pitrou commented Feb 17, 2020

Oh, sorry. Yes, I force-pushed after rebasing :-/

@fsaintjacques
Copy link
Contributor

It should pass the CI, I'll merge after. The diff of the dataset example shows how well designed the APIs is.

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 this pull request may close these issues.

2 participants