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 2019/12/26 04:30:08 UTC
[GitHub] [libcloud] c-w opened a new pull request #1397: Enable `ex_prefix`
to be passed via all storage SDK methods
c-w opened a new pull request #1397: Enable `ex_prefix` to be passed via all storage SDK methods
URL: https://github.com/apache/libcloud/pull/1397
## Enable `ex_prefix` to be passed via all storage SDK methods
### Description
There currently is an inconsistency in the storage SDK methods to list/iterate container objects since according to method signatures in the storage base classes `ex_prefix` can only be provided via `driver.list_container_objects` but not via `driver.iterate_container_objects`, `container.list_objects`, or `container.iterate_objects`.
This pull request fixes this inconsistency by adding `ex_prefix` to all the method signatures and also implements `ex_prefix` for the local storage provider.
### Status
- done, ready for review
### Checklist (tick everything that applies)
- [x] Code linting ([build passed](https://travis-ci.org/CatalystCode/libcloud/builds/629544380))
- [x] Documentation (updated existing docstrings)
- [x] Tests (updated existing tests)
- [x] ICLA ([committer](http://people.apache.org/phonebook.html?uid=clewolff))
----------------------------------------------------------------
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] c-w commented on issue #1397: Enable `ex_prefix` to be
passed via all storage SDK methods
Posted by GitBox <gi...@apache.org>.
c-w commented on issue #1397: Enable `ex_prefix` to be passed via all storage SDK methods
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569526239
Thanks for the additional context @Kami. I'll work on the change and update this PR.
----------------------------------------------------------------
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] c-w commented on issue #1397: Enable `ex_prefix` to be
passed via all storage SDK methods
Posted by GitBox <gi...@apache.org>.
c-w commented on issue #1397: Enable `ex_prefix` to be passed via all storage SDK methods
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569384089
@Kami Would promoting `ex_prefix` to `prefix` be a breaking API change or would we also keep the `ex_prefix` argument? If we're considering a breaking API change, I'd suggest to first merge this pull request (which is backwards compatible so potentially releasable via a patch version) and consider the argument promotion for a future change.
----------------------------------------------------------------
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 a change in pull request #1397:
Promote `ex_prefix` to top-level storage SDK argument
Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1397: Promote `ex_prefix` to top-level storage SDK argument
URL: https://github.com/apache/libcloud/pull/1397#discussion_r364075769
##########
File path: libcloud/storage/drivers/atmos.py
##########
@@ -451,11 +451,35 @@ def _emc_meta(self, response):
meta = meta.split(', ')
return dict([x.split('=', 1) for x in meta])
- def iterate_container_objects(self, container):
+ def _entries_to_objects(self, container, entries):
+ for entry in entries:
+ metadata = {'object_id': entry['id']}
+ yield Object(entry['name'], 0, '', {}, metadata, container, self)
+
+ def iterate_container_objects(self, container, prefix=None,
+ ex_prefix=None):
+ """
+ Return a generator of objects for the given container.
+
+ :param container: Container instance
+ :type container: :class:`Container`
+
+ :param prefix: Filter objects starting with a prefix.
+ Filtering is performed client-side.
+ :type prefix: ``str``
+
+ :param ex_prefix: (Deprecated.) Filter objects starting with a prefix.
+ Filtering is performed client-side.
+ :type ex_prefix: ``str``
+
+ :return: A generator of Object instances.
+ :rtype: ``generator`` of :class:`Object`
+ """
+ prefix = self._normalize_prefix_argument(prefix, ex_prefix)
+
headers = {'x-emc-include-meta': '1'}
path = self._namespace_path(container.name) + '/'
result = self.connection.request(path, headers=headers)
entries = self._list_objects(result.object, object_type='regular')
- for entry in entries:
- metadata = {'object_id': entry['id']}
- yield Object(entry['name'], 0, '', {}, metadata, container, self)
+ objects = self._entries_to_objects(container, entries)
+ return self._filter_listed_container_objects(objects, prefix)
Review comment:
I was thinking about doing this a bit differently, but that approach also works.
We just need to make sure that all the drivers which don't implement custom / server side based filtering call that method.
----------------------------------------------------------------
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] c-w commented on issue #1397: Promote `ex_prefix` to
top-level storage SDK argument
Posted by GitBox <gi...@apache.org>.
c-w commented on issue #1397: Promote `ex_prefix` to top-level storage SDK argument
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-572335948
Added upgrade note in 16c692f.
----------------------------------------------------------------
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 #1397: Enable `ex_prefix` to be
passed via all storage SDK methods
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1397: Enable `ex_prefix` to be passed via all storage SDK methods
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569446022
@c-w That's correct, but since it wasn't previously part of the base API, we probably should only update drivers and methods which supported ``ex_prefix`` argument before, to also support it for backward compatibility reasons going forward (at appears those are azure, cloudfiles, oss, dummy and s3 drivers).
And inside those methods we should probably just do something like ``prefix = prefix or ex_prefix`` and document in upgrade notes that the new argument name is ``prefix`` which has precedence over ``ex_prefix``, which was left in place for backward compatibility reasons.
In short - I would just like to avoid adding ``ex_prefix`` to the base API since base API shouldn't really have any ``ex_`` prefixed arguments and methods.
If you won't get a chance to make those changes in the near future, I can also look into it.
----------------------------------------------------------------
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 #1397: Promote `ex_prefix`
to top-level storage SDK argument
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1397: Promote `ex_prefix` to top-level storage SDK argument
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569533634
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=h1) Report
> Merging [#1397](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/043bb6d46248384bb6bc4357aa47c0c201d16b7b?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `94.73%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/libcloud/pull/1397/graphs/tree.svg?width=650&token=PYoduksh69&height=150&src=pr)](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1397 +/- ##
==========================================
- Coverage 86.58% 86.57% -0.01%
==========================================
Files 364 364
Lines 76193 76209 +16
Branches 7439 7441 +2
==========================================
+ Hits 65968 65981 +13
- Misses 7400 7402 +2
- Partials 2825 2826 +1
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/test/storage/test\_s3.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfczMucHk=) | `91.47% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_azure\_blobs.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfYXp1cmVfYmxvYnMucHk=) | `92% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_cloudfiles.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfY2xvdWRmaWxlcy5weQ==) | `89.38% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_dummy.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfZHVtbXkucHk=) | `90% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_oss.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3Rfb3NzLnB5) | `95.34% <ø> (ø)` | :arrow_up: |
| [libcloud/test/storage/test\_local.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9zdG9yYWdlL3Rlc3RfbG9jYWwucHk=) | `90.59% <100%> (+0.14%)` | :arrow_up: |
| [libcloud/storage/drivers/cloudfiles.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2Nsb3VkZmlsZXMucHk=) | `79.63% <100%> (-0.05%)` | :arrow_down: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `89.15% <100%> (-0.03%)` | :arrow_down: |
| [libcloud/storage/drivers/oss.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL29zcy5weQ==) | `70.55% <100%> (-0.08%)` | :arrow_down: |
| [libcloud/storage/drivers/local.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2xvY2FsLnB5) | `73.26% <100%> (+0.26%)` | :arrow_up: |
| ... and [5 more](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1397?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/1397?src=pr&el=footer). Last update [043bb6d...1ea7b36](https://codecov.io/gh/apache/libcloud/pull/1397?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] c-w merged pull request #1397: Promote `ex_prefix` to
top-level storage SDK argument
Posted by GitBox <gi...@apache.org>.
c-w merged pull request #1397: Promote `ex_prefix` to top-level storage SDK argument
URL: https://github.com/apache/libcloud/pull/1397
----------------------------------------------------------------
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 #1397: Enable `ex_prefix` to be
passed via all storage SDK methods
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1397: Enable `ex_prefix` to be passed via all storage SDK methods
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569141710
Overall, I'm fine with this change, but another option would be to simply name the argument ``prefix`` (aka promote it to be part of the base API).
Then we could also have a fallback, if driver doesn't implement ``_filter_objects_by_prefix`` method (or similar) we would simply fall back to the implementation on the base driver which performs simple local filtering on the whole result set.
Since that method would perform filtering locally, it would probably also be a good idea to document that behavior (perhaps also emit a warning?) to avoid surprises.
----------------------------------------------------------------
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 #1397: Promote
`ex_prefix` to top-level storage SDK argument
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1397: Promote `ex_prefix` to top-level storage SDK argument
URL: https://github.com/apache/libcloud/pull/1397#issuecomment-569533634
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=h1) Report
> Merging [#1397](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/1ea7b36185ef9974c2abb5ee332a3342fc0aa1d1?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/libcloud/pull/1397/graphs/tree.svg?width=650&token=PYoduksh69&height=150&src=pr)](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1397 +/- ##
==========================================
+ Coverage 86.57% 86.58% +<.01%
==========================================
Files 364 364
Lines 76209 76224 +15
Branches 7441 7443 +2
==========================================
+ Hits 65981 65996 +15
Misses 7402 7402
Partials 2826 2826
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1397?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/test/compute/test\_ec2.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3RfZWMyLnB5) | `97.97% <100%> (+0.01%)` | :arrow_up: |
| [libcloud/compute/drivers/ec2.py](https://codecov.io/gh/apache/libcloud/pull/1397/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2VjMi5weQ==) | `75.27% <100%> (+0.06%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1397?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/1397?src=pr&el=footer). Last update [1ea7b36...1420a55](https://codecov.io/gh/apache/libcloud/pull/1397?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