dentiny commented on code in PR #7827:
URL: https://github.com/apache/opendal/pull/7827#discussion_r3494274156


##########
bindings/go/writer.go:
##########
@@ -71,6 +71,36 @@ func (op *Operator) Write(path string, data []byte, opts 
...WithWriteFn) error {
        return err
 }
 
+// WriteWithMetadata writes data to the given path like Write, but additionally
+// returns the metadata of the written object (such as etag, version, or last
+// modified) as reported by the underlying service.
+//
+// WriteWithMetadata is a wrapper around the C-binding function
+// `opendal_operator_write_with_metadata`. Which fields of the returned 
Metadata
+// are populated depends on the service.
+func (op *Operator) WriteWithMetadata(path string, data []byte, opts 
...WithWriteFn) (*Metadata, error) {

Review Comment:
   Hi @yuchanns I'm wondering if you think it better to add a new API, or we 
could break current `Write` API and return metadata? 



##########
bindings/go/tests/behavior_tests/write_test.go:
##########
@@ -321,3 +323,35 @@ func testWriterWithAppend(assert *require.Assertions, op 
*opendal.Operator, fixt
        assert.Nil(err, "read must succeed")
        assert.Equal([]byte("hello world"), bs)
 }
+
+func testWriteWithMetadata(assert *require.Assertions, op *opendal.Operator, 
fixture *fixture) {
+       path, content, size := fixture.NewFile()
+
+       meta, err := op.WriteWithMetadata(path, content)
+       assert.Nil(err, "write with metadata must succeed")
+       assert.NotNil(meta, "returned metadata must not be nil")
+       assert.Equal(uint64(size), meta.ContentLength(), "write metadata 
content length")
+
+       data, err := op.Read(path)
+       assert.Nil(err)
+       assert.Equal(content, data, "written content")
+}
+
+func testWriterCloseWithMetadata(assert *require.Assertions, op 
*opendal.Operator, fixture *fixture) {
+       path := fixture.NewFilePath()
+       content := []byte("hello opendal write metadata")
+
+       w, err := op.Writer(path)
+       assert.Nil(err)
+       _, err = w.Write(content)
+       assert.Nil(err)
+
+       meta, err := w.CloseWithMetadata()
+       assert.Nil(err, "close with metadata must succeed")
+       assert.NotNil(meta, "returned metadata must not be nil")
+       assert.Equal(uint64(len(content)), meta.ContentLength(), "writer close 
metadata content length")

Review Comment:
   I'm not sure if it's proper to assert on content length, based on the 
[doc](https://opendal.apache.org/docs/rust/opendal/struct.Metadata.html#method.content_length),
 the return value could be 0 depending on the storage backend.



##########
bindings/go/tests/behavior_tests/write_test.go:
##########
@@ -321,3 +323,35 @@ func testWriterWithAppend(assert *require.Assertions, op 
*opendal.Operator, fixt
        assert.Nil(err, "read must succeed")
        assert.Equal([]byte("hello world"), bs)
 }
+
+func testWriteWithMetadata(assert *require.Assertions, op *opendal.Operator, 
fixture *fixture) {
+       path, content, size := fixture.NewFile()
+
+       meta, err := op.WriteWithMetadata(path, content)
+       assert.Nil(err, "write with metadata must succeed")
+       assert.NotNil(meta, "returned metadata must not be nil")
+       assert.Equal(uint64(size), meta.ContentLength(), "write metadata 
content length")

Review Comment:
   ditto



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