You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by burma-shave <no...@github.com> on 2014/11/29 23:59:39 UTC
[jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use Iterable
payload (#620)
Payload slicer has a method that returns an iterable of payloads that works on non-repeatable
InputStreams that was introduced to fix multi-part uploads in swift (JCLOUDS-356). This commit
applies the same method to fix multi-part uploads for azure blob store.
https://issues.apache.org/jira/browse/JCLOUDS-671
You can merge this Pull Request by running:
git pull https://github.com/burma-shave/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/620
-- Commit Summary --
* JCLOUDS-671: Modify AzureBlobUploadStrategy to use Iterable payload slicer.
-- File Changes --
M providers/azureblob/src/main/java/org/jclouds/azureblob/blobstore/strategy/AzureBlobBlockUploadStrategy.java (23)
M providers/azureblob/src/test/java/org/jclouds/azureblob/blobstore/strategy/AzureBlobBlockUploadStrategyTest.java (35)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/620.patch
https://github.com/jclouds/jclouds/pull/620.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds #1967](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1967/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-64971901
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1426](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1426/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-64970397
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by Andrew Phillips <no...@github.com>.
> In short, the handling of when a file is larger than the maximum block size is encapsulated in the
> PayloadIterator class in the BasePayloadSlicer.
Just to make sure I understand: you're saying the case that the test that has been modified used to cover (a payload _larger_ than the max block size) is now already covered by tests for PayloadIterator?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-65016578
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by burma-shave <no...@github.com>.
Thanks for looking at my pull-request.
In short, the handling of when a file is larger than the maximum block size is encapsulated in the PayloadIterator class in the BasePayloadSlicer. This magic happens when the higher level <code>Iterable<Payload> slice(Payload input, long size);</code> method is used. I modified the AzureBlobUploadStrategy to use this method. This is why that handling code has been removed from AzureBlobUploadStrategy and AzureBlobUploadStrategyTest.
I've updated the original comment for the pull request with more detail.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-65015420
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by Andrew Phillips <no...@github.com>.
@burma-shave The new code certainly looks simpler but I do recall the slightly more complex old code being added to introduce a problem where slices where not being handled properly if they overall size was larger than the maximum block size.
Unless I'm missing something, the changes to the test here seem to indicate that we are not testing for that case any more. Does your code change still work if the test is unchanged?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-65007183
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by Andrew Gaul <no...@github.com>.
Closed #620.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#event-203737149
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by Andrew Gaul <no...@github.com>.
I addressed some nits and pushed your changes to master as 66c4963b49a1ddd985e3b57309e465ff1922cc26. I also pushed an integration test as 1663e0911e71c57aeabd9a7e76732e205a305df5 which previously failed and now succeeds with your change. Unfortunately cherry-picking to 1.8.x had conflicts so you will need to raise another pull request to backport. Thank you for your contribution @burma-shave !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-65916006
Re: [jclouds] JCLOUDS-671: Modify AzureBlobUploadStrategy to use
Iterable payload (#620)
Posted by burma-shave <no...@github.com>.
@demobox, yes I think so.
In AzureBlobBlockUploadStrategyTest, there is a test case that verifies that MultipartUploadStrategy.MAX_BLOCK_SIZE is passed to the (mock) BasePayloadSlicer as the block size:
https://github.com/burma-shave/jclouds/blob/04decd1a836778537be58b131b85ab2398d29c14/providers/azureblob/src/test/java/org/jclouds/azureblob/blobstore/strategy/AzureBlobBlockUploadStrategyTest.java#L75
There is also a test in BasePayloadSlicerTest that tests that the stream is properly chuncked according to the passed in block size:
https://github.com/jclouds/jclouds/blob/master/core/src/test/java/org/jclouds/io/internal/BasePayloadSlicerTest.java#L56-L71
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/620#issuecomment-65017332