Skip to content

Commit

Permalink
Don't rely on vendored requests
Browse files Browse the repository at this point in the history
  • Loading branch information
joguSD committed Aug 21, 2018
1 parent 3a2d5ab commit c8225e8
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 46 deletions.
6 changes: 4 additions & 2 deletions awscli/paramfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
import os
import copy

from botocore.vendored import requests
from botocore.awsrequest import AWSRequest
from botocore.httpsession import URLLib3Session
from botocore.exceptions import ProfileNotFound
from awscli.compat import six

Expand Down Expand Up @@ -225,7 +226,8 @@ def get_file(prefix, path, mode):

def get_uri(prefix, uri):
try:
r = requests.get(uri)
session = URLLib3Session()
r = session.send(AWSRequest('GET', uri).prepare())
if r.status_code == 200:
return r.text
else:
Expand Down
8 changes: 4 additions & 4 deletions awscli/testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
from botocore.exceptions import ClientError
from botocore.exceptions import WaiterError
import botocore.loaders
from botocore.vendored import requests
from botocore.awsrequest import AWSResponse

import awscli.clidriver
from awscli.plugin import load_plugins
Expand Down Expand Up @@ -357,8 +357,7 @@ def setUp(self):
}
self.environ_patch = mock.patch('os.environ', self.environ)
self.environ_patch.start()
self.http_response = requests.models.Response()
self.http_response.status_code = 200
self.http_response = AWSResponse(None, 200, {}, None)
self.parsed_response = {}
self.make_request_patch = mock.patch('botocore.endpoint.Endpoint.make_request')
self.make_request_is_patched = False
Expand Down Expand Up @@ -475,7 +474,8 @@ def setUp(self):
}
self.environ_patch = mock.patch('os.environ', self.environ)
self.environ_patch.start()
self.send_patch = mock.patch('botocore.endpoint.Session.send')
# TODO: fix this patch when we have a better way to stub out responses
self.send_patch = mock.patch('botocore.endpoint.Endpoint._send')
self.send_is_patched = False
self.driver = create_clidriver()

Expand Down
3 changes: 2 additions & 1 deletion tests/functional/configservice/test_subscribe.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def test_subscribe_when_bucket_exists_and_sns_topic_arn_provided(self):
)

def test_subscribe_when_bucket_needs_to_be_created(self):
with mock.patch('botocore.endpoint.Session.send') as \
# TODO: fix this patch when we have a better way to stub out responses
with mock.patch('botocore.endpoint.Endpoint._send') as \
http_session_send_patch:
# Mock for HeadBucket request
head_bucket_response = mock.Mock()
Expand Down
40 changes: 18 additions & 22 deletions tests/functional/test_paramfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,25 @@ def test_does_not_prefixes_when_none_in_param(self):
expected_param='foobar'
)

@patch('awscli.paramfile.requests.get')
def test_does_use_http_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_use_http_prefix(self, mock_send):
content = 'http_content'
mock_get.return_value = FakeResponse(content=content)
mock_send.return_value = FakeResponse(content=content)
param = 'http://foobar.com'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=content
)
mock_get.assert_called_with(param)

@patch('awscli.paramfile.requests.get')
def test_does_use_https_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_use_https_prefix(self, mock_send):
content = 'https_content'
mock_get.return_value = FakeResponse(content=content)
mock_send.return_value = FakeResponse(content=content)
param = 'https://foobar.com'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=content
)
mock_get.assert_called_with(param)

def test_does_use_file_prefix(self):
path = self.files.create_file('foobar.txt', 'file content')
Expand Down Expand Up @@ -112,23 +110,23 @@ def test_does_not_prefixes_when_none_in_param(self):
expected_param=param
)

@patch('awscli.paramfile.requests.get')
def test_does_not_use_http_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_not_use_http_prefix(self, mock_send):
param = 'http://foobar'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=param
)
mock_get.assert_not_called()
mock_send.assert_not_called()

@patch('awscli.paramfile.requests.get')
def test_does_not_use_https_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_not_use_https_prefix(self, mock_send):
param = 'https://foobar'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=param
)
mock_get.assert_not_called()
mock_send.assert_not_called()

def test_does_use_file_prefix(self):
path = self.files.create_file('foobar.txt', 'file content')
Expand Down Expand Up @@ -161,27 +159,25 @@ def setUp(self):
def test_does_not_prefixes_when_none_in_param(self):
self.assert_param_expansion_is_correct('foobar', 'foobar')

@patch('awscli.paramfile.requests.get')
def test_does_use_http_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_use_http_prefix(self, mock_send):
content = 'http_content'
mock_get.return_value = FakeResponse(content=content)
mock_send.return_value = FakeResponse(content=content)
param = 'http://foobar.com'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=content
)
mock_get.assert_called_with(param)

@patch('awscli.paramfile.requests.get')
def test_does_use_https_prefix(self, mock_get):
@patch('awscli.paramfile.URLLib3Session.send')
def test_does_use_https_prefix(self, mock_send):
content = 'https_content'
mock_get.return_value = FakeResponse(content=content)
mock_send.return_value = FakeResponse(content=content)
param = 'https://foobar.com'
self.assert_param_expansion_is_correct(
provided_param=param,
expected_param=content
)
mock_get.assert_called_with(param)

