Add support for dynamic object store usage via clade #504
Conversation
src/provider.rs
Outdated
|
|
||
| if let Err(err) = delta_table.load().await { | ||
| warn!("Failed to load table {name}: {err}"); | ||
| println!("{err}"); |
There was a problem hiding this comment.
leftover debug code?
| println!("{err}"); |
src/catalog/metastore.rs
Outdated
| let table_log_store = match table.location { | ||
| // Use the provided customized location | ||
| Some(location) => { | ||
| let store = stores.get(&location).expect("store for location exists"); |
There was a problem hiding this comment.
Make this an Err()? We don't want to panic and crash if the upstream API doesn't give us a store for the right prefix and this returns a CatalogResult anyway.
There was a problem hiding this comment.
Yup planned to do it and forgot, thanks.
src/catalog/metastore.rs
Outdated
| Path::from("/"), | ||
| )) | ||
| } else { | ||
| // TODO: this won't work if the default store has the same scheme as the queried one |
There was a problem hiding this comment.
Does this mean we can't support multiple stores that use the same scheme (s3) but have different buckets / prefixes or is this benign?
There was a problem hiding this comment.
This is benign, this pathway doesn't even take place, since we instantiate all the tables with object stores (and not url + object store options).
The only reason we need to register these in the first place is so that delta-rs knows all allowed schemes. Will return an Err here explicitly to clarify that.
This enables metastore to setup delta tables with a different object store than the one configured in the toml file.
Currently does not support writes, due to metastore limitations.