-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get successful mypy pass on azure core #5466
Changes from 4 commits
da7764d
61464c5
38d2641
d8a4d34
785fddf
b29b5e3
ddee45b
148c30d
d454c4b
16d66d0
0bdc627
b8fe814
4746204
02761e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
__path__ = __import__('pkgutil').extend_path(__path__, __name__) | ||
__path__ = __import__('pkgutil').extend_path(__path__, __name__) # type: ignore |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
# -------------------------------------------------------------------------- | ||
# | ||
# Copyright (c) Microsoft Corporation. All rights reserved. | ||
# | ||
# The MIT License (MIT) | ||
# | ||
# Permission is hereby granted, free of charge, to any person obtaining a copy | ||
# of this software and associated documentation files (the ""Software""), to | ||
# deal in the Software without restriction, including without limitation the | ||
# rights to use, copy, modify, merge, publish, distribute, sublicense, and/or | ||
# sell copies of the Software, and to permit persons to whom the Software is | ||
# furnished to do so, subject to the following conditions: | ||
# | ||
# The above copyright notice and this permission notice shall be included in | ||
# all copies or substantial portions of the Software. | ||
# | ||
# THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS | ||
# IN THE SOFTWARE. | ||
# | ||
# -------------------------------------------------------------------------- | ||
from typing import List, Union, Optional, TypeVar, Callable | ||
from azure.core.pipeline.policies import AsyncHTTPPolicy, SansIOHTTPPolicy | ||
from azure.core.exceptions import ( | ||
ServiceRequestError, | ||
ServiceResponseError | ||
) | ||
import requests | ||
from msrest.serialization import Model # type: ignore # pylint: disable=unused-import | ||
|
||
AsyncHTTPResponseType = TypeVar("AsyncHTTPResponseType") | ||
HTTPRequestType = TypeVar("HTTPRequestType") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HTTPRequestType is already defined in sdk/core/azure-core/azure/core/pipeline/init.py There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And seems like the only one uses AsyncHTTPResponseType & ImplPoliciesType is base_async.py |
||
|
||
ErrorType = Optional[Union[ServiceRequestError, ServiceResponseError]] | ||
ImplPoliciesType = List[AsyncHTTPPolicy[HTTPRequestType, AsyncHTTPResponseType]] #pylint: disable=unsubscriptable-object | ||
PoliciesType = List[Union[AsyncHTTPPolicy, SansIOHTTPPolicy]] | ||
DeserializationCallbackType = Union[Model, Callable[[requests.Response], Model]] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,7 @@ def __init__(self, policy): | |
self._policy = policy | ||
|
||
def send(self, request): | ||
# type: (PipelineRequest[HTTPRequestType], Any) -> PipelineResponse[HTTPRequestType, HTTPResponseType] | ||
# type: (PipelineRequest) -> PipelineResponse | ||
self._policy.on_request(request) | ||
try: | ||
response = self.next.send(request) | ||
|
@@ -60,7 +60,7 @@ def send(self, request): | |
class _TransportRunner(HTTPPolicy): | ||
|
||
def __init__(self, sender): | ||
# type: (HttpTransport) -> None | ||
# type: (Any) -> None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is sender any? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had HTTPSender in msrest which we don't have anymore. Can't find a relevant sender class. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think HttpTransport is correct |
||
super(_TransportRunner, self).__init__() | ||
self._sender = sender | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. included this |
||
self._impl_policies = [] # type: List[HTTPPolicy] | ||
self._transport = transport # type: HTTPPolicy | ||
|
||
|
@@ -105,6 +105,6 @@ def __exit__(self, *exc_details): # pylint: disable=arguments-differ | |
def run(self, request, **kwargs): | ||
# type: (HTTPRequestType, Any) -> PipelineResponse | ||
context = PipelineContext(self._transport, **kwargs) | ||
pipeline_request = PipelineRequest(request, context) # type: PipelineRequest[HTTPRequestType] | ||
pipeline_request = PipelineRequest(request, context) # type: PipelineRequest | ||
first_node = self._impl_policies[0] if self._impl_policies else _TransportRunner(self._transport) | ||
return first_node.send(pipeline_request) # type: ignore |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AsyncHTTPResponseType is already defined in azure.core.common |
||
HTTPRequestType = TypeVar("HTTPRequestType") | ||
|
@@ -88,8 +89,8 @@ class AsyncPipeline(AbstractAsyncContextManager, Generic[HTTPRequestType, AsyncH | |
of the HTTP sender. | ||
""" | ||
|
||
def __init__(self, transport, policies: List[Union[AsyncHTTPPolicy, SansIOHTTPPolicy]] = None) -> None: | ||
self._impl_policies = [] # type: List[AsyncHTTPPolicy[HTTPRequestType, AsyncHTTPResponseType]] | ||
def __init__(self, transport, policies: PoliciesType = None) -> None: | ||
self._impl_policies = [] # type: ImplPoliciesType | ||
self._transport = transport | ||
|
||
for policy in (policies or []): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should use PipelineRequestType |
||
|
||
HTTPResponseType = TypeVar("HTTPResponseType") | ||
HTTPRequestType = TypeVar("HTTPRequestType") | ||
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
|
||
class HTTPPolicy(ABC, Generic[HTTPRequestType, HTTPResponseType]): | ||
class HTTPPolicy(ABC, Generic[HTTPRequestType, HTTPResponseType]): # type: ignore | ||
"""An HTTP policy ABC. | ||
""" | ||
def __init__(self): | ||
self.next = None | ||
|
||
@abc.abstractmethod | ||
def send(self, request): | ||
# type: (PipelineRequest[HTTPRequestType]) -> PipelineResponse[HTTPRequestType, HTTPResponseType] | ||
# type: (PipelineRequest) -> PipelineResponse | ||
"""Mutate the request. | ||
Context content is dependent on the HttpTransport. | ||
""" | ||
|
@@ -64,18 +64,18 @@ class SansIOHTTPPolicy(Generic[HTTPRequestType, HTTPResponseType]): | |
""" | ||
|
||
def on_request(self, request, **kwargs): | ||
# type: (PipelineRequest[HTTPRequestType], Any) -> None | ||
# type: (PipelineRequest, Any) -> None | ||
"""Is executed before sending the request to next policy. | ||
""" | ||
|
||
def on_response(self, request, response, **kwargs): | ||
# type: (PipelineRequest[HTTPRequestType], PipelineResponse[HTTPRequestType, HTTPResponseType], Any) -> None | ||
# type: (PipelineRequest, PipelineResponse, Any) -> None | ||
"""Is executed after the request comes back from the policy. | ||
""" | ||
|
||
#pylint: disable=no-self-use | ||
def on_exception(self, _request, **kwargs): #pylint: disable=unused-argument | ||
# type: (PipelineRequest[HTTPRequestType], Any) -> bool | ||
# type: (PipelineRequest, Any) -> bool | ||
"""Is executed if an exception comes back from the following | ||
policy. | ||
Return True if the exception has been handled and should not | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional[PipelineResponse] |
||
self.http_request = copy.deepcopy(http_request) | ||
self.http_response = http_response | ||
self.error = error | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
""" | ||
This module is the requests implementation of Pipeline ABC | ||
""" | ||
from azure.core.pipeline import PipelineRequest, PipelineResponse | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PipelineRequestType? |
||
from .base import SansIOHTTPPolicy | ||
|
||
class CustomHookPolicy(SansIOHTTPPolicy): | ||
|
@@ -35,12 +36,12 @@ class CustomHookPolicy(SansIOHTTPPolicy): | |
def __init__(self, **kwargs): # pylint: disable=unused-argument | ||
self._callback = None | ||
|
||
def on_request(self, request): # pylint: disable=arguments-differ | ||
def on_request(self, request): # type: ignore # pylint: disable=arguments-differ | ||
# type: (PipelineRequest) -> None | ||
self._callback = request.context.options.pop('raw_response_hook', None) | ||
self._callback = request.context.options.pop('raw_response_hook', None) # type: ignore | ||
|
||
def on_response(self, request, response): # pylint: disable=arguments-differ | ||
def on_response(self, request, response): # type: ignore # pylint: disable=arguments-differ | ||
# type: (PipelineRequest, PipelineResponse) -> None | ||
if self._callback: | ||
self._callback(response) | ||
request.context.options.update({'raw_response_hook': self._callback}) | ||
request.context.options.update({'raw_response_hook': self._callback}) # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need "try" for AsyncHTTPPolicy?