Skip to content

fix(python): use resolving storage for python binding #2246

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.

Sign up for GitHub

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

Merged
geruh merged 4 commits into from
Mar 18, 2026
Merged

fix(python): use resolving storage for python binding #2246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42c53b7
use resolving storage for python binding
CTTY Mar 17, 2026
1817c98
fmt
CTTY Mar 17, 2026
d2ff411
update toml to include all features
CTTY Mar 17, 2026
ae3b505
use opendal-all
CTTY Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bindings/python/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ crate-type = ["cdylib"]
[dependencies]
arrow = { version = "57.1", features = ["pyarrow", "chrono-tz"] }
iceberg = { path = "../../crates/iceberg" }
iceberg-storage-opendal = { path = "../../crates/storage/opendal", features = ["opendal-s3", "opendal-fs", "opendal-memory"] }
iceberg-storage-opendal = { path = "../../crates/storage/opendal", features = ["opendal-all"] }
pyo3 = { version = "0.26", features = ["extension-module", "abi3-py310"] }
iceberg-datafusion = { path = "../../crates/integrations/datafusion" }
datafusion-ffi = { version = "52.1" }
Expand Down
30 changes: 3 additions & 27 deletions bindings/python/src/datafusion_table_provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,16 @@ use std::sync::Arc;
use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
use datafusion_ffi::table_provider::FFI_TableProvider;
use iceberg::TableIdent;
use iceberg::io::{FileIOBuilder, StorageFactory};
use iceberg::io::FileIOBuilder;
use iceberg::table::StaticTable;
use iceberg_datafusion::table::IcebergStaticTableProvider;
use iceberg_storage_opendal::OpenDalStorageFactory;
use iceberg_storage_opendal::OpenDalResolvingStorageFactory;
use pyo3::exceptions::{PyRuntimeError, PyValueError};
use pyo3::prelude::{PyAnyMethods, PyCapsuleMethods, *};
use pyo3::types::{PyAny, PyCapsule};

use crate::runtime::runtime;

/// Parse the scheme from a URL and return the appropriate StorageFactory.
fn storage_factory_from_path(path: &str) -> PyResult<Arc<dyn StorageFactory>> {
let scheme = path
.split("://")
.next()
.ok_or_else(|| PyRuntimeError::new_err(format!("Invalid path, missing scheme: {path}")))?;

let factory: Arc<dyn StorageFactory> = match scheme {
"file" | "" => Arc::new(OpenDalStorageFactory::Fs),
"s3" | "s3a" => Arc::new(OpenDalStorageFactory::S3 {
configured_scheme: scheme.to_string(),
customized_credential_load: None,
}),
"memory" => Arc::new(OpenDalStorageFactory::Memory),
_ => {
return Err(PyRuntimeError::new_err(format!(
"Unsupported storage scheme: {scheme}"
)));
}
};

Ok(factory)
}

pub(crate) fn validate_pycapsule(capsule: &Bound<PyCapsule>, name: &str) -> PyResult<()> {
let capsule_name = capsule.name()?;
if capsule_name.is_none() {
Expand Down Expand Up @@ -110,7 +86,7 @@ impl PyIcebergDataFusionTable {
let table_ident = TableIdent::from_strs(identifier)
.map_err(|e| PyRuntimeError::new_err(format!("Invalid table identifier: {e}")))?;

let factory = storage_factory_from_path(&metadata_location)?;
let factory = Arc::new(OpenDalResolvingStorageFactory::new());
Copy link
Contributor

@kevinjqliu kevinjqliu Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it! its like java's ResolvingFileIO

Copy link
Contributor

@kevinjqliu kevinjqliu Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can start exporting this as another FileIO for pyiceberg

Copy link
Collaborator Author

@CTTY CTTY Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally agree that we should expose something like this to Pyiceberg

There is a tracking issue to expose storage factory as a configurable item to Pyiceberg: #2200


let mut builder = FileIOBuilder::new(factory);

Expand Down
Loading