chitralverma commented on code in PR #7869:
URL: https://github.com/apache/opendal/pull/7869#discussion_r3534441008


##########
bindings/python/src/operator.rs:
##########
@@ -45,10 +46,65 @@ fn build_blocking_operator(
     Ok(op)
 }
 
+fn build_operator_from_uri(uri: &str, map: HashMap<String, String>) -> 
PyResult<ocore::Operator> {
+    let op = ocore::Operator::from_uri((uri, map)).map_err(format_pyerr)?;
+    Ok(op)
+}
+
+fn build_blocking_operator_from_uri(
+    uri: &str,
+    map: HashMap<String, String>,
+) -> PyResult<ocore::blocking::Operator> {
+    let op = build_operator_from_uri(uri, map)?;
+
+    let runtime = pyo3_async_runtimes::tokio::get_runtime();
+    let _guard = runtime.enter();
+    let op = ocore::blocking::Operator::new(op).map_err(format_pyerr)?;
+    Ok(op)
+}
+
 fn normalize_scheme(raw: &str) -> String {
     raw.trim().to_ascii_lowercase().replace('_', "-")
 }
 
+/// Extract service options from `**kwargs`, propagating a Python exception if 
a
+/// value is not a `str -> str` mapping instead of panicking.
+fn extract_kwargs(kwargs: Option<&Bound<PyDict>>) -> PyResult<HashMap<String, 
String>> {

Review Comment:
   Done. Constructors now take `kwargs: Option<HashMap<String, String>>` 
directly; PyO3 raises `TypeError` on non-str values. Dropped the helper.



##########
bindings/python/src/operator.rs:
##########
@@ -692,11 +789,12 @@ impl Operator {
             )
         }
     }
-    fn __getnewargs_ex__(&self, py: Python) -> PyResult<Py<PyAny>> {
-        let args = vec![self.__scheme.clone()];
-        let args = PyTuple::new(py, args)?.into_py_any(py)?;
-        let kwargs = self.__map.clone().into_py_any(py)?;
-        PyTuple::new(py, [args, kwargs])?.into_py_any(py)
+    fn __reduce__(&self, py: Python) -> PyResult<Py<PyAny>> {
+        let reconstructor = py
+            .import("opendal._opendal")?
+            .getattr("_reconstruct_operator")?;
+        let args = (self.__scheme.clone(), 
self.__map.clone()).into_py_any(py)?;
+        PyTuple::new(py, [reconstructor.into_py_any(py)?, 
args])?.into_py_any(py)

Review Comment:
   Types are enforced on unpickle by the reconstructor signature 
`_reconstruct_operator(scheme: &str, map: HashMap<String, String>)`. 
`__reduce__` must return `(callable, tuple)` per the pickle protocol, so its 
return is inherently `Py<PyAny>` and can't be statically typed further.



-- 
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]

Reply via email to