Initial HTTP support for Virtual Refs#938
Conversation
paraseba
left a comment
paraseba
left a comment
There was a problem hiding this comment.
Can we document or link to documentation on what the options are?
|
Yeah I can actually make them more strongly typed |
|
I think this is good to merge |
I'll take a look today @mpiannucci |
|
This feature would be amazing to have! we are testing VirtualiZarr integrations and one of the most recurrent questions has been "can we test it with Icechunk?" and our data doesn't allow egress with S3(NASA Earth). thanks for all the work @paraseba @mpiannucci ! |
| .as_ref() | ||
| .unwrap_or(&HashMap::new()) | ||
| .iter() | ||
| .fold(builder, |builder, (key, value)| builder.with_config(*key, value)); |
There was a problem hiding this comment.
@mpiannucci can you use the Settings you get as argument to configure the retry attempts this store will do? You can base it on the S3 or GCS examples in this same file
|
|
||
| #[typetag::serde(name = "http_object_store_provider")] | ||
| impl ObjectStoreBackend for HttpObjectStoreBackend { | ||
| fn mk_object_store( |
There was a problem hiding this comment.
We would need to override the can_write method to return false.
| Err("in memory storage does not accept credentials".to_string()) | ||
| } | ||
| (ObjectStoreConfig::Http(_), _) => { | ||
| Err("http storage does not support credentials".to_string()) |
There was a problem hiding this comment.
Should we support basic auth at least? Is there a reason we cannot or simply not implementing in this PR? If so, please add a TODO in the code and a ticket
There was a problem hiding this comment.
I dont want to do it in this PR, Ill add a ticket
icechunk/src/virtual_chunks.rs
Outdated
| }, | ||
| ), | ||
| ( | ||
| "https".to_string(), |
There was a problem hiding this comment.
I guess we don't need to add https because http will match..
3 participants
Tested from python, lemme know if we need rust tests too
Closes #526