Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capability to set contentType not available in python binding #3242

Closed
jokester opened this issue Oct 8, 2023 · 5 comments · Fixed by #3256
Closed

Capability to set contentType not available in python binding #3242

jokester opened this issue Oct 8, 2023 · 5 comments · Fixed by #3256

Comments

@jokester
Copy link
Contributor

jokester commented Oct 8, 2023

I don't know much rust or python please don't put full trust in me.

When trying to use opendal in a python project I failed to find a way to set contentType when uploading a file to Object Storage service like GCS.

After investigated the core code it seems to me that,

  1. in core the capability is provided via BlockingOperator.{write_with,writer_with} Operator.{write_with,writer_with}
  2. such capability is not available in python binding
@jokester
Copy link
Contributor Author

jokester commented Oct 8, 2023

If this is confirmed I'm happy to try making a PR. I still want to use opendal and this actually blocks my work.

@Xuanwo
Copy link
Member

Xuanwo commented Oct 8, 2023

Would you like to share the design for implementing write_with in Python before makeing a PR?

@jokester
Copy link
Contributor Author

jokester commented Oct 8, 2023

Would you like to share the design for implementing write_with in Python before makeing a PR?

Sure. Like you I considered to add a write_with because there is already write in py binding.

I guess the changes will be this shape:

diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs
index 2d90c602..1887993e 100644
--- a/bindings/python/src/lib.rs
+++ b/bindings/python/src/lib.rs
@@ -124,6 +124,11 @@ impl Operator {
         self.0.write(path, bs).map_err(format_pyerr)
     }
 
+    pub fn write_with(&self, path: &str, bs: Vec<u8>) -> PyResult<FunctionWrite> {
+        todo!()
+    }
+
     /// Get current path's metadata **without cache** directly.
     pub fn stat(&self, path: &str) -> PyResult<Metadata> {
         self.0.stat(path).map_err(format_pyerr).map(Metadata)
@@ -200,6 +205,13 @@ impl Reader {
     }
 }
 
+#[pyclass(module = "opendal")]
+struct FunctionWrite(od::operator_functions::FunctionWrite);
+
+impl FunctionWrite {
+    // ... expose inner capabilities to python
+}

And some similar changes to AsyncOperator.

Finally in python we can write like:

operator.write_with(path, blob).content_type("image/png").call()

@Xuanwo
Copy link
Member

Xuanwo commented Oct 8, 2023

This design seems not native to python users. Maybe we can accept kwargs instead in python, so users can write code like:

op.write(path, blob, content_type="image/png")

Also cc python binding maintainer @messense for comments.

@jokester
Copy link
Contributor Author

jokester commented Oct 8, 2023

No problem for me. I don't use much python either so I'll try to be very open to your comments 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants