-
Notifications
You must be signed in to change notification settings - Fork 32
Support http(s) stores #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/object_store/http.rs
Outdated
| let allow_http = std::env::var("ALLOW_HTTP").is_ok(); | ||
|
|
||
| let client_options = ClientOptions::new() | ||
| .with_allow_invalid_certificates(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do not allow it without env vars. We may accept custom certificates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets not support custom certificates now.
src/arrow_parquet/uri_utils.rs
Outdated
| ObjectStoreScheme::parse(uri).map_err(|_| { | ||
| format!( | ||
| "unrecognized uri {}. pg_parquet supports local paths, s3:// or azure:// schemes.", | ||
| "unrecognized uri {}. pg_parquet supports local paths, https://, s3:// or azure:// schemes.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated, but I think it's better to advertise the az:// scheme
| ObjectStoreScheme::AmazonS3 => parse_s3_bucket(uri) | ||
| .ok_or(format!("unsupported s3 uri {uri}")) | ||
| .map(Some), | ||
| ObjectStoreScheme::MicrosoftAzure => parse_azure_blob_container(uri) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this preserve Azure's https://...windows.something.net/ syntax?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only allow az(ure)://{container}/key or https://{account}.blob.core.windows.net/{container}. Uris are correctly identified by object store. (some https are azure or aws)
110f50c to
b655643
Compare
934dfbf to
6a3c530
Compare
6a3c530 to
5eea67e
Compare
5eea67e to
6cf1b32
Compare
pg_parquet supports reading/writing from/to public http(s) parquet uris to/from Postgres tables.
6cf1b32 to
063c596
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
==========================================
+ Coverage 91.84% 91.94% +0.09%
==========================================
Files 83 84 +1
Lines 10494 10597 +103
==========================================
+ Hits 9638 9743 +105
+ Misses 856 854 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pg_parquet supports reading/writing from/to public http(s) parquet uris to/from Postgres tables.
Closes #108.