sdd commented on code in PR #1227:
URL: https://github.com/apache/iceberg-rust/pull/1227#discussion_r2053486295
##########
crates/iceberg/src/spec/manifest_list.rs:
##########
@@ -656,7 +658,29 @@ impl ManifestFile {
/// Load [`Manifest`].
///
/// This method will also initialize inherited values of
[`ManifestEntry`], such as `sequence_number`.
- pub async fn load_manifest(&self, file_io: &FileIO) -> Result<Manifest> {
+ ///
+ /// If `object_cache` is provided, it will be used to cache the manifest.
+ pub async fn load_manifest(
Review Comment:
> Perhaps we should design our API differently, such as using
`table.load_manifest()`, which might make more sense. This can also avoid pass
`FileIO` all the way.
I think this has the opportunity to provide the most ergonomic result
(whilst also potentially more disruptive to existing users, although we may be
able to mitigate disruption by exposing both old and new methods for a while
and mark the old ones as deprecated).
Most of the time, I'd anticipate that most users would want to use the same
FileIO and ObjectCache instance across all loaded tables and for each call to
table methods. But sometimes a user may want to have an instance of a Table
that has different FileIO / ObjectCache to other tables.
With that in mind, I'd suggest an API where Catalog builders would expose
`with_file_io()` / `with_object_cache()` methods that specify an FileIO and
ObjectCache that are provided by default to the table builder when the
catalog's `load_table` method is called.
Then, the table builders could expose `with_file_io()` /
`with_object_cache()` as well so that consumers can specify an alternative
FileIO and/or ObjectCache at the point at which they instantiate the Table,
that would be used instead of the default ones passed from the Catalog.
Putting both of these FileIO / ObjectCache / TableMetadata objects into a
`TableContext` struct feels like a good approach to avoid a proliferation of
arguments being passed around like here in `load_manifest` and below in
`load_manifest_list`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
