From 22303882a974187782fb4dd0f11847b44e0adb98 Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 12:21:37 +0800
Subject: [PATCH 1/7] feat(bindings/python): expose presign api

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/CONTRIBUTING.md             | 25 +++++---
 bindings/python/python/opendal/__init__.pyi | 11 ++++
 bindings/python/src/asyncio.rs              | 65 +++++++++++++++++++++
 bindings/python/src/lib.rs                  | 29 +++++++++
 4 files changed, 122 insertions(+), 8 deletions(-)

diff --git a/bindings/python/CONTRIBUTING.md b/bindings/python/CONTRIBUTING.md
index 7e6a73a128d0..cac74fc11a22 100644
--- a/bindings/python/CONTRIBUTING.md
+++ b/bindings/python/CONTRIBUTING.md
@@ -1,12 +1,13 @@
 # Contributing
 
-- [Setup](#setup)
-  - [Using a dev container environment](#using-a-devcontainer-environment)
-  - [Bring your own toolbox](#bring-your-own-toolbox)
-- [Prepare](#prepare)
-- [Build](#build)
-- [Test](#test)
-- [Docs](#docs)
+- [Contributing](#contributing)
+  - [Setup](#setup)
+    - [Using a dev container environment](#using-a-dev-container-environment)
+    - [Bring your own toolbox](#bring-your-own-toolbox)
+  - [Prepare](#prepare)
+  - [Build](#build)
+  - [Test](#test)
+  - [Docs](#docs)
 
 ## Setup
 
@@ -50,12 +51,20 @@ pip install maturin[patchelf]
 
 ## Build
 
-To build python binding:
+To build python binding only:
+
+```shell
+maturin build
+```
+
+To build and install python binding directly in the current virtualenv:
 
 ```shell
 maturin develop
 ```
 
+Note: `maturin develop` will be faster, but doesn't support all the features. In most development cases, we recommend using `maturin develop`.
+
 ## Test
 
 OpenDAL adopts `behave` for behavior tests:
diff --git a/bindings/python/python/opendal/__init__.pyi b/bindings/python/python/opendal/__init__.pyi
index 2f82aed6d6f7..eb8cc33ecff6 100644
--- a/bindings/python/python/opendal/__init__.pyi
+++ b/bindings/python/python/opendal/__init__.pyi
@@ -40,6 +40,9 @@ class AsyncOperator:
     async def delete(self, path: str): ...
     async def list(self, path: str) -> AsyncIterable[Entry]: ...
     async def scan(self, path: str) -> AsyncIterable[Entry]: ...
+    async def presign_stat(self, path: str, expire: int) -> PresignedRequest: ...
+    async def presign_read(self, path: str, expire: int) -> PresignedRequest: ...
+    async def presign_write(self, path: str, expire: int) -> PresignedRequest: ...
 
 class Reader:
     def read(self, size: Optional[int] = None) -> bytes: ...
@@ -76,3 +79,11 @@ class Metadata:
 class EntryMode:
     def is_file(self) -> bool: ...
     def is_dir(self) -> bool: ...
+
+class PresignedRequest:
+    @property
+    def url(self) -> str: ...
+    @property
+    def method(self) -> str: ...
+    @property
+    def headers(self) -> dict[str, bytes]: ...
\ No newline at end of file
diff --git a/bindings/python/src/asyncio.rs b/bindings/python/src/asyncio.rs
index 4b33015fa136..0c796b94792e 100644
--- a/bindings/python/src/asyncio.rs
+++ b/bindings/python/src/asyncio.rs
@@ -19,6 +19,7 @@ use std::collections::HashMap;
 use std::io::SeekFrom;
 use std::str::FromStr;
 use std::sync::Arc;
+use std::time::Duration;
 
 use ::opendal as od;
 use futures::TryStreamExt;
@@ -39,6 +40,7 @@ use crate::format_pyerr;
 use crate::layers;
 use crate::Entry;
 use crate::Metadata;
+use crate::PresignedRequest;
 
 /// `AsyncOperator` is the entry for all public async APIs
 ///
@@ -159,6 +161,69 @@ impl AsyncOperator {
         })
     }
 
+    /// Presign an operation for stat(head).
+    /// 
+    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    pub fn presign_stat<'p>(
+        &'p self,
+        py: Python<'p>,
+        path: String,
+        expire: u64,
+    ) -> PyResult<&'p PyAny> {
+        let this = self.0.clone();
+        future_into_py(py, async move {
+            let res = this
+                .presign_stat(&path, Duration::from_secs(expire))
+                .await
+                .map_err(format_pyerr)
+                .map(PresignedRequest)?;
+
+            Ok(res)
+        })
+    }
+
+    /// Presign an operation for read.
+    /// 
+    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    pub fn presign_read<'p>(
+        &'p self,
+        py: Python<'p>,
+        path: String,
+        expire: u64,
+    ) -> PyResult<&'p PyAny> {
+        let this = self.0.clone();
+        future_into_py(py, async move {
+            let res = this
+                .presign_read(&path, Duration::from_secs(expire))
+                .await
+                .map_err(format_pyerr)
+                .map(PresignedRequest)?;
+
+            Ok(res)
+        })
+    }
+
+    /// Presign an operation for write.
+    /// 
+    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    pub fn presign_write<'p>(
+        &'p self,
+        py: Python<'p>,
+        path: String,
+        expire: u64,
+    ) -> PyResult<&'p PyAny> {
+        let this = self.0.clone();
+        future_into_py(py, async move {
+            let res = this
+                .presign_write(&path, Duration::from_secs(expire))
+                .await
+                .map_err(format_pyerr)
+                .map(PresignedRequest)?;
+
+            Ok(res)
+        })
+    }
+
     fn __repr__(&self) -> String {
         let info = self.0.info();
         let name = info.name();
diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs
index 81189b3b07a2..0c6e50e1af9a 100644
--- a/bindings/python/src/lib.rs
+++ b/bindings/python/src/lib.rs
@@ -368,6 +368,34 @@ impl EntryMode {
     }
 }
 
+#[pyclass(module = "opendal")]
+struct PresignedRequest(od::raw::PresignedRequest);
+
+#[pymethods]
+impl PresignedRequest {
+    /// Return the URL of this request.
+    #[getter]
+    pub fn url(&self) -> String {
+        self.0.uri().to_string()
+    }
+
+    /// Return the HTTP method of this request.
+    #[getter]
+    pub fn method(&self) -> &str {
+        self.0.method().as_str()
+    }
+
+    /// Return the HTTP headers of this request.
+    #[getter]
+    pub fn headers<'p>(&'p self, py: Python<'p>) -> HashMap<&'p str, &'p PyBytes> {
+        self.0
+            .header()
+            .iter()
+            .map(|(k, v)| (k.as_str(), PyBytes::new(py, v.as_bytes())))
+            .collect()
+    }
+}
+
 fn format_pyerr(err: od::Error) -> PyErr {
     use od::ErrorKind::*;
     match err.kind() {
@@ -416,6 +444,7 @@ fn _opendal(py: Python, m: &PyModule) -> PyResult<()> {
     m.add_class::<Entry>()?;
     m.add_class::<EntryMode>()?;
     m.add_class::<Metadata>()?;
+    m.add_class::<PresignedRequest>()?;
     m.add("Error", py.get_type::<Error>())?;
 
     let layers = layers::create_submodule(py)?;

From f14ff6e33608cf7dafa2e9f65c5c710df204ed35 Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 12:58:43 +0800
Subject: [PATCH 2/7] add test for presign

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/src/asyncio.rs          |  6 +++---
 bindings/python/tests/steps/binding.py  | 11 +++++++++++
 bindings/tests/features/binding.feature |  4 ++++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/bindings/python/src/asyncio.rs b/bindings/python/src/asyncio.rs
index 0c796b94792e..3bd2395ed79c 100644
--- a/bindings/python/src/asyncio.rs
+++ b/bindings/python/src/asyncio.rs
@@ -162,7 +162,7 @@ impl AsyncOperator {
     }
 
     /// Presign an operation for stat(head).
-    /// 
+    ///
     /// The returned `PresignedRequest` will be expired after `expire` seconds.
     pub fn presign_stat<'p>(
         &'p self,
@@ -183,7 +183,7 @@ impl AsyncOperator {
     }
 
     /// Presign an operation for read.
-    /// 
+    ///
     /// The returned `PresignedRequest` will be expired after `expire` seconds.
     pub fn presign_read<'p>(
         &'p self,
@@ -204,7 +204,7 @@ impl AsyncOperator {
     }
 
     /// Presign an operation for write.
-    /// 
+    ///
     /// The returned `PresignedRequest` will be expired after `expire` seconds.
     pub fn presign_write<'p>(
         &'p self,
diff --git a/bindings/python/tests/steps/binding.py b/bindings/python/tests/steps/binding.py
index 4d343eaaffc5..1466f9cc5397 100644
--- a/bindings/python/tests/steps/binding.py
+++ b/bindings/python/tests/steps/binding.py
@@ -88,3 +88,14 @@ async def step_impl(context, filename, size):
 async def step_impl(context, filename, content):
     bs = await context.op.read(filename)
     assert bs == content.encode()
+
+@given("A new OpenDAL Async Operator (s3 presign only)")
+def step_impl(context):
+    context.op = opendal.AsyncOperator("s3", bucket="test", region="us-east-1", access_key_id="test", secret_access_key="test")
+
+@then("The operator is available for presign")
+@async_run_until_complete
+async def step_impl(context):
+    await context.op.presign_stat("test.txt", 10)
+    await context.op.presign_read("test.txt", 10)
+    await context.op.presign_write("test.txt", 10)
\ No newline at end of file
diff --git a/bindings/tests/features/binding.feature b/bindings/tests/features/binding.feature
index ad6832c091e5..4fdc8726cec4 100644
--- a/bindings/tests/features/binding.feature
+++ b/bindings/tests/features/binding.feature
@@ -32,3 +32,7 @@ Feature: OpenDAL Binding
         Then The async file "test" entry mode must be file
         Then The async file "test" content length must be 13
         Then The async file "test" must have content "Hello, World!"
+
+    Scenario: OpenDAL Operations with Presign
+        Given A new OpenDAL Async Operator (s3 presign only)
+        Then The operator is available for presign

From 150f54d6152a32a41e2ec9d69541820743396c0a Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 13:12:31 +0800
Subject: [PATCH 3/7] fix HeaderMap to return error for invalid value

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/src/lib.rs | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs
index 0c6e50e1af9a..32ba2311776f 100644
--- a/bindings/python/src/lib.rs
+++ b/bindings/python/src/lib.rs
@@ -387,12 +387,18 @@ impl PresignedRequest {
 
     /// Return the HTTP headers of this request.
     #[getter]
-    pub fn headers<'p>(&'p self, py: Python<'p>) -> HashMap<&'p str, &'p PyBytes> {
-        self.0
-            .header()
-            .iter()
-            .map(|(k, v)| (k.as_str(), PyBytes::new(py, v.as_bytes())))
-            .collect()
+    pub fn headers(&self) -> PyResult<HashMap<&str, &str>> {
+        let mut headers = HashMap::new();
+        for (k, v) in self.0.header().iter() {
+            let k = k.as_str();
+            let v = v
+                .to_str()
+                .map_err(|err| PyValueError::new_err(err.to_string()))?;
+            if headers.insert(k, v).is_some() {
+                return Err(PyValueError::new_err("duplicate header"));
+            }
+        }
+        Ok(headers)
     }
 }
 

From ec8e930a97dabbd351d741c03f802bf2e991158a Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 13:19:09 +0800
Subject: [PATCH 4/7] update headers function signature && rename presign test

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/python/opendal/__init__.pyi | 2 +-
 bindings/tests/features/binding.feature     | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bindings/python/python/opendal/__init__.pyi b/bindings/python/python/opendal/__init__.pyi
index eb8cc33ecff6..a60057a5b9d3 100644
--- a/bindings/python/python/opendal/__init__.pyi
+++ b/bindings/python/python/opendal/__init__.pyi
@@ -86,4 +86,4 @@ class PresignedRequest:
     @property
     def method(self) -> str: ...
     @property
-    def headers(self) -> dict[str, bytes]: ...
\ No newline at end of file
+    def headers(self) -> dict[str, str]: ...
\ No newline at end of file
diff --git a/bindings/tests/features/binding.feature b/bindings/tests/features/binding.feature
index 4fdc8726cec4..83921a63346d 100644
--- a/bindings/tests/features/binding.feature
+++ b/bindings/tests/features/binding.feature
@@ -33,6 +33,6 @@ Feature: OpenDAL Binding
         Then The async file "test" content length must be 13
         Then The async file "test" must have content "Hello, World!"
 
-    Scenario: OpenDAL Operations with Presign
-        Given A new OpenDAL Async Operator (s3 presign only)
+    Scenario: OpenDAL Async Operations with Presign
+        Given A new OpenDAL Async Operator (presign available)
         Then The operator is available for presign

From 47f200b5abc025d7a428028e8090c7287b62407d Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 13:26:34 +0800
Subject: [PATCH 5/7] typo

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/tests/steps/binding.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bindings/python/tests/steps/binding.py b/bindings/python/tests/steps/binding.py
index 1466f9cc5397..d4200bcb50ee 100644
--- a/bindings/python/tests/steps/binding.py
+++ b/bindings/python/tests/steps/binding.py
@@ -89,7 +89,7 @@ async def step_impl(context, filename, content):
     bs = await context.op.read(filename)
     assert bs == content.encode()
 
-@given("A new OpenDAL Async Operator (s3 presign only)")
+@given("A new OpenDAL Async Operator (presign available)")
 def step_impl(context):
     context.op = opendal.AsyncOperator("s3", bucket="test", region="us-east-1", access_key_id="test", secret_access_key="test")
 

From c154c21ed74177fa75d959e9caae5404616b5aa2 Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Mon, 28 Aug 2023 14:02:13 +0800
Subject: [PATCH 6/7] update test for presign

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/src/lib.rs              | 11 +++++++----
 bindings/python/tests/steps/binding.py  | 15 +++++++--------
 bindings/tests/features/binding.feature |  5 +----
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs
index 32ba2311776f..4e3dc11c2f77 100644
--- a/bindings/python/src/lib.rs
+++ b/bindings/python/src/lib.rs
@@ -27,9 +27,11 @@ use std::str::FromStr;
 use ::opendal as od;
 use pyo3::create_exception;
 use pyo3::exceptions::PyException;
+use pyo3::exceptions::PyFileExistsError;
 use pyo3::exceptions::PyFileNotFoundError;
 use pyo3::exceptions::PyIOError;
 use pyo3::exceptions::PyNotImplementedError;
+use pyo3::exceptions::PyPermissionError;
 use pyo3::exceptions::PyValueError;
 use pyo3::prelude::*;
 use pyo3::types::PyBytes;
@@ -391,11 +393,9 @@ impl PresignedRequest {
         let mut headers = HashMap::new();
         for (k, v) in self.0.header().iter() {
             let k = k.as_str();
-            let v = v
-                .to_str()
-                .map_err(|err| PyValueError::new_err(err.to_string()))?;
+            let v = v.to_str().map_err(|err| Error::new_err(err.to_string()))?;
             if headers.insert(k, v).is_some() {
-                return Err(PyValueError::new_err("duplicate header"));
+                return Err(Error::new_err("duplicate header"));
             }
         }
         Ok(headers)
@@ -406,6 +406,9 @@ fn format_pyerr(err: od::Error) -> PyErr {
     use od::ErrorKind::*;
     match err.kind() {
         NotFound => PyFileNotFoundError::new_err(err.to_string()),
+        AlreadyExists => PyFileExistsError::new_err(err.to_string()),
+        PermissionDenied => PyPermissionError::new_err(err.to_string()),
+        Unsupported => PyNotImplementedError::new_err(err.to_string()),
         _ => Error::new_err(err.to_string()),
     }
 }
diff --git a/bindings/python/tests/steps/binding.py b/bindings/python/tests/steps/binding.py
index d4200bcb50ee..06edb9f7ffc2 100644
--- a/bindings/python/tests/steps/binding.py
+++ b/bindings/python/tests/steps/binding.py
@@ -89,13 +89,12 @@ async def step_impl(context, filename, content):
     bs = await context.op.read(filename)
     assert bs == content.encode()
 
-@given("A new OpenDAL Async Operator (presign available)")
-def step_impl(context):
-    context.op = opendal.AsyncOperator("s3", bucket="test", region="us-east-1", access_key_id="test", secret_access_key="test")
-
-@then("The operator is available for presign")
+@then("The presign operation should success or raise exception Unsupported")
 @async_run_until_complete
 async def step_impl(context):
-    await context.op.presign_stat("test.txt", 10)
-    await context.op.presign_read("test.txt", 10)
-    await context.op.presign_write("test.txt", 10)
\ No newline at end of file
+    try:
+        await context.op.presign_stat("test.txt", 10)
+        await context.op.presign_read("test.txt", 10)
+        await context.op.presign_write("test.txt", 10)
+    except NotImplementedError:
+        pass
\ No newline at end of file
diff --git a/bindings/tests/features/binding.feature b/bindings/tests/features/binding.feature
index 83921a63346d..f9f02a7a7c00 100644
--- a/bindings/tests/features/binding.feature
+++ b/bindings/tests/features/binding.feature
@@ -32,7 +32,4 @@ Feature: OpenDAL Binding
         Then The async file "test" entry mode must be file
         Then The async file "test" content length must be 13
         Then The async file "test" must have content "Hello, World!"
-
-    Scenario: OpenDAL Async Operations with Presign
-        Given A new OpenDAL Async Operator (presign available)
-        Then The operator is available for presign
+        Then The presign operation should success or raise exception Unsupported

From 98ebd77d5287eef7263433721179d8bc645ceb77 Mon Sep 17 00:00:00 2001
From: silver-ymz <yinmingzhuo@gmail.com>
Date: Tue, 29 Aug 2023 10:58:28 +0800
Subject: [PATCH 7/7] make clear for expire second

Signed-off-by: silver-ymz <yinmingzhuo@gmail.com>
---
 bindings/python/python/opendal/__init__.pyi |  6 +++---
 bindings/python/src/asyncio.rs              | 24 ++++++++-------------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/bindings/python/python/opendal/__init__.pyi b/bindings/python/python/opendal/__init__.pyi
index a60057a5b9d3..2b3501500e73 100644
--- a/bindings/python/python/opendal/__init__.pyi
+++ b/bindings/python/python/opendal/__init__.pyi
@@ -40,9 +40,9 @@ class AsyncOperator:
     async def delete(self, path: str): ...
     async def list(self, path: str) -> AsyncIterable[Entry]: ...
     async def scan(self, path: str) -> AsyncIterable[Entry]: ...
-    async def presign_stat(self, path: str, expire: int) -> PresignedRequest: ...
-    async def presign_read(self, path: str, expire: int) -> PresignedRequest: ...
-    async def presign_write(self, path: str, expire: int) -> PresignedRequest: ...
+    async def presign_stat(self, path: str, expire_second: int) -> PresignedRequest: ...
+    async def presign_read(self, path: str, expire_second: int) -> PresignedRequest: ...
+    async def presign_write(self, path: str, expire_second: int) -> PresignedRequest: ...
 
 class Reader:
     def read(self, size: Optional[int] = None) -> bytes: ...
diff --git a/bindings/python/src/asyncio.rs b/bindings/python/src/asyncio.rs
index 3bd2395ed79c..b01ed930dc48 100644
--- a/bindings/python/src/asyncio.rs
+++ b/bindings/python/src/asyncio.rs
@@ -161,19 +161,17 @@ impl AsyncOperator {
         })
     }
 
-    /// Presign an operation for stat(head).
-    ///
-    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    /// Presign an operation for stat(head) which expires after `expire_second` seconds.
     pub fn presign_stat<'p>(
         &'p self,
         py: Python<'p>,
         path: String,
-        expire: u64,
+        expire_second: u64,
     ) -> PyResult<&'p PyAny> {
         let this = self.0.clone();
         future_into_py(py, async move {
             let res = this
-                .presign_stat(&path, Duration::from_secs(expire))
+                .presign_stat(&path, Duration::from_secs(expire_second))
                 .await
                 .map_err(format_pyerr)
                 .map(PresignedRequest)?;
@@ -182,19 +180,17 @@ impl AsyncOperator {
         })
     }
 
-    /// Presign an operation for read.
-    ///
-    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    /// Presign an operation for read which expires after `expire_second` seconds.
     pub fn presign_read<'p>(
         &'p self,
         py: Python<'p>,
         path: String,
-        expire: u64,
+        expire_second: u64,
     ) -> PyResult<&'p PyAny> {
         let this = self.0.clone();
         future_into_py(py, async move {
             let res = this
-                .presign_read(&path, Duration::from_secs(expire))
+                .presign_read(&path, Duration::from_secs(expire_second))
                 .await
                 .map_err(format_pyerr)
                 .map(PresignedRequest)?;
@@ -203,19 +199,17 @@ impl AsyncOperator {
         })
     }
 
-    /// Presign an operation for write.
-    ///
-    /// The returned `PresignedRequest` will be expired after `expire` seconds.
+    /// Presign an operation for write which expires after `expire_second` seconds.
     pub fn presign_write<'p>(
         &'p self,
         py: Python<'p>,
         path: String,
-        expire: u64,
+        expire_second: u64,
     ) -> PyResult<&'p PyAny> {
         let this = self.0.clone();
         future_into_py(py, async move {
             let res = this
-                .presign_write(&path, Duration::from_secs(expire))
+                .presign_write(&path, Duration::from_secs(expire_second))
                 .await
                 .map_err(format_pyerr)
                 .map(PresignedRequest)?;