openstack/swift
Code Issues Proposed changes

py3: port internal_client

Browse Source 
Change-Id: I4e69beac30be1d8726849aacb603af5e2aeecaf8
This commit is contained in:
Tim Burke 2018-09-15 00:45:00 -06:00
parent ddd5cc1592
commit ae496630b6
4 changed files with 127 additions and 95 deletions

30
swift/common/internal_client.py

@ -28,7 +28,7 @@ from zlib import compressobj
from swift.common.exceptions import ClientException
from swift.common.http import (HTTP_NOT_FOUND, HTTP_MULTIPLE_CHOICES,
is_server_error)
from swift.common.swob import Request
from swift.common.swob import Request, bytes_to_wsgi
from swift.common.utils import quote, closing_if_possible
from swift.common.wsgi import loadapp, pipeline_property
@ -95,7 +95,7 @@ class CompressingFileReader(object):
"""
if self.done:
return ''
return b''
x = self._f.read(*a, **kw)
if x:
self.crc32 = zlib.crc32(x, self.crc32) & 0xffffffff
@ -112,19 +112,21 @@ class CompressingFileReader(object):
self.done = True
if self.first:
self.first = False
header = '\037\213\010\000\000\000\000\000\002\377'
header = b'\037\213\010\000\000\000\000\000\002\377'
compressed = header + compressed
return compressed
def __iter__(self):
return self
def next(self):
def __next__(self):
chunk = self.read(self.chunk_size)
if chunk:
return chunk
raise StopIteration
next = __next__
def seek(self, offset, whence=0):
if not (offset == 0 and whence == 0):
raise NotImplementedError('Seek implemented on offset 0 only')
@ -276,17 +278,25 @@ class InternalClient(object):
:raises Exception: Exception is raised when code fails in an
unexpected way.
"""
if not isinstance(marker, bytes):
marker = marker.encode('utf8')
if not isinstance(end_marker, bytes):
end_marker = end_marker.encode('utf8')
if not isinstance(prefix, bytes):
prefix = prefix.encode('utf8')
while True:
resp = self.make_request(
'GET', '%s?format=json&marker=%s&end_marker=%s&prefix=%s' %
(path, quote(marker), quote(end_marker), quote(prefix)),
(path, bytes_to_wsgi(quote(marker)),
bytes_to_wsgi(quote(end_marker)),
bytes_to_wsgi(quote(prefix))),
{}, acceptable_statuses)
if not resp.status_int == 200:
if resp.status_int >= HTTP_MULTIPLE_CHOICES:
''.join(resp.app_iter)
b''.join(resp.app_iter)
break
data = json.loads(resp.body)
data = json.loads(resp.body.decode('ascii'))
if not data:
break
for item in data:
@ -685,7 +695,7 @@ class InternalClient(object):
if not resp.status_int // 100 == 2:
return
last_part = ''
last_part = b''
compressed = obj.endswith('.gz')
# magic in the following zlib.decompressobj argument is courtesy of
# Python decompressing gzip chunk-by-chunk
@ -694,7 +704,7 @@ class InternalClient(object):
for chunk in resp.app_iter:
if compressed:
chunk = d.decompress(chunk)
parts = chunk.split('\n')
parts = chunk.split(b'\n')
if len(parts) == 1:
last_part = last_part + parts[0]
else:
@ -832,7 +842,7 @@ class SimpleClient(object):
body = conn.read()
info = conn.info()
try:
body_data = json.loads(body)
body_data = json.loads(body.decode('ascii'))
except ValueError:
body_data = None
trans_stop = time()

2
test/unit/common/middleware/helpers.py

@ -29,6 +29,8 @@ from test.unit import FakeLogger, FakeRing
class LeakTrackingIter(object):
def __init__(self, inner_iter, mark_closed, path):
if isinstance(inner_iter, bytes):
inner_iter = (inner_iter, )
self.inner_iter = inner_iter
self.mark_closed = mark_closed
self.path = path

189
test/unit/common/test_internal_client.py

