Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bindings/python/DEPENDENCIES.rust.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ opendal-layer-concurrent-limit@0.57.0 X
opendal-layer-logging@0.57.0 X
opendal-layer-retry@0.57.0 X
opendal-layer-timeout@0.57.0 X
opendal-service-azblob@0.57.0 X
opendal-service-azdls@0.57.0 X
opendal-service-azure-common@0.57.0 X
opendal-service-fs@0.57.0 X
Expand Down
31 changes: 31 additions & 0 deletions crates/iceberg/public-api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,30 @@ pub fn iceberg::inspect::SnapshotsTable<'a>::new(table: &'a iceberg::table::Tabl
pub async fn iceberg::inspect::SnapshotsTable<'a>::scan(&self) -> iceberg::Result<iceberg::scan::ArrowRecordBatchStream>
pub fn iceberg::inspect::SnapshotsTable<'a>::schema(&self) -> iceberg::spec::Schema
pub mod iceberg::io
pub struct iceberg::io::AzblobConfig
pub iceberg::io::AzblobConfig::account_key: core::option::Option<alloc::string::String>
pub iceberg::io::AzblobConfig::account_name: core::option::Option<alloc::string::String>
pub iceberg::io::AzblobConfig::endpoint: core::option::Option<alloc::string::String>
pub iceberg::io::AzblobConfig::sas_token: core::option::Option<alloc::string::String>
impl core::clone::Clone for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::clone(&self) -> iceberg::io::AzblobConfig
impl core::cmp::Eq for iceberg::io::AzblobConfig
impl core::cmp::PartialEq for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::eq(&self, other: &iceberg::io::AzblobConfig) -> bool
impl core::convert::TryFrom<&iceberg::io::StorageConfig> for iceberg::io::AzblobConfig
pub type iceberg::io::AzblobConfig::Error = iceberg::Error
pub fn iceberg::io::AzblobConfig::try_from(config: &iceberg::io::StorageConfig) -> iceberg::Result<Self>
impl core::default::Default for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::default() -> iceberg::io::AzblobConfig
impl core::fmt::Debug for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
impl core::marker::StructuralPartialEq for iceberg::io::AzblobConfig
impl iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::builder() -> AzblobConfigBuilder<((), (), (), ())>
impl serde_core::ser::Serialize for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::serialize<__S>(&self, __serializer: __S) -> core::result::Result<<__S as serde_core::ser::Serializer>::Ok, <__S as serde_core::ser::Serializer>::Error> where __S: serde_core::ser::Serializer
impl<'de> serde_core::de::Deserialize<'de> for iceberg::io::AzblobConfig
pub fn iceberg::io::AzblobConfig::deserialize<__D>(__deserializer: __D) -> core::result::Result<Self, <__D as serde_core::de::Deserializer>::Error> where __D: serde_core::de::Deserializer<'de>
pub struct iceberg::io::AzdlsConfig
pub iceberg::io::AzdlsConfig::account_key: core::option::Option<alloc::string::String>
pub iceberg::io::AzdlsConfig::account_name: core::option::Option<alloc::string::String>
Expand Down Expand Up @@ -968,6 +992,9 @@ pub fn iceberg::io::StorageConfig::clone(&self) -> iceberg::io::StorageConfig
impl core::cmp::Eq for iceberg::io::StorageConfig
impl core::cmp::PartialEq for iceberg::io::StorageConfig
pub fn iceberg::io::StorageConfig::eq(&self, other: &iceberg::io::StorageConfig) -> bool
impl core::convert::TryFrom<&iceberg::io::StorageConfig> for iceberg::io::AzblobConfig
pub type iceberg::io::AzblobConfig::Error = iceberg::Error
pub fn iceberg::io::AzblobConfig::try_from(config: &iceberg::io::StorageConfig) -> iceberg::Result<Self>
impl core::convert::TryFrom<&iceberg::io::StorageConfig> for iceberg::io::AzdlsConfig
pub type iceberg::io::AzdlsConfig::Error = iceberg::Error
pub fn iceberg::io::AzdlsConfig::try_from(config: &iceberg::io::StorageConfig) -> iceberg::Result<Self>
Expand Down Expand Up @@ -1000,6 +1027,10 @@ pub const iceberg::io::ADLS_CLIENT_SECRET: &str
pub const iceberg::io::ADLS_CONNECTION_STRING: &str
pub const iceberg::io::ADLS_SAS_TOKEN: &str
pub const iceberg::io::ADLS_TENANT_ID: &str
pub const iceberg::io::AZBLOB_ACCOUNT_KEY: &str
pub const iceberg::io::AZBLOB_ACCOUNT_NAME: &str
pub const iceberg::io::AZBLOB_ENDPOINT: &str
pub const iceberg::io::AZBLOB_SAS_TOKEN: &str
pub const iceberg::io::CLIENT_REGION: &str
pub const iceberg::io::GCS_ALLOW_ANONYMOUS: &str
pub const iceberg::io::GCS_CREDENTIALS_JSON: &str
Expand Down
120 changes: 120 additions & 0 deletions crates/iceberg/src/io/storage/config/azblob.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

