You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by GitBox <gi...@apache.org> on 2020/02/20 10:10:14 UTC
[GitHub] [libcloud] Kami opened a new pull request #1431: Add new base
storage API methods for partial (range) object downloads
Kami opened a new pull request #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431
This pull request adds two new methods to the base storage API for downloading object range aka part of an object - ``download_object_range`` and ``download_object_range_as_stream``.
The reason why I added new methods and not new arguments to the existing ``download_object`` and ``download_object_as_stream`` methods is because of how "advertisement" of supported methods and functionality works in Libcloud.
If I added a new argument, there would be no easy programatically detectable way for driver to "advertise" that it support range download functionality.
Our option would be to throw an exception saying that this functionality is not supported if user passed in ``start_bytes`` or similar argument to that method, but the problem is that this would require us or user to actually run a method to determine if that functionality is available.
To begin with I implemented it for the following drivers:
- S3 (tested it with AWS S3 and Google Storage)
- Local storage
I also planned to look into adding it to the Azure driver.
## TODO
- [ ] Test
- [ ] Code refactor to avoid some duplicated code
- [ ] Documentation changes
- [ ] Upgrade notes entry
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-589938604
I've added test cases, etc.
First I used 1-base indexing, but then I decided to use 0 based one, so it's consistent with Python string / list indexing behavior. (aka ``content[start_bytes:end_bytes]``). This way first index is ``0`` and last one is ``len(obj_content)``.
And it appears it also works in that manner with the S3 provider.
If that's not the case for some other provider, we will need to handle that inside that provider driver to expose a consistent API to our users.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-592736996
@c-w Thanks for the response.
I pushed some changes which implement that functionality for Azure Blobs driver (it's pretty much the same as the code for S3 based drivers) - 76cde3007a60ced15ffa7c1eee9f6fafd5baa3c6, 8b16adde57f276b4613a63abb3b17385e0890d4b.
At the moment, I tested it manually using my Azure account.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami merged pull request #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami merged pull request #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io edited a comment on issue #1431: Add new base
storage API methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-589393496
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=h1) Report
> Merging [#1431](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/62c9c855f4fb8b409307cdc2f8358dda8f36f58a?src=pr&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `97.59%`.
[](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1431 +/- ##
==========================================
+ Coverage 86.11% 86.16% +0.05%
==========================================
Files 370 370
Lines 77586 77693 +107
Branches 7658 7641 -17
==========================================
+ Hits 66814 66948 +134
+ Misses 7876 7853 -23
+ Partials 2896 2892 -4
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/common/azure.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL2F6dXJlLnB5) | `84.76% <ø> (ø)` | :arrow_up: |
| [libcloud/common/types.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL3R5cGVzLnB5) | `100% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfbG9jYWwucHk=) | `93.64% <100%> (+3.05%)` | :arrow_up: |
| [libcloud/test/storage/test\_s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfczMucHk=) | `93.11% <100%> (+0.33%)` | :arrow_up: |
| [libcloud/storage/drivers/cloudfiles.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2Nsb3VkZmlsZXMucHk=) | `80.26% <100%> (+0.63%)` | :arrow_up: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `89.57% <100%> (+0.23%)` | :arrow_up: |
| [libcloud/test/storage/test\_azure\_blobs.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfYXp1cmVfYmxvYnMucHk=) | `94.67% <100%> (+0.32%)` | :arrow_up: |
| [libcloud/storage/drivers/azure\_blobs.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2F6dXJlX2Jsb2JzLnB5) | `86.24% <100%> (+0.61%)` | :arrow_up: |
| [libcloud/test/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL2Jhc2UucHk=) | `100% <100%> (ø)` | |
| [libcloud/test/storage/test\_base.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfYmFzZS5weQ==) | `95.23% <100%> (+1.12%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=footer). Last update [62c9c85...1b6e489](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io edited a comment on issue #1431: Add new base
storage API methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-589393496
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=h1) Report
> Merging [#1431](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/62c9c855f4fb8b409307cdc2f8358dda8f36f58a?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.34%`.
[](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1431 +/- ##
==========================================
+ Coverage 86.11% 86.12% +<.01%
==========================================
Files 370 370
Lines 77586 77743 +157
Branches 7658 7667 +9
==========================================
+ Hits 66814 66954 +140
- Misses 7876 7886 +10
- Partials 2896 2903 +7
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/test/storage/test\_local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfbG9jYWwucHk=) | `93.01% <100%> (+2.42%)` | :arrow_up: |
| [libcloud/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9iYXNlLnB5) | `81.21% <53.84%> (-1.25%)` | :arrow_down: |
| [libcloud/storage/drivers/local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2xvY2FsLnB5) | `72.05% <62.96%> (-1.22%)` | :arrow_down: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `89.45% <93.33%> (+0.1%)` | :arrow_up: |
| [libcloud/test/storage/test\_s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfczMucHk=) | `92.86% <94.44%> (+0.08%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=footer). Last update [62c9c85...943b604](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-592747588
This PR is now more or less ready to be merged.
I just need to decide (and make any code changes, if necessary) if the ``end_bytes`` offset should be inclusive or not. Currently, it is inclusive.
In Python indexing world, end index is not inclusive, but for Range header values, end offset seems to be inclusive for all the providers I tested it with (S3, Google Storage, Azure Blobs).
To put it into perspective - if you specify ``Range: bytes=0-5`` header value and the object content is ``0123456789``, you would get ``012345`` value back.
This would map to ``content[0: 5 + 1]`` in the Python world.
We can hide this behavior from the end user inside the implementation details, we just need to decide which behavior makes the most sense. I'm leaning towards the Python indexing behavior since Libcloud users are working with Python after all.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io edited a comment on issue #1431: Add new base
storage API methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-589393496
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=h1) Report
> Merging [#1431](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/62c9c855f4fb8b409307cdc2f8358dda8f36f58a?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `88.34%`.
[](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1431 +/- ##
==========================================
+ Coverage 86.11% 86.12% +<.01%
==========================================
Files 370 370
Lines 77586 77743 +157
Branches 7658 7667 +9
==========================================
+ Hits 66814 66954 +140
- Misses 7876 7886 +10
- Partials 2896 2903 +7
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/test/storage/test\_local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfbG9jYWwucHk=) | `93.01% <100%> (+2.42%)` | :arrow_up: |
| [libcloud/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9iYXNlLnB5) | `81.21% <53.84%> (-1.25%)` | :arrow_down: |
| [libcloud/storage/drivers/local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2xvY2FsLnB5) | `72.05% <62.96%> (-1.22%)` | :arrow_down: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `89.45% <93.33%> (+0.1%)` | :arrow_up: |
| [libcloud/test/storage/test\_s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfczMucHk=) | `92.86% <94.44%> (+0.08%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=footer). Last update [62c9c85...c4606a3](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-592974051
I've also implemented those new methods for the CloudFiles drivers, added more tests and changed end_bytes offset to be non-inclusive so it's consistent with Python indexing.
I will go ahead and merge this into trunk.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io commented on issue #1431: Add new base
storage API methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-589393496
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=h1) Report
> Merging [#1431](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/62c9c855f4fb8b409307cdc2f8358dda8f36f58a?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `77.77%`.
[](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1431 +/- ##
=========================================
- Coverage 86.11% 86.1% -0.02%
=========================================
Files 370 370
Lines 77586 77691 +105
Branches 7658 7667 +9
=========================================
+ Hits 66814 66895 +81
- Misses 7876 7895 +19
- Partials 2896 2901 +5
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/test/storage/test\_local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfbG9jYWwucHk=) | `92.74% <100%> (+2.15%)` | :arrow_up: |
| [libcloud/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9iYXNlLnB5) | `82.65% <100%> (+0.2%)` | :arrow_up: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `87.11% <12.5%> (-2.24%)` | :arrow_down: |
| [libcloud/storage/drivers/local.py](https://codecov.io/gh/apache/libcloud/pull/1431/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2xvY2FsLnB5) | `72.05% <62.96%> (-1.22%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=footer). Last update [62c9c85...fffdb16](https://codecov.io/gh/apache/libcloud/pull/1431?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1431: Add new base storage API
methods for partial (range) object downloads
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1431: Add new base storage API methods for partial (range) object downloads
URL: https://github.com/apache/libcloud/pull/1431#issuecomment-588897574
@c-w @tonybaloney @erjohnso Would also like to hear your feedback on the method vs argument point and method and argument naming.
As far as naming goes, perhaps``start_offset`` and ``end_offset`` would be better? But I did want to signal that it refers to bytes offset since it appears some providers also support range download units which are not bytes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services