From d914722f793e0536eb9729c9f121740f99f7b7b7 Mon Sep 17 00:00:00 2001 From: Ryan Green Date: Fri, 29 Nov 2024 11:06:18 -0330 Subject: [PATCH] Revert "feat: support remote options for remote lancedb connection. Send Azure storage account name via HTTP header." This reverts commit a6e4034dbaa5ae06c13495ab7192c9f2271a330a. --- Cargo.toml | 1 - rust/lancedb/Cargo.toml | 1 - rust/lancedb/src/connection.rs | 6 +--- rust/lancedb/src/remote/client.rs | 25 --------------- rust/lancedb/src/remote/db.rs | 53 ++----------------------------- 5 files changed, 3 insertions(+), 83 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1c529852..4a4d667f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ rust-version = "1.80.0" # TO lance = { "version" = "=0.20.0", "features" = [ "dynamodb", ], git = "https://github.com/lancedb/lance.git", tag = "v0.20.0-beta.3" } -lance-io = { version = "=0.20.0", git = "https://github.com/lancedb/lance.git", tag = "v0.20.0-beta.3" } lance-index = { version = "=0.20.0", git = "https://github.com/lancedb/lance.git", tag = "v0.20.0-beta.3" } lance-linalg = { version = "=0.20.0", git = "https://github.com/lancedb/lance.git", tag = "v0.20.0-beta.3" } lance-table = { version = "=0.20.0", git = "https://github.com/lancedb/lance.git", tag = "v0.20.0-beta.3" } diff --git a/rust/lancedb/Cargo.toml b/rust/lancedb/Cargo.toml index c5308672..2ac09fea 100644 --- a/rust/lancedb/Cargo.toml +++ b/rust/lancedb/Cargo.toml @@ -27,7 +27,6 @@ half = { workspace = true } lazy_static.workspace = true lance = { workspace = true } lance-datafusion.workspace = true -lance-io = { workspace = true } lance-index = { workspace = true } lance-table = { workspace = true } lance-linalg = { workspace = true } diff --git a/rust/lancedb/src/connection.rs b/rust/lancedb/src/connection.rs index 76632a97..f7e17b39 100644 --- a/rust/lancedb/src/connection.rs +++ b/rust/lancedb/src/connection.rs @@ -38,8 +38,6 @@ use crate::table::{NativeTable, TableDefinition, WriteOptions}; use crate::utils::validate_table_name; use crate::Table; pub use lance_encoding::version::LanceFileVersion; -#[cfg(feature = "remote")] -use lance_io::object_store::StorageOptions; use lance_table::io::commit::commit_handler_from_url; pub const LANCE_FILE_EXTENSION: &str = "lance"; @@ -720,14 +718,12 @@ impl ConnectBuilder { message: "An api_key is required when connecting to LanceDb Cloud".to_string(), })?; - let mut storage_options = StorageOptions(self.storage_options.clone()); let internal = Arc::new(crate::remote::db::RemoteDatabase::try_new( &self.uri, &api_key, ®ion, self.host_override, self.client_config, - storage_options.into(), )?); Ok(Connection { internal, @@ -860,7 +856,7 @@ impl Database { let table_base_uri = if let Some(store) = engine { static WARN_ONCE: std::sync::Once = std::sync::Once::new(); WARN_ONCE.call_once(|| { - log::warn!("Specifying engine is not a publicly supported feature in lancedb yet. THE API WILL CHANGE"); + log::warn!("Specifing engine is not a publicly supported feature in lancedb yet. THE API WILL CHANGE"); }); let old_scheme = url.scheme().to_string(); let new_scheme = format!("{}+{}", old_scheme, store); diff --git a/rust/lancedb/src/remote/client.rs b/rust/lancedb/src/remote/client.rs index a1b0ac74..48c8aa1c 100644 --- a/rust/lancedb/src/remote/client.rs +++ b/rust/lancedb/src/remote/client.rs @@ -21,7 +21,6 @@ use reqwest::{ }; use crate::error::{Error, Result}; -use crate::remote::db::RemoteOptions; const REQUEST_ID_HEADER: &str = "x-request-id"; @@ -216,7 +215,6 @@ impl RestfulLanceDbClient { region: &str, host_override: Option, client_config: ClientConfig, - options: &RemoteOptions, ) -> Result { let parsed_url = url::Url::parse(db_url).map_err(|err| Error::InvalidInput { message: format!("db_url is not a valid URL. '{db_url}'. Error: {err}"), @@ -257,7 +255,6 @@ impl RestfulLanceDbClient { region, db_name, host_override.is_some(), - options, )?) .user_agent(client_config.user_agent) .build() @@ -265,8 +262,6 @@ impl RestfulLanceDbClient { message: "Failed to build HTTP client".into(), source: Some(Box::new(err)), })?; - println!("{:#?}", client); - let host = match host_override { Some(host_override) => host_override, None => format!("https://{}.{}.api.lancedb.com", db_name, region), @@ -292,7 +287,6 @@ impl RestfulLanceDbClient { region: &str, db_name: &str, has_host_override: bool, - options: &RemoteOptions, ) -> Result { let mut headers = HeaderMap::new(); headers.insert( @@ -319,25 +313,6 @@ impl RestfulLanceDbClient { ); } - if let Some(v) = options.0.get("account_name") { - headers.insert( - "x-azure-storage-account-name", - HeaderValue::from_str(v).map_err(|_| Error::InvalidInput { - message: format!("non-ascii storage account name '{}' provided", db_name), - })?, - ); - } - if let Some(v) = options.0.get("azure_storage_account_name") { - headers.insert( - "x-azure-storage-account-name", - HeaderValue::from_str(v).map_err(|_| Error::InvalidInput { - message: format!("non-ascii storage account name '{}' provided", db_name), - })?, - ); - } - - // todo: db prefix - Ok(headers) } diff --git a/rust/lancedb/src/remote/db.rs b/rust/lancedb/src/remote/db.rs index 6bd73fd2..fc10fbdb 100644 --- a/rust/lancedb/src/remote/db.rs +++ b/rust/lancedb/src/remote/db.rs @@ -12,13 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashMap; use std::sync::Arc; use arrow_array::RecordBatchReader; use async_trait::async_trait; use http::StatusCode; -use lance_io::object_store::StorageOptions; use moka::future::Cache; use reqwest::header::CONTENT_TYPE; use serde::Deserialize; @@ -46,7 +44,6 @@ struct ListTablesResponse { pub struct RemoteDatabase { client: RestfulLanceDbClient, table_cache: Cache, - options: RemoteOptions, } impl RemoteDatabase { @@ -56,16 +53,9 @@ impl RemoteDatabase { region: &str, host_override: Option, client_config: ClientConfig, - options: RemoteOptions, ) -> Result { - let client = RestfulLanceDbClient::try_new( - uri, - api_key, - region, - host_override, - client_config, - &options, - )?; + let client = + RestfulLanceDbClient::try_new(uri, api_key, region, host_override, client_config)?; let table_cache = Cache::builder() .time_to_live(std::time::Duration::from_secs(300)) @@ -75,7 +65,6 @@ impl RemoteDatabase { Ok(Self { client, table_cache, - options, }) } } @@ -96,7 +85,6 @@ mod test_utils { Self { client, table_cache: Cache::new(0), - options: RemoteOptions::default(), } } } @@ -255,29 +243,6 @@ impl ConnectionInternal for RemoteDatabase { } } -/// A subset of StorageOptions that are compatible with Remote LanceDB client -#[derive(Clone, Debug, Default)] -pub struct RemoteOptions(pub HashMap); - -impl RemoteOptions { - pub fn new(options: HashMap) -> Self { - Self(options) - } -} - -impl From for RemoteOptions { - fn from(options: StorageOptions) -> Self { - let supported_opts = vec!["account_name", "azure_storage_account_name"]; - let mut filtered = HashMap::new(); - for opt in supported_opts { - if let Some(v) = options.0.get(opt) { - filtered.insert(opt.to_string(), v.to_string()); - } - } - RemoteOptions::new(filtered) - } -} - #[cfg(test)] mod tests { use std::sync::{Arc, OnceLock}; @@ -285,8 +250,6 @@ mod tests { use arrow_array::{Int32Array, RecordBatch, RecordBatchIterator}; use arrow_schema::{DataType, Field, Schema}; - use crate::connection::ConnectBuilder; - use crate::remote::client::test_utils::client_with_handler; use crate::{ connection::CreateTableMode, remote::{ARROW_STREAM_CONTENT_TYPE, JSON_CONTENT_TYPE}, @@ -578,16 +541,4 @@ mod tests { }); conn.rename_table("table1", "table2").await.unwrap(); } - - #[tokio::test] - async fn test_connect_remote_options() { - let db_uri = "db://my-container/my-prefix"; - let _ = ConnectBuilder::new(db_uri) - .region("us-east-1") - .api_key("my-api-key") - .storage_options(vec![("azure_storage_account_name", "my-storage-account")]) - .execute() - .await - .unwrap(); - } }