Add appender support #40
Conversation
service.toml
Outdated
| @@ -17,7 +17,7 @@ optional = ["location"] | |||
| optional = ["location"] | |||
|
|
|||
| [namespace.storage] | |||
| implement = ["copier", "fetcher", "mover", "multiparter", "reacher"] | |||
| implement = ["copier", "fetcher", "mover", "multiparter", "reacher", "appender"] | |||
There was a problem hiding this comment.
Let's make them in alphabet order.
storage.go
Outdated
| @@ -79,6 +79,15 @@ func (s *Storage) create(path string, opt pairStorageCreate) (o *Object) { | |||
| return o | |||
| } | |||
|
|
|||
| func (s *Storage) createAppend(ctx context.Context, path string, opt pairStorageCreateAppend) (o *Object, err error) { | |||
| o = s.newObject(false) | |||
There was a problem hiding this comment.
Should be s.newObject(true)
storage.go
Outdated
| return | ||
| } | ||
|
|
||
| offset = *output.XQSNextAppendPosition |
There was a problem hiding this comment.
Maybe we need to check it before use. The value could be a nil.
There was a problem hiding this comment.
I wonder whether the current data uploaded successfully when the err is nil but the XQSNextAppendPosition is also nil. And what kind of return values are more appropriate in this situation?
There was a problem hiding this comment.
Yes, it's won't happen in the normal situation because we know the behavior of qingstor service and it's SDK. But we can't trust them because the behavior could be changed in the future (like API changed or a bug in SDK).
So check the value is a safeguard: No matter how the behavior changes, it's won't lead panic in our logic and our users applications.
I plan to formalize the errors returned in service, for now, let's just returns a errors.New("next append position is empty").
storager_test.go
Outdated
| @@ -315,9 +315,10 @@ func TestStorage_Stat(t *testing.T) { | |||
| checkSum, ok := o.GetEtag() | |||
| assert.True(t, ok) | |||
| assert.Equal(t, "test_etag", checkSum) | |||
| storageClass, ok := o.MustGetServiceMetadata()[MetadataStorageClass] | |||
| sm := o.MustGetServiceMetadata() | |||
There was a problem hiding this comment.
We can use GetObjectMetadata here which is generated by go-storage.
service.toml
Outdated
| @@ -44,6 +44,9 @@ optional = ["offset", "io_callback", "size", "encryption_customer_algorithm", "e | |||
| [namespace.storage.op.write] | |||
| optional = ["content_md5", "content_type", "io_callback", "storage_class", "encryption_customer_algorithm", "encryption_customer_key"] | |||
|
|
|||
| [namespace.storage.op.write_append] | |||
| optional = ["content_md5", "content_type", "storage_class"] | |||
There was a problem hiding this comment.
We can specify content_type and storage_class in write_append?
There was a problem hiding this comment.
Well, those values should be specified in the first write. But it's not a good idea to introduce these value in write_append. Can we set them in create_append like append 0 byte to object?
There was a problem hiding this comment.
If we can't do this, we need to save those value in the object and set them in the first time that user call write_append.
Something like the following:
offset, _ := o.GetAppendOffset()
if offset == 0 {
value, _ = o.GetStorageClass()
}
There was a problem hiding this comment.
We can set them in create_append through append 0 byte to object.
There was a problem hiding this comment.
If we can't, we can set those metadata in create_append and use them while the user calls write_append the first time.
| @@ -484,6 +497,46 @@ func (s *Storage) write(ctx context.Context, path string, r io.Reader, size int6 | |||
| return size, nil | |||
| } | |||
|
|
|||
| func (s *Storage) writeAppend(ctx context.Context, o *Object, r io.Reader, size int64, opt pairStorageWriteAppend) (n int64, err error) { | |||
| rp := o.GetID() | |||
|
|
|||
There was a problem hiding this comment.
Is isRead also a necessary check?
There was a problem hiding this comment.
No, we don't need to check IsRead during WriteAppend operation.
Add appender support in ref: beyondstorage/go-storage#529.