Skip to content
This repository has been archived by the owner on Oct 18, 2021. It is now read-only.

Add appender support #18

Merged
merged 3 commits into from
Apr 25, 2021
Merged

Add appender support #18

merged 3 commits into from
Apr 25, 2021

Conversation

JinnyYi
Copy link
Contributor

@JinnyYi JinnyYi commented Apr 25, 2021

Add appender support in ref: beyondstorage/go-storage#529.

@Xuanwo
Copy link
Contributor

Xuanwo commented Apr 25, 2021

Please rebase to master and don't include unrelated commits.

storage.go Outdated
}
_, err = s.bucket.NewAppendBlobURL(rp).Create(ctx, azblob.BlobHTTPHeaders{}, nil,
azblob.BlobAccessConditions{}, nil, cpk)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this empty line

storage.go Outdated
appendResp, err := s.bucket.NewAppendBlobURL(rp).AppendBlock(
ctx, iowrap.SizedReadSeekCloser(r, size),
azblob.AppendBlobAccessConditions{}, nil, cpk)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

if err != nil {
return
}
offset += size
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.microsoft.com/en-us/rest/api/storageservices/append-block

Maybe we don't need to update offset again here?

Copy link
Contributor Author

@JinnyYi JinnyYi Apr 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func (s *aztestsSuite) TestAppendBlock(c *chk.C) {
	bsu := getBSU()
	container, _ := createNewContainer(c, bsu)
	defer delContainer(c, container)

	blob := container.NewAppendBlobURL(generateBlobName())

	resp, err := blob.Create(context.Background(), BlobHTTPHeaders{}, nil, BlobAccessConditions{}, nil, ClientProvidedKeyOptions{})
	c.Assert(err, chk.IsNil)
	c.Assert(resp.StatusCode(), chk.Equals, 201)

	appendResp, err := blob.AppendBlock(context.Background(), getReaderToRandomBytes(1024), AppendBlobAccessConditions{}, nil, ClientProvidedKeyOptions{})
	c.Assert(err, chk.IsNil)
	c.Assert(appendResp.Response().StatusCode, chk.Equals, 201)
	c.Assert(appendResp.BlobAppendOffset(), chk.Equals, "0")
	c.Assert(appendResp.BlobCommittedBlockCount(), chk.Equals, int32(1))
	c.Assert(appendResp.ETag(), chk.Not(chk.Equals), ETagNone)
	c.Assert(appendResp.LastModified().IsZero(), chk.Equals, false)
	c.Assert(appendResp.ContentMD5(), chk.Not(chk.Equals), "")
	c.Assert(appendResp.RequestID(), chk.Not(chk.Equals), "")
	c.Assert(appendResp.Version(), chk.Not(chk.Equals), "")
	c.Assert(appendResp.Date().IsZero(), chk.Equals, false)

	appendResp, err = blob.AppendBlock(context.Background(), getReaderToRandomBytes(1024), AppendBlobAccessConditions{}, nil, ClientProvidedKeyOptions{})
	c.Assert(err, chk.IsNil)
	c.Assert(appendResp.BlobAppendOffset(), chk.Equals, "1024")
	c.Assert(appendResp.BlobCommittedBlockCount(), chk.Equals, int32(2))
}

From the test file in https://github.com/Azure/azure-storage-blob-go/blob/master/azblob/zt_url_append_blob_test.go, we can see that the appendResp.BlobAppendOffset() is not the next append position.

}
}

appendResp, err := s.bucket.NewAppendBlobURL(rp).AppendBlock(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to set x-ms-blob-condition-appendpos to prevent the object has been appended via other process.

Optional conditional header, used only for the Append Block operation. A number indicating the byte offset to compare. Append Block will succeed only if the append position is equal to this number. If it is not, the request will fail with the AppendPositionConditionNotMet error (HTTP status code 412 – Precondition Failed).

https://docs.microsoft.com/en-us/rest/api/storageservices/append-block

@Xuanwo Xuanwo merged commit 31065e0 into master Apr 25, 2021
@Xuanwo Xuanwo deleted the appender-support branch April 25, 2021 10:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants