apache / iceberg-rust

Apache Iceberg
https://rust.iceberg.apache.org/
Apache License 2.0
477 stars 97 forks source link

fix: enable public access to ManifestEntry properties #284

Closed a-agmon closed 3 months ago

a-agmon commented 3 months ago

resolves #282 The purpose of the PR is to change the ManifestEntry properties and FileScanTask data_file() method to public in order to enable public access to the file metadata API. Some consumers are interested in the file paths that are part of the current table and their properties for query and monitoring purposes.

a-agmon commented 3 months ago

Just to add to the motivation, part of the purpose here is to allow this kind of API access:

    let mut stream = table_scan.plan_files().await?;
    while let Some(file) = stream.next().await {
        let data_file = file?.data_file();
        println!("---------------------------------");
        println!("File Path: {:?}", data_file.file_path());
        println!("File Size: {:?}", data_file.file_size_in_bytes());
    }

My team is using these APIs to creat a layer of monitoring on top of Iceberg table that can help track issues and identify problems

a-agmon commented 3 months ago

Hi @a-agmon Thanks for this pr. But per our previous discussion, we prefer to expose getter methods rather fields directly to user, you can see the discussion here.

Thanks @liurenjie1024 ! Sure. I will fix this.

a-agmon commented 3 months ago

Hi @liurenjie1024 , @Xuanwo I have addressed the issues and updated the PR. Please let me know what do you think

liurenjie1024 commented 3 months ago

cc @Xuanwo Any other comments?