//! Azure Blob Storage configuration.
//!
//! This module provides configuration constants and types for Azure Blob Storage.

use serde::{Deserialize, Serialize};
use typed_builder::TypedBuilder;

use super::StorageConfig;
use crate::Result;

/// Azure Blob Storage endpoint URL.
pub const AZBLOB_ENDPOINT: &str = "azblob.endpoint";
/// Azure Blob Storage account name.
pub const AZBLOB_ACCOUNT_NAME: &str = "azblob.account-name";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't find a java implementation for azblob so that isn't incorrect but I did notice something that doesn't quite seem correct about the azdls config (pre-existing not this PR). It seems that we're using adls.account-name compared to java's adls.auth.shared-key.account.name which I think causes issues when we get the config back from the catalog for vended credentials.

Maybe we need to bottom this out before we commit to this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay Claude did a little audit for me and found that py-iceberg and java use GCS_SERVICE_HOST = "gcs.service.host" and we use gcs.service.path so it would seem we're inconsistent here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I took another look at this. The configuration required by Azblob is effectively the same as Azdls; the only difference is the property prefix (azblob.* vs. adls.*).

Would you prefer keeping the dedicated azblob.* keys, or following PyIceberg’s convention and reusing the existing adls.* keys?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suspect we do really want different clients to expect the same properties, having said that the fact that python accepts the same for both is maybe a little confusing to me but I’ve never used the older blob storage so I’m not sure how reasonable it is to couple the properties?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In both PyArrow and Arrow C++, the logic is to automatically detect whether HNS is enabled and then use different APIs.

For example, in azurefs.cc:

  ARROW_ASSIGN_OR_RAISE(
      auto hns_support,
      HierarchicalNamespaceSupport(adlfs_client));

However, OpenDAL cannot do this. Therefore, I think it is reasonable to keep the dedicated Azblob configuration here. The caller needs to explicitly specify whether the namespace is DLS or Blob.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the separate storage impl makes sense to me, I guess my question is just if we should reuse the same config keys or not. I'm leaning towards yes? @CTTY or @blackmwk might have stronger opinions here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just to confirm, I see two options:

  1. Keep the current PR keys: azblob.endpoint, azblob.account-name, azblob.account-key, and azblob.sas-token.
  2. Keep azblob.endpoint, but reuse adls.account-name, adls.account-key, and adls.sas-token for the shared Azure credentials.

The Azblob and Azdls implementations would remain separate in either case. It sounds like you are leaning toward the second option. Could you confirm that this is what you had in mind? If so, I’ll update the PR accordingly.

/// Azure Blob Storage account key.
pub const AZBLOB_ACCOUNT_KEY: &str = "azblob.account-key";
/// Azure Blob Storage shared access signature.
pub const AZBLOB_SAS_TOKEN: &str = "azblob.sas-token";

/// Azure Blob Storage configuration.
///
/// This struct contains all the configuration options for connecting to Azure Blob Storage.
/// Use the builder pattern via `AzblobConfig::builder()` to construct instances.
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, TypedBuilder)]
pub struct AzblobConfig {
/// Endpoint URL.
#[builder(default, setter(strip_option, into))]
pub endpoint: Option<String>,
/// Account name.
#[builder(default, setter(strip_option, into))]
pub account_name: Option<String>,
/// Account key.
#[builder(default, setter(strip_option, into))]
pub account_key: Option<String>,
/// SAS token.
#[builder(default, setter(strip_option, into))]
pub sas_token: Option<String>,
}