@ -19,11 +19,10 @@ import unittest
import zlib
from textwrap import dedent
import os
from itertools import izip_longest
import six
from six import StringIO
from six.moves import range
from six import BytesIO
from six.moves import range, zip_longest
from six.moves.urllib.parse import quote, parse_qsl
from test.unit import FakeLogger
from swift.common import exceptions, internal_client, swob
@ -48,7 +47,7 @@ class FakeConn(object):
self.body = body
def read(self):
return json.dumps(self.body)
return json.dumps(self.body).encode('ascii')
def info(self):
return {}
@ -82,7 +81,7 @@ def make_path_info(account, container=None, obj=None):
# FakeSwift keys on PATH_INFO - which is *encoded* but unquoted
path = '/v1/%s' % '/'.join(
p for p in (account, container, obj) if p)
return path.encode('utf-8')
return swob.bytes_to_wsgi(path.encode('utf-8'))
def get_client_app():
@ -179,7 +178,7 @@ class TestCompressingfileReader(unittest.TestCase):
old_compressobj = internal_client.compressobj
internal_client.compressobj = compressobj.method
f = StringIO('')
f = BytesIO(b'')
fobj = internal_client.CompressingFileReader(f)
self.assertEqual(f, fobj._f)
@ -192,21 +191,20 @@ class TestCompressingfileReader(unittest.TestCase):
internal_client.compressobj = old_compressobj
def test_read(self):
exp_data = 'abcdefghijklmnopqrstuvwxyz'
exp_data = b'abcdefghijklmnopqrstuvwxyz'
fobj = internal_client.CompressingFileReader(
StringIO(exp_data), chunk_size=5)
BytesIO(exp_data), chunk_size=5)
data = ''
d = zlib.decompressobj(16 + zlib.MAX_WBITS)
for chunk in fobj.read():
data += d.decompress(chunk)
data = b''.join(d.decompress(chunk)
for chunk in iter(fobj.read, b''))
self.assertEqual(exp_data, data)
def test_seek(self):
exp_data = 'abcdefghijklmnopqrstuvwxyz'
exp_data = b'abcdefghijklmnopqrstuvwxyz'
fobj = internal_client.CompressingFileReader(
StringIO(exp_data), chunk_size=5)
BytesIO(exp_data), chunk_size=5)
# read a couple of chunks only
for _ in range(2):
@ -214,15 +212,14 @@ class TestCompressingfileReader(unittest.TestCase):
# read whole thing after seek and check data
fobj.seek(0)
data = ''
d = zlib.decompressobj(16 + zlib.MAX_WBITS)
for chunk in fobj.read():
data += d.decompress(chunk)
data = b''.join(d.decompress(chunk)
for chunk in iter(fobj.read, b''))
self.assertEqual(exp_data, data)
def test_seek_not_implemented_exception(self):
fobj = internal_client.CompressingFileReader(
StringIO(''), chunk_size=5)
BytesIO(b''), chunk_size=5)
self.assertRaises(NotImplementedError, fobj.seek, 10)
self.assertRaises(NotImplementedError, fobj.seek, 0, 10)
@ -409,13 +406,22 @@ class TestInternalClient(unittest.TestCase):
_, resp_body = sc.base_request('GET', full_listing=True)
self.assertEqual(body1 + body2, resp_body)
self.assertEqual(3, mock_urlopen.call_count)
actual_requests = map(
lambda call: call[0][0], mock_urlopen.call_args_list)
self.assertEqual('/?format=json', actual_requests[0].get_selector())
self.assertEqual(
'/?format=json&marker=c', actual_requests[1].get_selector())
self.assertEqual(
'/?format=json&marker=d', actual_requests[2].get_selector())
actual_requests = [call[0][0] for call in mock_urlopen.call_args_list]
if six.PY2:
# The get_selector method was deprecated in favor of a selector
# attribute in py31 and removed in py34
self.assertEqual(
'/?format=json', actual_requests[0].get_selector())
self.assertEqual(
'/?format=json&marker=c', actual_requests[1].get_selector())
self.assertEqual(
'/?format=json&marker=d', actual_requests[2].get_selector())
else:
self.assertEqual('/?format=json', actual_requests[0].selector)
self.assertEqual(
'/?format=json&marker=c', actual_requests[1].selector)
self.assertEqual(
'/?format=json&marker=d', actual_requests[2].selector)
def test_make_request_method_path_headers(self):
class InternalClient(internal_client.InternalClient):
@ -452,7 +458,7 @@ class TestInternalClient(unittest.TestCase):
self.request_tries = 3
def fake_app(self, env, start_response):
body = 'fake error response'
body = b'fake error response'
start_response('409 Conflict',
[('Content-Length', str(len(body)))])
return [body]
@ -466,7 +472,7 @@ class TestInternalClient(unittest.TestCase):
# succeed (assuming the failure was due to clock skew between servers)
expected = (' HTTP/1.0 409 ',)
loglines = client.logger.get_lines_for_level('info')
for expected, logline in izip_longest(expected, loglines):
for expected, logline in zip_longest(expected, loglines):
if not expected:
self.fail('Unexpected extra log line: %r' % logline)
self.assertIn(expected, logline)
@ -484,7 +490,7 @@ class TestInternalClient(unittest.TestCase):
self.closed_paths = []
def fake_app(self, env, start_response):
body = 'fake error response'
body = b'fake error response'
start_response(self.resp_status,
[('Content-Length', str(len(body)))])
return LeakTrackingIter(body, self.closed_paths.append,
@ -506,11 +512,11 @@ class TestInternalClient(unittest.TestCase):
self.assertEqual(closed_paths, [])
# ...and it'll be on us (the caller) to close (for example, by using
# swob.Response's body property)
self.assertEqual(resp.body, 'fake error response')
self.assertEqual(resp.body, b'fake error response')
self.assertEqual(closed_paths, ['/cont/obj'])
expected = (' HTTP/1.0 200 ', )
for expected, logline in izip_longest(expected, loglines):
for expected, logline in zip_longest(expected, loglines):
if not expected:
self.fail('Unexpected extra log line: %r' % logline)
self.assertIn(expected, logline)
@ -521,7 +527,7 @@ class TestInternalClient(unittest.TestCase):
self.assertEqual(closed_paths, ['/cont/obj'] * 3)
expected = (' HTTP/1.0 503 ', ' HTTP/1.0 503 ', ' HTTP/1.0 503 ', )
for expected, logline in izip_longest(expected, loglines):
for expected, logline in zip_longest(expected, loglines):
if not expected:
self.fail('Unexpected extra log line: %r' % logline)
self.assertIn(expected, logline)
@ -548,22 +554,21 @@ class TestInternalClient(unittest.TestCase):
client.make_request('GET', '/', {}, (400, 200))
client.make_request('GET', '/', {}, (400, 2))
try:
with self.assertRaises(internal_client.UnexpectedResponse) \
as raised:
client.make_request('GET', '/', {}, (400,))
except Exception as err:
pass
self.assertEqual(200, err.resp.status_int)
try:
self.assertEqual(200, raised.exception.resp.status_int)
with self.assertRaises(internal_client.UnexpectedResponse) \
as raised:
client.make_request('GET', '/', {}, (201,))
except Exception as err:
pass
self.assertEqual(200, err.resp.status_int)
try:
self.assertEqual(200, raised.exception.resp.status_int)
with self.assertRaises(internal_client.UnexpectedResponse) \
as raised:
client.make_request('GET', '/', {}, (111,))
except Exception as err:
self.assertTrue(str(err).startswith('Unexpected response'))
else:
self.fail("Expected the UnexpectedResponse")
self.assertTrue(str(raised.exception).startswith(
'Unexpected response'))
finally:
internal_client.sleep = old_sleep
@ -678,7 +683,7 @@ class TestInternalClient(unittest.TestCase):
def fake_app(self, environ, start_response):
start_response('404 Not Found', [('x-foo', 'bar')])
return ['nope']
return [b'nope']
client = InternalClient()
self.assertRaises(internal_client.UnexpectedResponse,
@ -717,7 +722,7 @@ class TestInternalClient(unittest.TestCase):
return self.responses.pop(0)
exp_items = []
responses = [Response(200, json.dumps([])), ]
responses = [Response(200, json.dumps([]).encode('ascii')), ]
items = []
client = InternalClient(self, responses)
for item in client._iter_items('/'):
@ -730,7 +735,7 @@ class TestInternalClient(unittest.TestCase):
data = [
{'name': 'item%02d' % (2 * i)},
{'name': 'item%02d' % (2 * i + 1)}]
responses.append(Response(200, json.dumps(data)))
responses.append(Response(200, json.dumps(data).encode('ascii')))
exp_items.extend(data)
responses.append(Response(204, ''))
@ -744,7 +749,7 @@ class TestInternalClient(unittest.TestCase):
class Response(object):
def __init__(self, status_int, body):
self.status_int = status_int
self.body = body
self.body = body.encode('ascii')
class InternalClient(internal_client.InternalClient):
def __init__(self, test, paths, responses):
@ -766,7 +771,8 @@ class TestInternalClient(unittest.TestCase):
]
responses = [
Response(200, json.dumps([{'name': 'one\xc3\xa9'}, ])),
Response(200, json.dumps([{
'name': b'one\xc3\xa9'.decode('utf8')}, ])),
Response(200, json.dumps([{'name': 'two'}, ])),
Response(204, ''),
]
@ -776,13 +782,13 @@ class TestInternalClient(unittest.TestCase):
for item in client._iter_items('/', marker='start', end_marker='end'):
items.append(item['name'].encode('utf8'))
self.assertEqual('one\xc3\xa9 two'.split(), items)
self.assertEqual(b'one\xc3\xa9 two'.split(), items)
def test_iter_items_with_markers_and_prefix(self):
class Response(object):
def __init__(self, status_int, body):
self.status_int = status_int
self.body = body
self.body = body.encode('ascii')
class InternalClient(internal_client.InternalClient):
def __init__(self, test, paths, responses):
@ -807,7 +813,8 @@ class TestInternalClient(unittest.TestCase):
]
responses = [
Response(200, json.dumps([{'name': 'prefixed_one\xc3\xa9'}, ])),
Response(200, json.dumps([{
'name': b'prefixed_one\xc3\xa9'.decode('utf8')}, ])),
Response(200, json.dumps([{'name': 'prefixed_two'}, ])),
Response(204, ''),
]
@ -819,7 +826,7 @@ class TestInternalClient(unittest.TestCase):
prefix='prefixed_'):
items.append(item['name'].encode('utf8'))
self.assertEqual('prefixed_one\xc3\xa9 prefixed_two'.split(), items)
self.assertEqual(b'prefixed_one\xc3\xa9 prefixed_two'.split(), items)
def test_iter_item_read_response_if_status_is_acceptable(self):
class Response(object):
@ -848,11 +855,12 @@ class TestInternalClient(unittest.TestCase):
def generate_resp_body():
for i in range(1, 5):
yield str(i)
yield str(i).encode('ascii')
num_list.append(i)
exp_items = []
responses = [Response(204, json.dumps([]), generate_resp_body())]
responses = [Response(204, json.dumps([]).encode('ascii'),
generate_resp_body())]
items = []
client = InternalClient(self, responses)
for item in client._iter_items('/'):
@ -860,13 +868,15 @@ class TestInternalClient(unittest.TestCase):
self.assertEqual(exp_items, items)
self.assertEqual(len(num_list), 0)
responses = [Response(300, json.dumps([]), generate_resp_body())]
responses = [Response(300, json.dumps([]).encode('ascii'),
generate_resp_body())]
client = InternalClient(self, responses)
self.assertRaises(internal_client.UnexpectedResponse,
next, client._iter_items('/'))
exp_items = []
responses = [Response(404, json.dumps([]), generate_resp_body())]
responses = [Response(404, json.dumps([]).encode('ascii'),
generate_resp_body())]
items = []
client = InternalClient(self, responses)
for item in client._iter_items('/'):
@ -1208,7 +1218,7 @@ class TestInternalClient(unittest.TestCase):
path_info = make_path_info(account, container, obj)
client, app = get_client_app()
headers = {'foo': 'bar'}
body = 'some_object_body'
body = b'some_object_body'
params = {'symlink': 'get'}
app.register('GET', path_info, swob.HTTPOk, headers, body)
req_headers = {'x-important-header': 'some_important_value'}
@ -1217,7 +1227,7 @@ class TestInternalClient(unittest.TestCase):
self.assertEqual(status_int // 100, 2)
for k, v in headers.items():
self.assertEqual(v, resp_headers[k])
self.assertEqual(''.join(obj_iter), body)
self.assertEqual(b''.join(obj_iter), body)
self.assertEqual(resp_headers['content-length'], str(len(body)))
self.assertEqual(app.call_count, 1)
req_headers.update({
@ -1237,9 +1247,9 @@ class TestInternalClient(unittest.TestCase):
def fake_app(self, env, start_response):
start_response('200 Ok', [('Content-Length', '0')])
return ['%s\n' % x for x in self.lines]
return [b'%s\n' % x for x in self.lines]
lines = 'line1 line2 line3'.split()
lines = b'line1 line2 line3'.split()
client = InternalClient(lines)
ret_lines = []
for line in client.iter_object_lines('account', 'container', 'object'):
@ -1257,9 +1267,9 @@ class TestInternalClient(unittest.TestCase):
def fake_app(self, env, start_response):
start_response('200 Ok', [('Content-Length', '0')])
return internal_client.CompressingFileReader(
StringIO('\n'.join(self.lines)))
BytesIO(b'\n'.join(self.lines)))
lines = 'line1 line2 line3'.split()
lines = b'line1 line2 line3'.split()
client = InternalClient(lines)
ret_lines = []
for line in client.iter_object_lines(
@ -1356,8 +1366,8 @@ class TestInternalClient(unittest.TestCase):
class TestGetAuth(unittest.TestCase):
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch('eventlet.green.urllib2.Request')
@mock.patch.object(urllib2, 'urlopen')
@mock.patch.object(urllib2, 'Request')
def test_ok(self, request, urlopen):
def getheader(name):
d = {'X-Storage-Url': 'url', 'X-Auth-Token': 'token'}
@ -1392,7 +1402,7 @@ class TestSimpleClient(unittest.TestCase):
with mock.patch('swift.common.internal_client.time', mock_time):
# basic request, only url as kwarg
request.return_value.get_type.return_value = "http"
urlopen.return_value.read.return_value = ''
urlopen.return_value.read.return_value = b''
urlopen.return_value.getcode.return_value = 200
urlopen.return_value.info.return_value = {'content-length': '345'}
sc = internal_client.SimpleClient(url='http://127.0.0.1')
@ -1411,7 +1421,7 @@ class TestSimpleClient(unittest.TestCase):
'123 345 1401224050.98 1401224051.98 1.0 -',), {})])
# Check if JSON is decoded
urlopen.return_value.read.return_value = '{}'
urlopen.return_value.read.return_value = b'{}'
retval = sc.retry_request(method)
self.assertEqual([{'content-length': '345'}, {}], retval)
@ -1447,18 +1457,18 @@ class TestSimpleClient(unittest.TestCase):
data=None)
self.assertEqual([{'content-length': '345'}, {}], retval)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch('eventlet.green.urllib2.Request')
@mock.patch.object(urllib2, 'urlopen')
@mock.patch.object(urllib2, 'Request')
def test_get(self, request, urlopen):
self._test_get_head(request, urlopen, 'GET')
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch('eventlet.green.urllib2.Request')
@mock.patch.object(urllib2, 'urlopen')
@mock.patch.object(urllib2, 'Request')
def test_head(self, request, urlopen):
self._test_get_head(request, urlopen, 'HEAD')
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch('eventlet.green.urllib2.Request')
@mock.patch.object(urllib2, 'urlopen')
@mock.patch.object(urllib2, 'Request')
def test_get_with_retries_all_failed(self, request, urlopen):
# Simulate a failing request, ensure retries done
request.return_value.get_type.return_value = "http"
@ -1470,13 +1480,13 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(request.call_count, 2)
self.assertEqual(urlopen.call_count, 2)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch('eventlet.green.urllib2.Request')
@mock.patch.object(urllib2, 'urlopen')
@mock.patch.object(urllib2, 'Request')
def test_get_with_retries(self, request, urlopen):
# First request fails, retry successful
request.return_value.get_type.return_value = "http"
mock_resp = mock.MagicMock()
mock_resp.read.return_value = ''
mock_resp.read.return_value = b''
mock_resp.info.return_value = {}
urlopen.side_effect = [urllib2.URLError(''), mock_resp]
sc = internal_client.SimpleClient(url='http://127.0.0.1', retries=1,
@ -1492,10 +1502,10 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual([{}, None], retval)
self.assertEqual(sc.attempts, 2)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch.object(urllib2, 'urlopen')
def test_get_with_retries_param(self, mock_urlopen):
mock_response = mock.MagicMock()
mock_response.read.return_value = ''
mock_response.read.return_value = b''
mock_response.info.return_value = {}
mock_urlopen.side_effect = internal_client.httplib.BadStatusLine('')
c = internal_client.SimpleClient(url='http://127.0.0.1', token='token')
@ -1524,10 +1534,10 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(mock_urlopen.call_count, 2)
self.assertEqual([{}, None], retval)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch.object(urllib2, 'urlopen')
def test_request_with_retries_with_HTTPError(self, mock_urlopen):
mock_response = mock.MagicMock()
mock_response.read.return_value = ''
mock_response.read.return_value = b''
c = internal_client.SimpleClient(url='http://127.0.0.1', token='token')
self.assertEqual(c.retries, 5)
@ -1541,11 +1551,11 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(mock_sleep.call_count, 1)
self.assertEqual(mock_urlopen.call_count, 2)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch.object(urllib2, 'urlopen')
def test_request_container_with_retries_with_HTTPError(self,
mock_urlopen):
mock_response = mock.MagicMock()
mock_response.read.return_value = ''
mock_response.read.return_value = b''
c = internal_client.SimpleClient(url='http://127.0.0.1', token='token')
self.assertEqual(c.retries, 5)
@ -1560,11 +1570,11 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(mock_sleep.call_count, 1)
self.assertEqual(mock_urlopen.call_count, 2)
@mock.patch('eventlet.green.urllib2.urlopen')
@mock.patch.object(urllib2, 'urlopen')
def test_request_object_with_retries_with_HTTPError(self,
mock_urlopen):
mock_response = mock.MagicMock()
mock_response.read.return_value = ''
mock_response.read.return_value = b''
c = internal_client.SimpleClient(url='http://127.0.0.1', token='token')
self.assertEqual(c.retries, 5)
@ -1602,7 +1612,12 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(0.1, kwargs['timeout'])
self.assertTrue(isinstance(args[0], urllib2.Request))
self.assertEqual(proxy_host, args[0].host)
self.assertEqual(scheme, args[0].type)
if six.PY2:
self.assertEqual(scheme, args[0].type)
else:
# TODO: figure out why this happens, whether py2 or py3 is
# messed up, whether we care, and what can be done about it
self.assertEqual('https', args[0].type)
# class methods
content = mock.MagicMock()
@ -1622,7 +1637,11 @@ class TestSimpleClient(unittest.TestCase):
self.assertEqual(0.1, kwargs['timeout'])
self.assertTrue(isinstance(args[0], urllib2.Request))
self.assertEqual(proxy_host, args[0].host)
self.assertEqual(scheme, args[0].type)
if six.PY2:
self.assertEqual(scheme, args[0].type)
else:
# See above
self.assertEqual('https', args[0].type)
if __name__ == '__main__':
unittest.main()

1
tox.ini

@ -52,6 +52,7 @@ commands =
test/unit/common/test_direct_client.py \
test/unit/common/test_exceptions.py \
test/unit/common/test_header_key_dict.py \
test/unit/common/test_internal_client.py \
test/unit/common/test_linkat.py \
test/unit/common/test_manager.py \
test/unit/common/test_memcached.py \