def test_does_use_file_prefix(self):
path = self.files.create_file('foobar.txt', 'file content')
Expand Down
1 change: 0 additions & 1 deletion tests/unit/customizations/emr/test_add_instance_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
CONSTANTS
import json
from mock import patch
from botocore.vendored import requests

INSTANCE_GROUPS_WITH_AUTOSCALING_POLICY = (
' InstanceGroupType=TASK,InstanceType=d2.xlarge,InstanceCount=2,'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import os
import json
from mock import patch
from botocore.vendored import requests


DEFAULT_CLUSTER_NAME = "Development Cluster"
Expand Down
5 changes: 2 additions & 3 deletions tests/unit/customizations/emr/test_create_default_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import mock

from botocore.compat import json
from botocore.vendored import requests
from botocore.awsrequest import AWSResponse
from botocore.exceptions import ClientError

import awscli.customizations.emr.emrutils as emrutils
Expand Down Expand Up @@ -77,8 +77,7 @@
}
]

http_response = requests.models.Response()
http_response.status_code = 200
http_response = AWSResponse(None, 200, {}, None)

CN_EC2_ROLE_ARN = ('arn:aws-cn:iam::aws:policy/service-role/'
'AmazonElasticMapReduceforEC2Role')
Expand Down
7 changes: 3 additions & 4 deletions tests/unit/test_clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import mock
import nose
from awscli.compat import six
from botocore.vendored.requests import models
from botocore.awsrequest import AWSResponse
from botocore.exceptions import NoCredentialsError
from botocore.compat import OrderedDict
import botocore.model
Expand Down Expand Up @@ -717,8 +717,7 @@ def setUp(self):
self.endpoint = self.create_endpoint.return_value
self.endpoint.host = 'https://example.com'
# Have the endpoint give a dummy empty response.
http_response = models.Response()
http_response.status_code = 200
http_response = AWSResponse(None, 200, {}, None)
self.endpoint.make_request.return_value = (
http_response, {})

Expand Down Expand Up @@ -812,7 +811,7 @@ def test_http_file_param_does_not_exist(self):
error_msg = ("Error parsing parameter '--filters': "
"Unable to retrieve http://does/not/exist.json: "
"received non 200 status code of 404")
with mock.patch('botocore.vendored.requests.get') as get:
with mock.patch('awscli.paramfile.URLLib3Session.send') as get:
get.return_value.status_code = 404
self.assert_params_for_cmd(
'ec2 describe-instances --filters http://does/not/exist.json',
Expand Down
12 changes: 5 additions & 7 deletions tests/unit/test_paramfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ def test_non_string_type_returns_none(self):

class TestHTTPBasedResourceLoading(unittest.TestCase):
def setUp(self):
self.requests_patch = mock.patch('awscli.paramfile.requests')
self.requests_mock = self.requests_patch.start()
self.session_patch = mock.patch('awscli.paramfile.URLLib3Session.send')
self.session_mock = self.session_patch.start()
self.response = mock.Mock(status_code=200)
self.requests_mock.get.return_value = self.response
self.session_mock.return_value = self.response

def tearDown(self):
self.requests_patch.stop()
self.session_patch.stop()

def get_paramfile(self, path):
return get_paramfile(path, REMOTE_PREFIX_MAP.copy())
Expand All @@ -85,21 +85,19 @@ def test_resource_from_http(self):
self.response.text = 'http contents'
loaded = self.get_paramfile('http://foo.bar.baz')
self.assertEqual(loaded, 'http contents')
self.requests_mock.get.assert_called_with('http://foo.bar.baz')

def test_resource_from_https(self):
self.response.text = 'http contents'
loaded = self.get_paramfile('https://foo.bar.baz')
self.assertEqual(loaded, 'http contents')
self.requests_mock.get.assert_called_with('https://foo.bar.baz')

def test_non_200_raises_error(self):
self.response.status_code = 500
with self.assertRaisesRegexp(ResourceLoadingError, 'foo\.bar\.baz'):
self.get_paramfile('https://foo.bar.baz')

def test_connection_error_raises_error(self):
self.requests_mock.get.side_effect = Exception("Connection error.")
self.session_mock.side_effect = Exception("Connection error.")
with self.assertRaisesRegexp(ResourceLoadingError, 'foo\.bar\.baz'):
self.get_paramfile('https://foo.bar.baz')

Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

class TestCreateBucket(BaseCLIDriverTest):
def test_bucket_already_owned_by_you(self):
with mock.patch('botocore.endpoint.Session.send') as _send:
# TODO: fix this patch when we have a better way to stub out responses
with mock.patch('botocore.endpoint.Endpoint._send') as _send:
_send.side_effect = [
mock.Mock(status_code=500, headers={}, content=b''),
mock.Mock(
Expand Down

0 comments on commit c8225e8

Please sign in to comment.