Copilot commented on code in PR #7869:
URL: https://github.com/apache/opendal/pull/7869#discussion_r3531670438
##########
bindings/python/src/operator.rs:
##########
@@ -101,6 +178,52 @@ impl Operator {
core: build_blocking_operator(&scheme, map.clone())?,
__scheme: scheme,
__map: map,
+ __from_uri: false,
+ })
+ }
+
+ /// Create a new blocking `Operator` from a URI string.
+ ///
+ /// The URI encodes the scheme and configuration in a single string, e.g.
+ /// ``memory://`` or ``s3://bucket/path?region=us-east-1``. The scheme must
+ /// belong to a service enabled in this build.
+ ///
+ /// Parameters
+ /// ----------
+ /// uri : str
+ /// The URI of the service.
+ /// **kwargs : dict
+ /// Extra options that override or supplement values encoded in the
URI.
+ ///
+ /// Returns
+ /// -------
+ /// Operator
+ /// The new operator.
+ ///
+ /// Examples
+ /// --------
+ /// >>> import opendal
+ /// >>> op = opendal.Operator.from_uri("memory://")
+ /// >>> op = opendal.Operator.from_uri("s3://bucket/path",
region="us-east-1")
+ #[classmethod]
+ #[pyo3(signature = (uri, **kwargs))]
+ pub fn from_uri(
+ _cls: &Bound<PyType>,
+ uri: &str,
+ kwargs: Option<&Bound<PyDict>>,
+ ) -> PyResult<Self> {
+ let map = kwargs
+ .map(|v| {
+ v.extract::<HashMap<String, String>>()
+ .expect("must be valid hashmap")
+ })
+ .unwrap_or_default();
Review Comment:
`from_uri` converts `kwargs` with `expect("must be valid hashmap")`, which
will panic and abort the Python process if a caller passes a non-`str -> str`
mapping (e.g., an int value). This should return a Python exception instead of
panicking.
##########
bindings/python/tests/test_from_uri.py:
##########
@@ -0,0 +1,104 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import os
+import pickle
+from random import randint
+from uuid import uuid4
+
+import pytest
+
+import opendal
+from opendal.exceptions import Unsupported
+
+
[email protected](scope="session")
+def uri_async_operator(service_name, setup_config):
+ # Mirror the `async_operator` fixture but construct via `from_uri`: a bare
+ # `scheme://` URI plus the same config passed as keyword options. This
gives
+ # `from_uri` parity with the `via_iter`-based constructor across every
+ # service the suite runs against.
+ return opendal.AsyncOperator.from_uri(f"{service_name}://", **setup_config)
Review Comment:
`uri_async_operator` claims to mirror the `async_operator` fixture, but it
currently omits the standard layers (Retry/ConcurrentLimit/MimeGuess) and the
optional `OPENDAL_TEST_CAPABILITY_OVERRIDES`. This can make capability gating
(`check_capability`) and capability equality assertions diverge from the
operator actually used in these tests.
--
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]
