Yansongsongsong commented on code in PR #2094:
URL:
https://github.com/apache/incubator-opendal/pull/2094#discussion_r1175268093
##########
core/src/docs/concepts.rs:
##########
@@ -53,15 +61,20 @@
//! ```
//!
//! # Operator
-//!
-//! The [`Operator`] is a bridge between the underlying implementation details
and the unified abstraction. OpenDAL will erase all generic types and higher
abstraction around it.
+//! The [`Operator`] is a delegate for underlying implementation detail, and
provides one unified access interface.
+//! It will hold one reference of Service with its all generic types erased by
OpenDAL, which is the reason why we
+//! say the Operator is the delegate of one Service.
+//! And the Service also has another name called
[`Accessor`][crate::raw::Accessor].
//!
//! ```text
-//! ┌───────────┐ ┌───────────┐ ┌─────────────────┐
-//! │ │ build() │ │ type erase │ │
-//! │ Builder ├──────────►│ Service ├─────────────►│ Operator │
-//! │ │ │ │ │ │
-//! └───────────┘ └───────────┘ └─────────────────┘
+//! ┌─────────────────────────────┐
Review Comment:
sorry for misleading. here I just drafted it from a very rough view of API
usage.
`Operator` does accept one `Builder` when calling `new()` method to create
one operator.
```rust
impl Operator {
#[allow(clippy::new_ret_no_self)]
pub fn new<B: Builder>(mut ab: B) -> Result<OperatorBuilder<impl
Accessor>> {
let acc = ab.build()?;
Ok(OperatorBuilder::new(acc))
}
}
```
remove the misleading part. thank you to point it out.
##########
core/src/docs/concepts.rs:
##########
@@ -17,20 +17,26 @@
//! The core concepts of OpenDAL's public API.
//!
-//! OpenDAL provides a unified abstraction for all storage services.
+//! OpenDAL provides a unified abstraction that helps developers access all
storage services.
//!
//! There are two core concepts in OpenDAL:
//!
-//! - [`Builder`]: Build an instance of underlying services.
-//! - [`Operator`]: A bridge between underlying implementation detail and
unified abstraction.
+//! - [`Builder`]: Builder accepts a series of parameters to set up an
instance of underlying services.
+//! You can adjust the behaviour of underlying services with these parameters.
+//! - [`Operator`]: Developer can access underlying storage services with
manipulating one Operator.
+//! The Operator is a delegate for underlying implementation detail, and
provides one unified access interface,
+//! including `read`, `write`, `list` and so on.
+//! Through Operators, OpenDAL standardizes the access methods of different
storage services into access to a file system-like interface.
Review Comment:
sorry for the misleading. so what is the actual goal?
actually, the API design of Operator is similar to file system-like API
during my reading of the code and the documentation.
##########
core/src/types/builder.rs:
##########
@@ -21,16 +21,25 @@ use std::env;
use crate::raw::*;
use crate::*;
-/// Builder is used to build a storage accessor used by [`Operator`].
+/// Builder is used to set up a real underlying service, i.e. storage accessor.
+///
+/// One builder is usually used by [`Operator`] during its initialization.
+/// It can be created by accepting several k-v pairs from one HashMap, one
iterator and specific environment variables.
+///
+/// By default each builder of underlying service must support deriving from
one HashMap.
+/// Besides that, according to the implementation, each builder may have its
own special methods
Review Comment:
fixed. thank you so much
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
