Get successful mypy pass on azure core #5466

rakshith91 · 2019-05-24T01:22:42Z

Duplicate of #5266

Resolves #5022

adxsdk6 · 2019-05-24T01:22:44Z

Can one of the admins verify this patch?

xiangyan99 · 2019-05-24T17:39:58Z

sdk/core/azure-core/azure/core/common.py

+# --------------------------------------------------------------------------
+from typing import List, Union, Optional, TypeVar, Callable
+from azure.core.pipeline.policies import AsyncHTTPPolicy, SansIOHTTPPolicy
+from azure.core.exceptions import (


Do we need "try" for AsyncHTTPPolicy?

xiangyan99 · 2019-05-24T17:40:33Z

sdk/core/azure-core/azure/core/pipeline/base.py

@@ -60,7 +60,7 @@ def send(self, request):
 class _TransportRunner(HTTPPolicy):

    def __init__(self, sender):
-        # type: (HttpTransport) -> None
+        # type: (Any) -> None


Is sender any?

We had HTTPSender in msrest which we don't have anymore. Can't find a relevant sender class.

I think HttpTransport is correct

xiangyan99 · 2019-05-24T17:42:39Z

sdk/core/azure-core/azure/core/pipeline/base.py

@@ -80,7 +80,7 @@ class Pipeline(AbstractContextManager, Generic[HTTPRequestType, HTTPResponseType
    """

    def __init__(self, transport, policies=None):
-        # type: (HttpTransport, List[Union[HTTPPolicy, SansIOHTTPPolicy]]) -> None
+        # type: (Any, List[Union[HTTPPolicy, SansIOHTTPPolicy]]) -> None


transport is one kind of transport type, not any

Shouldn't List[Union[HTTPPolicy, SansIOHTTPPolicy] be PoliciesType?

included this

xiangyan99 · 2019-05-24T17:46:10Z

sdk/core/azure-core/azure/core/pipeline/policies/universal.py

@@ -204,15 +206,15 @@ class ContentDecodePolicy(SansIOHTTPPolicy):

    @classmethod
    def deserialize_from_text(cls, response, content_type=None):
-        # type: (Optional[Union[AnyStr, IO]], Optional[str]) -> Any
+        # type: (Optional[PipelineResponse], Optional[str]) -> Any


Seems like we missed cls and response is not optional

xiangyan99 · 2019-05-24T17:46:28Z

sdk/core/azure-core/azure/core/pipeline/policies/universal.py

@@ -260,16 +262,16 @@ def _json_attemp(data):

    @classmethod
    def deserialize_from_http_generics(cls, response):
-        # type: (Optional[Union[AnyStr, IO]], Mapping) -> Any
+        # type: (PipelineResponse) -> Any


xiangyan99 · 2019-05-24T17:48:00Z

sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py

@@ -127,7 +129,7 @@ class RequestsTransport(HttpTransport):
    _protocols = ['http://', 'https://']

    def __init__(self, configuration=None, session=None, session_owner=True):
-        # type: (Optional[requests.Session]) -> None
+        # type: (Optional[Any], Optional[requests.Session], Any) -> None


session_owner is bool

xiangyan99 · 2019-05-24T21:42:33Z

sdk/core/azure-core/azure/core/common.py

+from msrest.serialization import Model # type: ignore # pylint: disable=unused-import
+
+AsyncHTTPResponseType = TypeVar("AsyncHTTPResponseType")
+HTTPRequestType = TypeVar("HTTPRequestType")


HTTPRequestType is already defined in sdk/core/azure-core/azure/core/pipeline/init.py

And seems like the only one uses AsyncHTTPResponseType & ImplPoliciesType is base_async.py

xiangyan99 · 2019-05-24T21:43:13Z

sdk/core/azure-core/azure/core/pipeline/base_async.py

@@ -29,6 +29,7 @@

 from azure.core.pipeline import PipelineRequest, PipelineResponse, PipelineContext
 from azure.core.pipeline.policies import AsyncHTTPPolicy, SansIOHTTPPolicy
+from azure.core.common import ImplPoliciesType, PoliciesType

 AsyncHTTPResponseType = TypeVar("AsyncHTTPResponseType")


AsyncHTTPResponseType is already defined in azure.core.common

xiangyan99 · 2019-05-25T00:51:18Z

sdk/core/azure-core/azure/core/pipeline/transport/aiohttp.py

@@ -102,7 +102,7 @@ def _get_request_data(self, request): #pylint: disable=no-self-use
            return form_data
        return request.data

-    async def send(self, request: HttpRequest, **config: Any) -> AsyncHttpResponse:
+    async def send(self, request: HttpRequest, **config: Any) -> Optional[Any]:


Why not AsyncHttpResponse?

xiangyan99 · 2019-05-25T00:51:56Z

sdk/core/azure-core/azure/core/pipeline/transport/base.py

@@ -43,21 +43,22 @@
 # If one day we reach the point where "requests" can be skip totally,
 # might provide our own implementation
 from requests.structures import CaseInsensitiveDict
-from azure.core.pipeline import ABC, AbstractContextManager
+from azure.core.pipeline import ABC, AbstractContextManager, PipelineRequest, PipelineResponse


Maybe we should use PipelineRequestType instead of import?
Can this change pass PyLint?

yes, it does.

xiangyan99 · 2019-05-25T00:52:56Z

sdk/core/azure-core/azure/core/pipeline/transport/requests_basic.py

@@ -127,7 +129,7 @@ class RequestsTransport(HttpTransport):
    _protocols = ['http://', 'https://']

    def __init__(self, configuration=None, session=None, session_owner=True):
-        # type: (Optional[requests.Session]) -> None
+        # type: (Optional[Any], Optional[requests.Session], bool) -> None


This is configuration type, not any

xiangyan99 · 2019-05-25T00:54:19Z

sdk/core/azure-core/azure/core/pipeline/policies/base.py

@@ -31,23 +31,23 @@
 from typing import (TYPE_CHECKING, Generic, TypeVar, cast, IO, List, Union, Any, Mapping, Dict, Optional,  # pylint: disable=unused-import
                    Tuple, Callable, Iterator)

-from azure.core.pipeline import ABC
+from azure.core.pipeline import ABC, PipelineRequest, PipelineResponse


Maybe we should use PipelineRequestType

xiangyan99 · 2019-05-25T00:55:01Z

sdk/core/azure-core/azure/core/pipeline/policies/base.py

@@ -98,7 +98,7 @@ class RequestHistory(object):
    """

    def __init__(self, http_request, http_response=None, error=None, context=None):
-        # type: (PipelineRequest[HTTPRequestType], Exception, Optional[Dict[str, Any]]) -> None
+        # type: (PipelineRequest, PipelineResponse, Exception, Optional[Dict[str, Any]]) -> None


Optional[PipelineResponse]

xiangyan99 · 2019-05-25T00:55:20Z

sdk/core/azure-core/azure/core/pipeline/policies/custom_hook.py

@@ -26,6 +26,7 @@
 """
 This module is the requests implementation of Pipeline ABC
 """
+from azure.core.pipeline import PipelineRequest, PipelineResponse


PipelineRequestType?

…5456

…5456 (#28923) Co-authored-by: Mike Harder <[email protected]>

Mypy rebase

da7764d

rakshith91 requested a review from lmazuel as a code owner May 24, 2019 01:22

Merge branch 'master' into mypy-core

61464c5

rakshith91 mentioned this pull request May 24, 2019

Get successful MyPy pass for azure-core #5266

Closed

rakshith91 added 3 commits May 24, 2019 09:56

Merge branch 'master' into mypy-core

38d2641

use common type aliases

d8a4d34

Merge branch 'master' into mypy-core

785fddf

rakshith91 requested a review from bryevdv May 24, 2019 17:40

Merge branch 'master' into mypy-core

b29b5e3

rakshith91 self-assigned this May 24, 2019

rakshith91 added the Azure.Core label May 24, 2019

xiangyan99 requested changes May 24, 2019

View reviewed changes

xiangyan99 reviewed May 24, 2019

View reviewed changes

rakshith91 added 7 commits May 24, 2019 15:07

comments address

ddee45b

Merge branch 'master' into mypy-core

148c30d

changes

d454c4b

upgrade python for mypy

16d66d0

Merge branch 'master' into mypy-core

0bdc627

fix

b8fe814

small fix

4746204

xiangyan99 reviewed May 25, 2019

View reviewed changes

xiangyan99 approved these changes May 25, 2019

View reviewed changes

comments

02761e0

rakshith91 merged commit 76e055c into Azure:master May 25, 2019

rakshith91 deleted the mypy-core branch May 25, 2019 01:44

azure-sdk pushed a commit that referenced this pull request Feb 21, 2023

Add vmImage back to common perf.yml - Fixes #5466 - Partially reverts #…

20d0508

…5456

azure-sdk pushed a commit that referenced this pull request Feb 21, 2023

Add vmImage back to common perf.yml - Fixes #5466 - Partially reverts #…

f6dbd79

…5456

azure-sdk added a commit that referenced this pull request Feb 21, 2023

Add vmImage back to common perf.yml - Fixes #5466 - Partially reverts #…

4c1ae6b

…5456 (#28923) Co-authored-by: Mike Harder <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get successful mypy pass on azure core #5466

Get successful mypy pass on azure core #5466

rakshith91 commented May 24, 2019

adxsdk6 commented May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019

rakshith91 May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019 •

edited

Loading

rakshith91 May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019

xiangyan99 May 24, 2019 •

edited

Loading

xiangyan99 May 24, 2019

xiangyan99 May 25, 2019

xiangyan99 May 25, 2019

rakshith91 May 25, 2019

xiangyan99 May 25, 2019

xiangyan99 May 25, 2019

xiangyan99 May 25, 2019

xiangyan99 May 25, 2019

Get successful mypy pass on azure core #5466

Get successful mypy pass on azure core #5466

Conversation

rakshith91 commented May 24, 2019

adxsdk6 commented May 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiangyan99 May 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiangyan99 May 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiangyan99 May 24, 2019 •

edited

Loading

xiangyan99 May 24, 2019 •

edited

Loading