Skip to content

Commit

Permalink
fix env handling
Browse files Browse the repository at this point in the history
  • Loading branch information
wjones127 committed Feb 11, 2023
1 parent 8e07cc6 commit b6bd80b
Show file tree
Hide file tree
Showing 14 changed files with 36 additions and 61 deletions.
4 changes: 2 additions & 2 deletions rust/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion rust/src/delta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
8 changes: 4 additions & 4 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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;
Expand Down
50 changes: 15 additions & 35 deletions rust/src/storage/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ 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};
#[cfg(feature = "gcs")]
use object_store::gcp::{GoogleCloudStorage, GoogleCloudStorageBuilder, GoogleConfigKey};
#[cfg(any(
feature = "s3",
feature = "s3-rustls",
feature = "s3-native-tls",
feature = "gcs",
feature = "azure"
))]
Expand Down Expand Up @@ -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<AmazonS3ConfigKey, String> {
self.0
.iter()
Expand Down Expand Up @@ -100,7 +100,7 @@ impl From<HashMap<String, String>> 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),
Expand All @@ -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)),
Expand All @@ -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),
Expand Down Expand Up @@ -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"))]
Expand All @@ -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"))]
Expand Down
2 changes: 1 addition & 1 deletion rust/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion rust/tests/command_filesystem_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions rust/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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"),
}
Expand Down
2 changes: 1 addition & 1 deletion rust/tests/integration_checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions rust/tests/integration_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions rust/tests/integration_concurrent_writes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Expand Down
2 changes: 1 addition & 1 deletion rust/tests/integration_datafusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions rust/tests/integration_object_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions rust/tests/integration_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion rust/tests/repair_s3_rename_test.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down

0 comments on commit b6bd80b

Please sign in to comment.