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/01/14 15:40:06 UTC
[GitHub] [libcloud] c-w opened a new pull request #1410: Add type
annotations to storage API
c-w opened a new pull request #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410
## Add type annotations to storage API
### Description
As discussed in https://github.com/apache/libcloud/pull/1408#discussion_r366147905, this PR adds type annotations to the StorageDriver API.
For now, mypy errors are ignored for the s3, google_storage and cloudfiles drivers since the use of inheritance on the connection classes is causing issues which will require a more thorough strategy to address.
### Status
- done, ready for review
### Checklist
- [x] Code linting ([build passed](https://travis-ci.org/CatalystCode/libcloud/builds/636950988))
- [x] Documentation (updated docstrings)
- [x] Tests ([build passed](https://travis-ci.org/CatalystCode/libcloud/builds/636950988))
- [x] ICLA ([committer](https://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] codecov-io edited a comment on issue #1410: Add type
annotations to storage API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-574261146
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=h1) Report
> Merging [#1410](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/bb865b1ee029b9bceaa83917c9b9f7b16033547e?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1410 +/- ##
==========================================
+ Coverage 86.46% 86.46% +<.01%
==========================================
Files 366 366
Lines 76791 76794 +3
Branches 7529 7529
==========================================
+ Hits 66396 66399 +3
Misses 7527 7527
Partials 2868 2868
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/storage/drivers/nimbus.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL25pbWJ1cy5weQ==) | `0% <ø> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/local.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2xvY2FsLnB5) | `73.26% <ø> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/dummy.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2R1bW15LnB5) | `50.83% <ø> (ø)` | :arrow_up: |
| [libcloud/storage/providers.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9wcm92aWRlcnMucHk=) | `57.14% <100%> (+7.14%)` | :arrow_up: |
| [libcloud/storage/drivers/s3.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL3MzLnB5) | `89.34% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9iYXNlLnB5) | `82.03% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/oss.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL29zcy5weQ==) | `70.55% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/azure\_blobs.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2F6dXJlX2Jsb2JzLnB5) | `85.62% <100%> (ø)` | :arrow_up: |
| [libcloud/common/aws.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL2F3cy5weQ==) | `84.48% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/backblaze\_b2.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2JhY2tibGF6ZV9iMi5weQ==) | `87.87% <100%> (ø)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1410?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/1410?src=pr&el=footer). Last update [bb865b1...5e3ef03](https://codecov.io/gh/apache/libcloud/pull/1410?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 commented on a change in pull request #1410: Add
type annotations to storage API
Posted by GitBox <gi...@apache.org>.
c-w commented on a change in pull request #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#discussion_r366911717
##########
File path: libcloud/storage/base.py
##########
@@ -383,28 +408,32 @@ def download_object(self, obj, destination_path, overwrite_existing=False,
'download_object not implemented for this driver')
def download_object_as_stream(self, obj, chunk_size=None):
+ # type: (Object, Optional[int]) -> Iterator[bytes]
"""
- Return a generator which yields object data.
+ Return a iterator which yields object data.
:param obj: Object instance
- :type obj: :class:`Object`
+ :type obj: :class:`libcloud.storage.base.Object`
:param chunk_size: Optional chunk size (in bytes).
:type chunk_size: ``int``
+
+ :rtype: ``iterator`` of ``bytes``
"""
raise NotImplementedError(
'download_object_as_stream not implemented for this driver')
def upload_object(self, file_path, container, object_name, extra=None,
verify_hash=True, headers=None):
+ # type: (str, Container, str, Optional[dict], Optional[bool], Optional[dict]) -> Object # noqa: E501
Review comment:
Done in d80b17c.
----------------------------------------------------------------
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 #1410: Add type annotations to
storage API
Posted by GitBox <gi...@apache.org>.
c-w commented on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-574698496
> What's your take on utilizing Python 3 native syntax for type annotations vs using comments?
@Kami Personally I'm not a huge fan of the type comments for function signatures and would be in favor of eventually switching to the native type annotations to increase readability.
E.g. compare the following two snippets:
```py
def upload_object_via_stream(self, iterator, object_name, extra=None,
headers=None):
# type: (Iterator[bytes], str, Optional[dict], Optional[Dict[str, str]]) -> Object # noqa: E501
```
```py
def upload_object_via_stream(self,
iterator: Iterator[bytes],
object_name: str,
extra: Optional[dict] = None,
headers: Optional[Dict[str, str]] = None,
) -> Object:
```
From my point of view, the former is harder to read since the function arguments must be matched with the type declaration based on index/position as opposed to being attached to each argument explicitly.
----------------------------------------------------------------
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 #1410: Add
type annotations to storage API
Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#discussion_r366757040
##########
File path: libcloud/storage/base.py
##########
@@ -383,28 +408,32 @@ def download_object(self, obj, destination_path, overwrite_existing=False,
'download_object not implemented for this driver')
def download_object_as_stream(self, obj, chunk_size=None):
+ # type: (Object, Optional[int]) -> Iterator[bytes]
"""
- Return a generator which yields object data.
+ Return a iterator which yields object data.
:param obj: Object instance
- :type obj: :class:`Object`
+ :type obj: :class:`libcloud.storage.base.Object`
:param chunk_size: Optional chunk size (in bytes).
:type chunk_size: ``int``
+
+ :rtype: ``iterator`` of ``bytes``
"""
raise NotImplementedError(
'download_object_as_stream not implemented for this driver')
def upload_object(self, file_path, container, object_name, extra=None,
verify_hash=True, headers=None):
+ # type: (str, Container, str, Optional[dict], Optional[bool], Optional[dict]) -> Object # noqa: E501
Review comment:
And for ``headers`` we could probably use ``Dict[str, str]``.
----------------------------------------------------------------
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 #1410: Add type annotations to
storage API
Posted by GitBox <gi...@apache.org>.
c-w merged pull request #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410
----------------------------------------------------------------
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 #1410: Add
type annotations to storage API
Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#discussion_r366756856
##########
File path: libcloud/storage/base.py
##########
@@ -383,28 +408,32 @@ def download_object(self, obj, destination_path, overwrite_existing=False,
'download_object not implemented for this driver')
def download_object_as_stream(self, obj, chunk_size=None):
+ # type: (Object, Optional[int]) -> Iterator[bytes]
"""
- Return a generator which yields object data.
+ Return a iterator which yields object data.
:param obj: Object instance
- :type obj: :class:`Object`
+ :type obj: :class:`libcloud.storage.base.Object`
:param chunk_size: Optional chunk size (in bytes).
:type chunk_size: ``int``
+
+ :rtype: ``iterator`` of ``bytes``
"""
raise NotImplementedError(
'download_object_as_stream not implemented for this driver')
def upload_object(self, file_path, container, object_name, extra=None,
verify_hash=True, headers=None):
+ # type: (str, Container, str, Optional[dict], Optional[bool], Optional[dict]) -> Object # noqa: E501
Review comment:
Since ``verify_hash`` has a default value, I don't think it needs ``Optional`` annotation.
----------------------------------------------------------------
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 #1410: Add type annotations to
storage API
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-574561929
Nice work :+1:
---
What's your take on utilizing Python 3 native syntax for type annotations vs using comments?
In theory, using comments, allows the trunk to still work with Python 2.7, but since we don't officially support it any more, I'm not sure if that matters or not since sooner or later we will likely start utilizing other Python 3 only features as well (and we have v2.8.x branch we can base v2.8.x bug fix release on, if needed).
----------------------------------------------------------------
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 #1410: Add type annotations to
storage API
Posted by GitBox <gi...@apache.org>.
c-w commented on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-574721737
The type annotations uncovered a bug: most drivers were discarding the base API's `headers` argument in `upload_object` and `upload_object_via_stream`. I fixed this in 3b6cdc9.
----------------------------------------------------------------
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 #1410: Add type
annotations to storage API
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-574261146
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=h1) Report
> Merging [#1410](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/bb865b1ee029b9bceaa83917c9b9f7b16033547e?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1410 +/- ##
==========================================
+ Coverage 86.46% 86.46% +<.01%
==========================================
Files 366 366
Lines 76791 76792 +1
Branches 7529 7529
==========================================
+ Hits 66396 66397 +1
Misses 7527 7527
Partials 2868 2868
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1410?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/storage/drivers/nimbus.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL25pbWJ1cy5weQ==) | `0% <ø> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/backblaze\_b2.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2JhY2tibGF6ZV9iMi5weQ==) | `87.87% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/base.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9iYXNlLnB5) | `82.03% <100%> (ø)` | :arrow_up: |
| [libcloud/common/openstack.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL29wZW5zdGFjay5weQ==) | `83.24% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/drivers/atmos.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9kcml2ZXJzL2F0bW9zLnB5) | `78.37% <100%> (ø)` | :arrow_up: |
| [libcloud/storage/providers.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvc3RvcmFnZS9wcm92aWRlcnMucHk=) | `57.14% <100%> (+7.14%)` | :arrow_up: |
| [libcloud/common/aws.py](https://codecov.io/gh/apache/libcloud/pull/1410/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL2F3cy5weQ==) | `84.48% <100%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1410?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/1410?src=pr&el=footer). Last update [bb865b1...c62129c](https://codecov.io/gh/apache/libcloud/pull/1410?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 #1410: Add type annotations to
storage API
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1410: Add type annotations to storage API
URL: https://github.com/apache/libcloud/pull/1410#issuecomment-576127933
@c-w Good catch on the headers issue :+1:
As far as comments notation goes - in-line annotations can also be used when using comments, but I also agree that native notation is nicer.
So perhaps now that we don't need to support Python 2.7 anymore, we can just switch to that notation.
----------------------------------------------------------------
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