chitralverma commented on code in PR #7883:
URL: https://github.com/apache/opendal/pull/7883#discussion_r3552660639
##########
bindings/python/src/services.rs:
##########
@@ -273,3 +266,6974 @@ impl_enum_to_str!(
YandexDisk => "yandex-disk",
}
);
+
+pub trait ConfigBuilder: Send + Sync {
+ fn build(&self) -> PyResult<ocore::Operator>;
+ fn scheme(&self) -> &'static str;
+ fn to_map(&self) -> HashMap<String, String>;
+ fn is_picklable(&self) -> bool;
+}
+
+/// Base class for all service configs.
+#[pyclass(module = "opendal.services", subclass)]
+pub struct ServiceConfig(pub Box<dyn ConfigBuilder>);
+
+#[pymethods]
+impl ServiceConfig {
+ /// The service scheme this config targets, e.g. ``"s3"``.
+ #[getter]
+ pub fn scheme(&self) -> &'static str {
+ self.0.scheme()
+ }
+}
+
+#[cfg(feature = "services-aliyun-drive")]
+/// Configuration for the `aliyun-drive` service.
+#[pyclass(module = "opendal.services", extends = ServiceConfig, frozen,
skip_from_py_object)]
+#[derive(Clone)]
+pub struct AliyunDriveConfig(ocore::services::AliyunDriveConfig);
+
+#[cfg(feature = "services-aliyun-drive")]
+#[allow(deprecated)]
+impl ConfigBuilder for AliyunDriveConfig {
+ fn build(&self) -> PyResult<ocore::Operator> {
+ ocore::Operator::from_config(self.0.clone()).map_err(format_pyerr)
+ }
+ fn scheme(&self) -> &'static str {
+ "aliyun-drive"
+ }
+ fn to_map(&self) -> HashMap<String, String> {
+ #[allow(unused_mut)]
+ let mut map = HashMap::new();
+ map.insert("drive_type".to_string(), self.0.drive_type.clone());
+ if let Some(v) = &self.0.access_token {
+ map.insert("access_token".to_string(), v.clone());
+ }
+ if let Some(v) = &self.0.client_id {
+ map.insert("client_id".to_string(), v.clone());
+ }
+ if let Some(v) = &self.0.client_secret {
+ map.insert("client_secret".to_string(), v.clone());
+ }
+ if let Some(v) = &self.0.refresh_token {
+ map.insert("refresh_token".to_string(), v.clone());
+ }
+ if let Some(v) = &self.0.root {
+ map.insert("root".to_string(), v.clone());
+ }
+ map
+ }
+ fn is_picklable(&self) -> bool {
+ #[allow(unused_mut)]
+ let mut ok = true;
+ ok
+ }
+}
+
+#[cfg(feature = "services-aliyun-drive")]
+#[allow(deprecated)]
+#[pymethods]
+impl AliyunDriveConfig {
+ #[new]
+ #[pyo3(signature = (
+ drive_type,
+ access_token = None,
+ client_id = None,
+ client_secret = None,
+ refresh_token = None,
+ root = None,
+ ))]
+ #[allow(clippy::too_many_arguments)]
+ fn new(
+ drive_type: String,
+ access_token: Option<String>,
+ client_id: Option<String>,
+ client_secret: Option<String>,
+ refresh_token: Option<String>,
+ root: Option<crate::PyPath>,
+ ) -> PyResult<pyo3::PyClassInitializer<Self>> {
+ #[allow(unused_mut)]
+ let mut opts: HashMap<String, String> = HashMap::new();
+ opts.insert("drive_type".to_string(), drive_type);
Review Comment:
Fair point. The typed `#[new]` is the goal (typed, discoverable
`S3Config(bucket=...)`); the string map underneath is redundant for most fields.
I did it this way to match Java, where `ServiceConfig.configMap()` produces
the same string map, and to keep all three paths (`Operator(scheme, **kwargs)`,
`from_uri`, `from_config`) on one deserializer.
The core config structs are `#[non_exhaustive]` with public fields, so I can
instead assign directly (`cfg.field = value`) and only special-case the few
non-1:1 types (Duration, hf enums) — typed end-to-end, no string round-trip.
Prefer that, or keep parity with Java's `configMap()`?
--
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]
