From b6bd80b14d12c51713ff059c48ff5a545d9606a4 Mon Sep 17 00:00:00 2001 From: Will Jones Date: Sat, 11 Feb 2023 10:25:08 -0800 Subject: [PATCH] fix env handling --- rust/README.md | 4 +- rust/src/delta.rs | 2 +- rust/src/lib.rs | 8 ++-- rust/src/storage/config.rs | 50 +++++++-------------- rust/src/storage/mod.rs | 2 +- rust/tests/command_filesystem_check.rs | 2 +- rust/tests/common/mod.rs | 4 +- rust/tests/integration_checkpoint.rs | 2 +- rust/tests/integration_commit.rs | 5 +-- rust/tests/integration_concurrent_writes.rs | 2 - rust/tests/integration_datafusion.rs | 2 +- rust/tests/integration_object_store.rs | 2 - rust/tests/integration_read.rs | 10 ++--- rust/tests/repair_s3_rename_test.rs | 2 +- 14 files changed, 36 insertions(+), 61 deletions(-) diff --git a/rust/README.md b/rust/README.md index e7e0ff5dcd..0ea3e4976f 100644 --- a/rust/README.md +++ b/rust/README.md @@ -41,8 +41,8 @@ cargo run --example read_delta_table ## Optional cargo package features -- `s3` - enable the S3 storage backend to work with Delta Tables in AWS S3. -- `s3-rustls` - enable the S3 storage backend but rely on [rustls](https://github.com/ctz/rustls) rather than OpenSSL (`native-tls`). +- `s3` - enable the S3 storage backend to work with Delta Tables in AWS S3. Uses [rustls](https://github.com/ctz/rustls). +- `s3-native-tls` - enable the S3 storage backend but rely on OpenSSL. - `glue` - enable the Glue data catalog to work with Delta Tables with AWS Glue. - `azure` - enable the Azure storage backend to work with Delta Tables in Azure Data Lake Storage Gen2 accounts. - `gcs` - enable the Google storage backend to work with Delta Tables in Google Cloud Storage. diff --git a/rust/src/delta.rs b/rust/src/delta.rs index f3b95fe899..783cca2026 100644 --- a/rust/src/delta.rs +++ b/rust/src/delta.rs @@ -1556,7 +1556,7 @@ mod tests { drop(tmp_dir); } - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[test] fn normalize_table_uri_s3() { std::env::set_var("AWS_DEFAULT_REGION", "us-east-1"); diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d90692470f..9784010259 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -38,8 +38,8 @@ //! # Optional cargo package features //! //! - `s3`, `gcs`, `azure` - enable the storage backends for AWS S3, Google Cloud Storage (GCS), -//! or Azure Blob Storage / Azure Data Lake Storage Gen2 (ADLS2). Use `s3-rustls` to use Rust TLS -//! instead of native TLS implementation. +//! or Azure Blob Storage / Azure Data Lake Storage Gen2 (ADLS2). Use `s3-native-tls` to use native TLS +//! instead of Rust TLS implementation. //! - `glue` - enable the Glue data catalog to work with Delta Tables with AWS Glue. //! - `datafusion` - enable the `datafusion::datasource::TableProvider` trait implementation //! for Delta Tables, allowing them to be queried using [DataFusion](https://github.com/apache/arrow-datafusion). @@ -77,8 +77,8 @@ compile_error!( "Features parquet and parquet2 are mutually exclusive and cannot be enabled together" ); -#[cfg(all(feature = "s3", feature = "s3-rustls"))] -compile_error!("Features s3 and s3-rustls are mutually exclusive and cannot be enabled together"); +#[cfg(all(feature = "s3", feature = "s3-native-tls"))] +compile_error!("Features s3 and s3-native-tls are mutually exclusive and cannot be enabled together"); pub mod action; pub mod builder; diff --git a/rust/src/storage/config.rs b/rust/src/storage/config.rs index cc6833daa4..1f4eb999a1 100644 --- a/rust/src/storage/config.rs +++ b/rust/src/storage/config.rs @@ -11,9 +11,9 @@ use std::collections::HashMap; use std::sync::Arc; use url::Url; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] use super::s3::{S3StorageBackend, S3StorageOptions}; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] use object_store::aws::{AmazonS3Builder, AmazonS3ConfigKey}; #[cfg(feature = "azure")] use object_store::azure::{AzureConfigKey, MicrosoftAzure, MicrosoftAzureBuilder}; @@ -21,7 +21,7 @@ use object_store::azure::{AzureConfigKey, MicrosoftAzure, MicrosoftAzureBuilder} use object_store::gcp::{GoogleCloudStorage, GoogleCloudStorageBuilder, GoogleConfigKey}; #[cfg(any( feature = "s3", - feature = "s3-rustls", + feature = "s3-native-tls", feature = "gcs", feature = "azure" ))] @@ -67,7 +67,7 @@ impl StorageOptions { } /// Subset of options relevant for s3 storage - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] pub fn as_s3_options(&self) -> HashMap { self.0 .iter() @@ -100,7 +100,7 @@ impl From> for StorageOptions { pub(crate) enum ObjectStoreImpl { Local(FileStorageBackend), InMemory(InMemory), - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] S3(S3StorageBackend), #[cfg(feature = "gcs")] Google(GoogleCloudStorage), @@ -115,7 +115,7 @@ impl ObjectStoreImpl { ObjectStoreImpl::InMemory(store) => Arc::new(PrefixObjectStore::new(store, prefix)), #[cfg(feature = "azure")] ObjectStoreImpl::Azure(store) => Arc::new(PrefixObjectStore::new(store, prefix)), - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] ObjectStoreImpl::S3(store) => Arc::new(PrefixObjectStore::new(store, prefix)), #[cfg(feature = "gcs")] ObjectStoreImpl::Google(store) => Arc::new(PrefixObjectStore::new(store, prefix)), @@ -128,7 +128,7 @@ impl ObjectStoreImpl { ObjectStoreImpl::InMemory(store) => Arc::new(store), #[cfg(feature = "azure")] ObjectStoreImpl::Azure(store) => Arc::new(store), - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] ObjectStoreImpl::S3(store) => Arc::new(store), #[cfg(feature = "gcs")] ObjectStoreImpl::Google(store) => Arc::new(store), @@ -183,44 +183,30 @@ impl ObjectStoreKind { match self { ObjectStoreKind::Local => Ok(ObjectStoreImpl::Local(FileStorageBackend::new())), ObjectStoreKind::InMemory => Ok(ObjectStoreImpl::InMemory(InMemory::new())), - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] ObjectStoreKind::S3 => { - let store = AmazonS3Builder::new() + let store = AmazonS3Builder::from_env() .with_url(storage_url.as_ref()) .try_with_options(&_options.as_s3_options())? .with_allow_http(_options.allow_http()) - .build() - .or_else(|_| { - AmazonS3Builder::from_env() - .with_url(storage_url.as_ref()) - .try_with_options(&_options.as_s3_options())? - .with_allow_http(_options.allow_http()) - .build() - })?; + .build()?; Ok(ObjectStoreImpl::S3(S3StorageBackend::try_new( Arc::new(store), S3StorageOptions::from_map(&_options.0), )?)) } - #[cfg(not(any(feature = "s3", feature = "s3-rustls")))] + #[cfg(not(any(feature = "s3", feature = "s3-native-tls")))] ObjectStoreKind::S3 => Err(DeltaTableError::MissingFeature { feature: "s3", url: storage_url.as_ref().into(), }), #[cfg(feature = "azure")] ObjectStoreKind::Azure => { - let store = MicrosoftAzureBuilder::new() + let store = MicrosoftAzureBuilder::from_env() .with_url(storage_url.as_ref()) .try_with_options(&_options.as_azure_options())? .with_allow_http(_options.allow_http()) - .build() - .or_else(|_| { - MicrosoftAzureBuilder::from_env() - .with_url(storage_url.as_ref()) - .try_with_options(&_options.as_azure_options())? - .with_allow_http(_options.allow_http()) - .build() - })?; + .build()?; Ok(ObjectStoreImpl::Azure(store)) } #[cfg(not(feature = "azure"))] @@ -230,16 +216,10 @@ impl ObjectStoreKind { }), #[cfg(feature = "gcs")] ObjectStoreKind::Google => { - let store = GoogleCloudStorageBuilder::new() + let store = GoogleCloudStorageBuilder::from_env() .with_url(storage_url.as_ref()) .try_with_options(&_options.as_gcs_options())? - .build() - .or_else(|_| { - GoogleCloudStorageBuilder::from_env() - .with_url(storage_url.as_ref()) - .try_with_options(&_options.as_gcs_options())? - .build() - })?; + .build()?; Ok(ObjectStoreImpl::Google(store)) } #[cfg(not(feature = "gcs"))] diff --git a/rust/src/storage/mod.rs b/rust/src/storage/mod.rs index 14a8240729..37ec6ae899 100644 --- a/rust/src/storage/mod.rs +++ b/rust/src/storage/mod.rs @@ -20,7 +20,7 @@ use std::sync::Arc; use tokio::io::AsyncWrite; use url::Url; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] pub mod s3; #[cfg(feature = "datafusion")] diff --git a/rust/tests/command_filesystem_check.rs b/rust/tests/command_filesystem_check.rs index 602371f99f..afc6297640 100644 --- a/rust/tests/command_filesystem_check.rs +++ b/rust/tests/command_filesystem_check.rs @@ -15,7 +15,7 @@ async fn test_filesystem_check_local() -> TestResult { Ok(test_filesystem_check(StorageIntegration::Local).await?) } -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn test_filesystem_check_aws() -> TestResult { diff --git a/rust/tests/common/mod.rs b/rust/tests/common/mod.rs index cd1758bffe..439bf911b6 100644 --- a/rust/tests/common/mod.rs +++ b/rust/tests/common/mod.rs @@ -18,7 +18,7 @@ pub mod adls; pub mod clock; #[cfg(feature = "datafusion")] pub mod datafusion; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] pub mod s3; pub mod schemas; @@ -45,7 +45,7 @@ impl TestContext { Ok("LOCALFS") | Err(std::env::VarError::NotPresent) => setup_local_context().await, #[cfg(feature = "azure")] Ok("AZURE_GEN2") => adls::setup_azure_gen2_context().await, - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] Ok("S3_LOCAL_STACK") => s3::setup_s3_context().await, _ => panic!("Invalid backend for delta-rs tests"), } diff --git a/rust/tests/integration_checkpoint.rs b/rust/tests/integration_checkpoint.rs index 4d65eb6145..aeeb4c38d1 100644 --- a/rust/tests/integration_checkpoint.rs +++ b/rust/tests/integration_checkpoint.rs @@ -13,7 +13,7 @@ async fn cleanup_metadata_fs_test() -> TestResult { Ok(()) } -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn cleanup_metadata_aws_test() -> TestResult { diff --git a/rust/tests/integration_commit.rs b/rust/tests/integration_commit.rs index 9a1c3a8c81..5112f65c77 100644 --- a/rust/tests/integration_commit.rs +++ b/rust/tests/integration_commit.rs @@ -14,8 +14,7 @@ async fn test_commit_tables_local() { commit_tables(StorageIntegration::Local).await.unwrap(); } -// rustls doesn't support http scheme, so we are skipping the test when s3-rustls is enabled. -#[cfg(feature = "s3")] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn test_commit_tables_aws() { @@ -37,7 +36,7 @@ async fn test_commit_tables_gcp() { commit_tables(StorageIntegration::Google).await.unwrap(); } -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn test_two_commits_s3_fails_with_no_lock() -> TestResult { diff --git a/rust/tests/integration_concurrent_writes.rs b/rust/tests/integration_concurrent_writes.rs index d13f7d1d9b..32f0493615 100644 --- a/rust/tests/integration_concurrent_writes.rs +++ b/rust/tests/integration_concurrent_writes.rs @@ -15,8 +15,6 @@ async fn test_concurrent_writes_local() -> TestResult { Ok(()) } -// rustls doesn't support http scheme, so we are skipping the test when s3-rustls is enabled. -#[cfg(feature = "s3")] #[tokio::test] async fn concurrent_writes_s3() -> TestResult { test_concurrent_writes(StorageIntegration::Amazon).await?; diff --git a/rust/tests/integration_datafusion.rs b/rust/tests/integration_datafusion.rs index 9fb9e92125..7a3fdde464 100644 --- a/rust/tests/integration_datafusion.rs +++ b/rust/tests/integration_datafusion.rs @@ -14,7 +14,7 @@ async fn test_datafusion_local() -> TestResult { Ok(test_datafusion(StorageIntegration::Local).await?) } -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn test_datafusion_aws() -> TestResult { diff --git a/rust/tests/integration_object_store.rs b/rust/tests/integration_object_store.rs index fe2e688c4c..9c32ec0e59 100644 --- a/rust/tests/integration_object_store.rs +++ b/rust/tests/integration_object_store.rs @@ -22,8 +22,6 @@ async fn test_object_store_azure() -> TestResult { Ok(()) } -// rustls doesn't support http scheme, so we are skipping the test when s3-rustls is enabled. -#[cfg(feature = "s3")] #[tokio::test] #[serial] async fn test_object_store_aws() -> TestResult { diff --git a/rust/tests/integration_read.rs b/rust/tests/integration_read.rs index 32d25d454c..4b7531441d 100644 --- a/rust/tests/integration_read.rs +++ b/rust/tests/integration_read.rs @@ -2,9 +2,9 @@ use deltalake::test_utils::{IntegrationContext, StorageIntegration, TestResult, TestTables}; use deltalake::DeltaTableBuilder; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] use dynamodb_lock::dynamo_lock_options; -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] use maplit::hashmap; use object_store::path::Path; use serial_test::serial; @@ -22,7 +22,7 @@ async fn test_read_tables_azure() -> TestResult { Ok(read_tables(StorageIntegration::Microsoft).await?) } -#[cfg(any(feature = "s3", feature = "s3-rustls"))] +#[cfg(any(feature = "s3", feature = "s3-native-tls"))] #[tokio::test] #[serial] async fn test_read_tables_aws() -> TestResult { @@ -44,11 +44,11 @@ async fn read_tables(storage: StorageIntegration) -> TestResult { async fn read_simple_table(integration: &IntegrationContext) -> TestResult { let table_uri = integration.uri_for_table(TestTables::Simple); // the s3 options don't hurt us for other integrations ... - #[cfg(any(feature = "s3", feature = "s3-rustls"))] + #[cfg(any(feature = "s3", feature = "s3-native-tls"))] let table = DeltaTableBuilder::from_uri(table_uri).with_allow_http(true).with_storage_options(hashmap! { dynamo_lock_options::DYNAMO_LOCK_OWNER_NAME.to_string() => "s3::deltars/simple".to_string(), }).load().await?; - #[cfg(not(any(feature = "s3", feature = "s3-rustls")))] + #[cfg(not(any(feature = "s3", feature = "s3-native-tls")))] let table = DeltaTableBuilder::from_uri(table_uri) .with_allow_http(true) .load() diff --git a/rust/tests/repair_s3_rename_test.rs b/rust/tests/repair_s3_rename_test.rs index 3ae924ba66..91e0644d3e 100644 --- a/rust/tests/repair_s3_rename_test.rs +++ b/rust/tests/repair_s3_rename_test.rs @@ -1,4 +1,4 @@ -#![cfg(all(feature = "s3", feature = "s3-rustls", feature = "integration_test"))] +#![cfg(all(any(feature = "s3", feature = "s3-native-tls"), feature = "integration_test"))] use bytes::Bytes; use deltalake::storage::s3::S3StorageOptions; use deltalake::test_utils::{IntegrationContext, StorageIntegration};