diff --git a/core/src/docs/internals/accessor.rs b/core/src/docs/internals/accessor.rs index 8a14ed4898a..3158cfdbfa4 100644 --- a/core/src/docs/internals/accessor.rs +++ b/core/src/docs/internals/accessor.rs @@ -214,15 +214,22 @@ //! /// Ok(()) //! /// } //! /// ``` +//! #[derive(Default, Serialize, Deserialize, Clone, PartialEq, Eq)] +//! #[serde(default)] +//! #[non_exhaustive] +//! pub struct DuckConfig { +//! pub root: Option, +//! } +//! //! #[derive(Default, Clone)] //! pub struct DuckBuilder { -//! root: Option, +//! config: DuckConfig, //! } //! ``` //! //! Note that `DuckBuilder` is part of our public API, so it needs to be //! documented. And any changes you make will directly affect users, so -//! please take it seriously. Otherwise you will be hunted down by many +//! please take it seriously. Otherwise, you will be hunted down by many //! angry ducks. //! //! Then, we can implement required APIs for `DuckBuilder`: @@ -233,7 +240,7 @@ //! /// //! /// All operations will happen under this root. //! pub fn root(&mut self, root: &str) -> &mut Self { -//! self.root = if root.is_empty() { +//! self.config.root = if root.is_empty() { //! None //! } else { //! Some(root.to_string()) @@ -246,19 +253,16 @@ //! impl Builder for DuckBuilder { //! const SCHEME: Scheme = Scheme::Duck; //! type Accessor = DuckBackend; +//! type Config = DuckConfig; //! -//! fn from_map(map: HashMap) -> Self { -//! let mut builder = DuckBuilder::default(); -//! -//! map.get("root").map(|v| builder.root(v)); -//! -//! builder +//! fn from_config(config: Self::Config) -> Self { +//! DuckBuilder { config } //! } //! //! fn build(&mut self) -> Result { //! debug!("backend build started: {:?}", &self); //! -//! let root = normalize_root(&self.root.clone().unwrap_or_default()); +//! let root = normalize_root(&self.config.root.clone().unwrap_or_default()); //! debug!("backend use root {}", &root); //! //! Ok(DuckBackend { root }) diff --git a/core/src/layers/immutable_index.rs b/core/src/layers/immutable_index.rs index 1889878c974..d0b13d2718b 100644 --- a/core/src/layers/immutable_index.rs +++ b/core/src/layers/immutable_index.rs @@ -264,12 +264,12 @@ mod tests { iil.insert(i.to_string()) } - let op = Operator::new(Http::from_map({ + let op = Http::from_map({ let mut map = HashMap::new(); map.insert("endpoint".to_string(), "https://xuanwo.io".to_string()); - map - }))? + }) + .and_then(Operator::new)? .layer(LoggingLayer::default()) .layer(iil) .finish(); @@ -302,12 +302,12 @@ mod tests { iil.insert(i.to_string()) } - let op = Operator::new(Http::from_map({ + let op = Http::from_map({ let mut map = HashMap::new(); map.insert("endpoint".to_string(), "https://xuanwo.io".to_string()); - map - }))? + }) + .and_then(Operator::new)? .layer(LoggingLayer::default()) .layer(iil) .finish(); @@ -346,12 +346,12 @@ mod tests { iil.insert(i.to_string()) } - let op = Operator::new(Http::from_map({ + let op = Http::from_map({ let mut map = HashMap::new(); map.insert("endpoint".to_string(), "https://xuanwo.io".to_string()); - map - }))? + }) + .and_then(Operator::new)? .layer(LoggingLayer::default()) .layer(iil) .finish(); @@ -404,12 +404,12 @@ mod tests { iil.insert(i.to_string()) } - let op = Operator::new(Http::from_map({ + let op = Http::from_map({ let mut map = HashMap::new(); map.insert("endpoint".to_string(), "https://xuanwo.io".to_string()); - map - }))? + }) + .and_then(Operator::new)? .layer(LoggingLayer::default()) .layer(iil) .finish(); diff --git a/core/src/layers/retry.rs b/core/src/layers/retry.rs index be11eb1abaa..4ccc616b61f 100644 --- a/core/src/layers/retry.rs +++ b/core/src/layers/retry.rs @@ -756,7 +756,6 @@ impl oio::BlockingList for RetryWrapp #[cfg(test)] mod tests { - use std::collections::HashMap; use std::sync::Arc; use std::sync::Mutex; @@ -782,10 +781,6 @@ mod tests { Self::default() } - fn from_map(_: HashMap) -> Self { - Self::default() - } - fn build(&mut self) -> Result { Ok(MockService { attempt: self.attempt.clone(), diff --git a/core/src/services/alluxio/backend.rs b/core/src/services/alluxio/backend.rs index 59c927fb540..8471d84a5be 100644 --- a/core/src/services/alluxio/backend.rs +++ b/core/src/services/alluxio/backend.rs @@ -259,7 +259,7 @@ mod test { map.insert("root".to_string(), "/".to_string()); map.insert("endpoint".to_string(), "http://127.0.0.1:39999".to_string()); - let builder = AlluxioBuilder::from_map(map); + let builder = AlluxioBuilder::from_map(map).unwrap(); assert_eq!(builder.config.root, Some("/".to_string())); assert_eq!( diff --git a/core/src/types/builder.rs b/core/src/types/builder.rs index b7075916180..123377e0762 100644 --- a/core/src/types/builder.rs +++ b/core/src/types/builder.rs @@ -48,10 +48,14 @@ pub trait Builder: Default { fn from_config(config: Self::Config) -> Self; /// Construct a builder from given map which contains several parameters needed by underlying service. - fn from_map(map: HashMap) -> Self { - Self::Config::deserialize(ConfigDeserializer::new(map)) - .map(Self::from_config) - .expect("config deserialize must succeed") + fn from_map(map: HashMap) -> Result { + match Self::Config::deserialize(ConfigDeserializer::new(map)) { + Ok(config) => Ok(Self::from_config(config)), + Err(err) => Err( + Error::new(ErrorKind::ConfigInvalid, "failed to deserialize config") + .set_source(err), + ), + } } /// Consume the accessor builder to build a service. @@ -68,8 +72,6 @@ impl Builder for () { fn from_config(_: Self::Config) -> Self {} - fn from_map(_: HashMap) -> Self {} - fn build(&mut self) -> Result { Ok(()) } diff --git a/core/src/types/operator/builder.rs b/core/src/types/operator/builder.rs index 562cabf0fb0..7f0f1480dd4 100644 --- a/core/src/types/operator/builder.rs +++ b/core/src/types/operator/builder.rs @@ -112,7 +112,7 @@ impl Operator { pub fn from_map( map: HashMap, ) -> Result> { - let acc = B::from_map(map).build()?; + let acc = B::from_map(map)?.build()?; Ok(OperatorBuilder::new(acc)) } diff --git a/integrations/object_store/README.md b/integrations/object_store/README.md index 24465ed95bb..80d56235841 100644 --- a/integrations/object_store/README.md +++ b/integrations/object_store/README.md @@ -53,7 +53,7 @@ async fn main() { ] .into_iter() .collect(), - ); + ).unwrap(); // Create a new operator let operator = Operator::new(builder).unwrap().finish(); diff --git a/integrations/object_store/examples/basic.rs b/integrations/object_store/examples/basic.rs index 3f8085c482b..c2cccc5cae8 100644 --- a/integrations/object_store/examples/basic.rs +++ b/integrations/object_store/examples/basic.rs @@ -18,7 +18,8 @@ async fn main() { ] .into_iter() .collect(), - ); + ) + .unwrap(); // Create a new operator let operator = Operator::new(builder).unwrap().finish(); diff --git a/integrations/object_store/src/lib.rs b/integrations/object_store/src/lib.rs index 9b2d691655d..37b46d171bb 100644 --- a/integrations/object_store/src/lib.rs +++ b/integrations/object_store/src/lib.rs @@ -40,7 +40,7 @@ //! ] //! .into_iter() //! .collect(), -//! ); +//! ).unwrap(); //! //! // Create a new operator //! let operator = Operator::new(builder).unwrap().finish(); diff --git a/integrations/object_store/src/store.rs b/integrations/object_store/src/store.rs index daa2fcced66..420bd2322f4 100644 --- a/integrations/object_store/src/store.rs +++ b/integrations/object_store/src/store.rs @@ -68,7 +68,7 @@ use std::sync::Arc; /// ] /// .into_iter() /// .collect(), -/// ); +/// ).unwrap(); /// /// // Create a new operator /// let operator = Operator::new(builder).unwrap().finish();