impl TryFrom<&StorageConfig> for AzblobConfig {
type Error = crate::Error;

fn try_from(config: &StorageConfig) -> Result<Self> {
let props = config.props();

let mut cfg = AzblobConfig::default();

if let Some(endpoint) = props.get(AZBLOB_ENDPOINT) {
cfg.endpoint = Some(endpoint.clone());
}
if let Some(account_name) = props.get(AZBLOB_ACCOUNT_NAME) {
cfg.account_name = Some(account_name.clone());
}
if let Some(account_key) = props.get(AZBLOB_ACCOUNT_KEY) {
cfg.account_key = Some(account_key.clone());
}
if let Some(sas_token) = props.get(AZBLOB_SAS_TOKEN) {
cfg.sas_token = Some(sas_token.clone());
}

Ok(cfg)
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_azblob_config_builder() {
let config = AzblobConfig::builder()
.endpoint("https://account.blob.core.windows.net")
.account_name("myaccount")
.account_key("my-account-key")
.build();

assert_eq!(
config.endpoint.as_deref(),
Some("https://account.blob.core.windows.net")
);
assert_eq!(config.account_name.as_deref(), Some("myaccount"));
assert_eq!(config.account_key.as_deref(), Some("my-account-key"));
}

#[test]
fn test_azblob_config_from_storage_config() {
let storage_config = StorageConfig::new()
.with_prop(AZBLOB_ENDPOINT, "https://account.blob.core.windows.net")
.with_prop(AZBLOB_ACCOUNT_NAME, "myaccount")
.with_prop(AZBLOB_ACCOUNT_KEY, "my-account-key")
.with_prop(AZBLOB_SAS_TOKEN, "my-sas-token");

let azblob_config = AzblobConfig::try_from(&storage_config).unwrap();

assert_eq!(
azblob_config.endpoint.as_deref(),
Some("https://account.blob.core.windows.net")
);
assert_eq!(azblob_config.account_name.as_deref(), Some("myaccount"));
assert_eq!(azblob_config.account_key.as_deref(), Some("my-account-key"));
assert_eq!(azblob_config.sas_token.as_deref(), Some("my-sas-token"));
}
}
2 changes: 2 additions & 0 deletions crates/iceberg/src/io/storage/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
//! - [`OssConfig`]: Alibaba Cloud OSS specific configuration
//! - [`AzdlsConfig`]: Azure Data Lake Storage specific configuration

mod azblob;
mod azdls;
mod gcs;
mod hf;
Expand All @@ -38,6 +39,7 @@ mod s3;

use std::collections::HashMap;

pub use azblob::*;
pub use azdls::*;
pub use gcs::*;
pub use hf::*;
Expand Down
2 changes: 2 additions & 0 deletions crates/storage/opendal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ opendal-all = [
"opendal-s3",
"opendal-gcs",
"opendal-oss",
"opendal-azblob",
"opendal-azdls",
"opendal-hf",
]

opendal-azblob = ["opendal/services-azblob"]
opendal-azdls = ["opendal/services-azdls"]
opendal-fs = ["opendal/services-fs"]
opendal-gcs = ["opendal/services-gcs"]
Expand Down
1 change: 1 addition & 0 deletions crates/storage/opendal/DEPENDENCIES.rust.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ opendal-layer-concurrent-limit@0.57.0 X
opendal-layer-logging@0.57.0 X
opendal-layer-retry@0.57.0 X
opendal-layer-timeout@0.57.0 X
opendal-service-azblob@0.57.0 X
opendal-service-azdls@0.57.0 X
opendal-service-azure-common@0.57.0 X
opendal-service-fs@0.57.0 X
Expand Down
1 change: 1 addition & 0 deletions crates/storage/opendal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ OpenDAL-based storage backend implementations for [Apache Iceberg Rust](https://
| Google Cloud Storage | `opendal-gcs` | ✅ Stable | Google Cloud Storage |
| Hugging Face | `opendal-hf` | ✅ Stable | Hugging Face buckets and repositories |
| Alibaba Cloud OSS | `opendal-oss` | 🧪 Experimental | Alibaba Cloud Object Storage Service |
| Azure Blob Storage | `opendal-azblob` | 🧪 Experimental | Azure Blob Storage without requiring HNS |
| Azure Datalake | `opendal-azdls` | 🧪 Experimental | Azure Datalake Storage v2 |

You can enable all stable storage backends at once using the `opendal-all` feature flag.
Expand Down
3 changes: 3 additions & 0 deletions crates/storage/opendal/public-api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ pub mod iceberg_storage_opendal
pub use iceberg_storage_opendal::AwsCredential
pub use iceberg_storage_opendal::ProvideCredential
pub enum iceberg_storage_opendal::OpenDalStorage
pub iceberg_storage_opendal::OpenDalStorage::Azblob
pub iceberg_storage_opendal::OpenDalStorage::Azblob::config: alloc::sync::Arc<opendal_service_azblob::config::AzblobConfig>
pub iceberg_storage_opendal::OpenDalStorage::Azdls
pub iceberg_storage_opendal::OpenDalStorage::Azdls::config: alloc::sync::Arc<opendal_service_azdls::config::AzdlsConfig>
pub iceberg_storage_opendal::OpenDalStorage::Gcs
Expand Down Expand Up @@ -36,6 +38,7 @@ pub fn iceberg_storage_opendal::OpenDalStorage::serialize<__S>(&self, __serializ
impl<'de> serde_core::de::Deserialize<'de> for iceberg_storage_opendal::OpenDalStorage
pub fn iceberg_storage_opendal::OpenDalStorage::deserialize<__D>(__deserializer: __D) -> core::result::Result<Self, <__D as serde_core::de::Deserializer>::Error> where __D: serde_core::de::Deserializer<'de>
pub enum iceberg_storage_opendal::OpenDalStorageFactory
pub iceberg_storage_opendal::OpenDalStorageFactory::Azblob
pub iceberg_storage_opendal::OpenDalStorageFactory::Azdls
pub iceberg_storage_opendal::OpenDalStorageFactory::Fs
pub iceberg_storage_opendal::OpenDalStorageFactory::Gcs
Expand Down
Loading
Loading