closes #2496 and #2552 #2603
closes #2496 and #2552 #2603
Conversation
|
@nim-nim @dmcgowan @sajayantony @sivagms |
|
Try updating the |
|
Please sign your commits following these rules: $ git clone -b "newazuresdk" git@github.com:yuwaMSFT2/distribution.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354362440
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
Codecov Report
@@ Coverage Diff @@
## master #2603 +/- ##
==========================================
- Coverage 60.92% 51.09% -9.84%
==========================================
Files 129 129
Lines 11876 11901 +25
==========================================
- Hits 7236 6081 -1155
- Misses 3739 5057 +1318
+ Partials 901 763 -138
Continue to review full report at Codecov.
|
|
@dmcgowan Thanks! I updated the vendor package and ran dep-validate locally. |
| @@ -118,7 +119,7 @@ func (d *driver) GetContent(ctx context.Context, path string) ([]byte, error) { | |||
|
|
|||
| // PutContent stores the []byte content at a location designated by "path". | |||
| func (d *driver) PutContent(ctx context.Context, path string, contents []byte) error { | |||
| if limit := 64 * 1024 * 1024; len(contents) > limit { // max size for block blobs uploaded via single "Put Blob" | |||
| if limit := 256 * 1024 * 1024; len(contents) > limit { // max size for block blobs uploaded via single "Put Blob" | |||
There was a problem hiding this comment.
This limit was changed on the Azure side? Was this documented somewhere, I know this hard coded value has been confusing in the past
There was a problem hiding this comment.
Yes. The new SDK is using a newer version header when sending request. The new version supports 256MB. This will allow all driver tests to pass (previously there was one failed test due to the size limit here).
https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob#remarks
There was a problem hiding this comment.
I updated the PR with comments.
|
Much cleaner API. The change looks good, I just had one question about the PutBlob size, and maybe a request to clarify in the comment. Otherwise LGTM, can you also get someone on the Azure side to LGTM here before merging. |
|
@marstr @shizhMSFT @colemickens |
There was a problem hiding this comment.
If it were me, I'd be using this as an opportunity to take advantage of the new repository:
https://github.com/Azure/azure-storage-blob-go
It is the replacement for the blob functions that you're referencing today.
| if limit := 64 * 1024 * 1024; len(contents) > limit { // max size for block blobs uploaded via single "Put Blob" | ||
| // max size for block blobs uploaded via single "Put Blob" for version after "2016-05-31" | ||
| // https://docs.microsoft.com/en-us/rest/api/storageservices/put-blob#remarks | ||
| if limit := 256 * 1024 * 1024; len(contents) > limit { |
There was a problem hiding this comment.
[nit] I'd declare this as:
const limit = 256 * 1024 * 1024
if len(contents) > limit {
// stuff
}There was a problem hiding this comment.
updated the const declaration.
|
@marstr Regarding the new storage repo, how reliable is it? I saw only prelease builds for github.com/Azure/azure-storage-blob-go ( as well as the referenced package Azure/azure-pipeline-go). This is a bit uncomfortable. Would prefer a stable version. Do you think it's Ok to move to azure-storage-blob-go later once it starts to generate official release? |
Totally fair, there have been a few breaking changes introduced. In terms of functionality, it's pretty robust, and checking in a vendored copy should protect you from having breaking changes forced on you. However, I can totally understand waiting for |
|
@marstr Thanks. I will check back and update the reference once the new blob storage sdk has 1.0. |
Update Azure SDK with release v16.2.1 Update Azure autorest SDK with release v10.8.1 Signed-off-by: Yu Wang <yuwa@microsoft.com>
|
LGTM |
Update Azure SDK with release v16.2.1
Update Azure autorest SDK with release v10.8.1
Signed-off-by: Yu Wang yuwa@microsoft.com