openstack
/
glance
Code Issues Proposed changes

Port image cache to Python 3 

* Use bytes for image content
* On Python 3, set_xattr() encodes the text value to UTF-8
* Open files in binary mode, not in text mode
* get_caching_iter(): pass a list as the image iterator, not a string.
  On Python 3, list(b'abc') returns [97, 98, 99], whereas Python 2 returns
  ['a', 'b', 'c'].
* tox.ini: add glance.tests.unit.test_image_cache to Python 3.4

Change-Id: I638525d19c42990852cf45dd416318d9a847c303
This commit is contained in:
Victor Stinner 2015-07-28 13:08:42 +02:00
parent e14f2da796
commit eeedd2030d
4 changed files with 36 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
glance/image_cache/drivers/xattr.py
View File

@ -62,6 +62,7 @@ from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import encodeutils
from oslo_utils import excutils
import six
import xattr
from glance.common import exception
@ -101,7 +102,7 @@ class Driver(base.Driver):
image_cache_dir = self.base_dir
fake_image_filepath = os.path.join(image_cache_dir, 'checkme')
with open(fake_image_filepath, 'wb') as fake_file:
fake_file.write("XXX")
fake_file.write(b"XXX")
fake_file.flush()
try:
set_xattr(fake_image_filepath, 'hits', '1')
@ -491,7 +492,11 @@ def set_xattr(path, key, value):
If xattrs aren't supported by the file-system, we skip setting the value.
"""
namespaced_key = _make_namespaced_xattr_key(key)
xattr.setxattr(path, namespaced_key, str(value))
if not isinstance(value, six.binary_type):
value = str(value)
if six.PY3:
value = value.encode('utf-8')
xattr.setxattr(path, namespaced_key, value)
def inc_xattr(path, key, n=1):

50
glance/tests/unit/test_image_cache.py
View File

@ -35,13 +35,13 @@ from glance.tests.utils import skip_if_disabled
from glance.tests.utils import xattr_writes_supported
FIXTURE_LENGTH = 1024
FIXTURE_DATA = '*' * FIXTURE_LENGTH
FIXTURE_DATA = b'*' * FIXTURE_LENGTH
class ImageCacheTestCase(object):
def _setup_fixture_file(self):
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertFalse(self.cache.is_cached(1))
@ -64,7 +64,7 @@ class ImageCacheTestCase(object):
"""
self._setup_fixture_file()
buff = six.StringIO()
buff = six.BytesIO()
with self.cache.open_for_read(1) as cache_file:
for chunk in cache_file:
buff.write(chunk)
@ -78,7 +78,7 @@ class ImageCacheTestCase(object):
"""
self._setup_fixture_file()
buff = six.StringIO()
buff = six.BytesIO()
with self.cache.open_for_read(1) as cache_file:
for chunk in cache_file:
buff.write(chunk)
@ -112,7 +112,7 @@ class ImageCacheTestCase(object):
self.assertFalse(self.cache.is_cached(image_id))
for image_id in (1, 2):
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertTrue(self.cache.cache_image_file(image_id,
FIXTURE_FILE))
@ -128,7 +128,7 @@ class ImageCacheTestCase(object):
def test_clean_stalled(self):
"""Test the clean method removes expected images."""
incomplete_file_path = os.path.join(self.cache_dir, 'incomplete', '1')
incomplete_file = open(incomplete_file_path, 'w')
incomplete_file = open(incomplete_file_path, 'wb')
incomplete_file.write(FIXTURE_DATA)
incomplete_file.close()
@ -148,7 +148,7 @@ class ImageCacheTestCase(object):
incomplete_file_path_2 = os.path.join(self.cache_dir,
'incomplete', '2')
for f in (incomplete_file_path_1, incomplete_file_path_2):
incomplete_file = open(f, 'w')
incomplete_file = open(f, 'wb')
incomplete_file.write(FIXTURE_DATA)
incomplete_file.close()
@ -181,21 +181,21 @@ class ImageCacheTestCase(object):
# prune. We should see only 5 images left after pruning, and the
# images that are least recently accessed should be the ones pruned...
for x in range(10):
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertTrue(self.cache.cache_image_file(x, FIXTURE_FILE))
self.assertEqual(10 * units.Ki, self.cache.get_cache_size())
# OK, hit the images that are now cached...
for x in range(10):
buff = six.StringIO()
buff = six.BytesIO()
with self.cache.open_for_read(x) as cache_file:
for chunk in cache_file:
buff.write(chunk)
# Add a new image to cache.
# This is specifically to test the bug: 1438564
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertTrue(self.cache.cache_image_file(99, FIXTURE_FILE))
self.cache.prune()
@ -223,13 +223,13 @@ class ImageCacheTestCase(object):
"""
self.assertEqual(0, self.cache.get_cache_size())
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertTrue(self.cache.cache_image_file('xxx', FIXTURE_FILE))
self.assertEqual(1024, self.cache.get_cache_size())
# OK, hit the image that is now cached...
buff = six.StringIO()
buff = six.BytesIO()
with self.cache.open_for_read('xxx') as cache_file:
for chunk in cache_file:
buff.write(chunk)
@ -249,7 +249,7 @@ class ImageCacheTestCase(object):
self.assertFalse(self.cache.is_cached(1))
self.assertFalse(self.cache.is_queued(1))
FIXTURE_FILE = six.StringIO(FIXTURE_DATA)
FIXTURE_FILE = six.BytesIO(FIXTURE_DATA)
self.assertTrue(self.cache.queue_image(1))
@ -289,7 +289,7 @@ class ImageCacheTestCase(object):
image_id = '1'
self.assertFalse(self.cache.is_cached(image_id))
with self.cache.driver.open_for_write(image_id) as cache_file:
cache_file.write('a')
cache_file.write(b'a')
self.assertTrue(self.cache.is_cached(image_id),
"Image %s was NOT cached!" % image_id)
# make sure it has tidied up
@ -332,7 +332,7 @@ class ImageCacheTestCase(object):
# test a case where an exception NOT raised while the file is open,
# and a consuming iterator completes
def consume(image_id):
data = ['a', 'b', 'c', 'd', 'e', 'f']
data = [b'a', b'b', b'c', b'd', b'e', b'f']
checksum = None
caching_iter = self.cache.get_caching_iter(image_id, checksum,
iter(data))
@ -356,9 +356,9 @@ class ImageCacheTestCase(object):
to consume data, and rolls back the cache.
"""
def faulty_backend():
data = ['a', 'b', 'c', 'Fail', 'd', 'e', 'f']
data = [b'a', b'b', b'c', b'Fail', b'd', b'e', b'f']
for d in data:
if d == 'Fail':
if d == b'Fail':
raise exception.GlanceException('Backend failure')
yield d
@ -383,11 +383,11 @@ class ImageCacheTestCase(object):
"""
# test a case where a consuming iterator just stops.
def falloffend(image_id):
data = ['a', 'b', 'c', 'd', 'e', 'f']
data = [b'a', b'b', b'c', b'd', b'e', b'f']
checksum = None
caching_iter = self.cache.get_caching_iter(image_id, checksum,
iter(data))
self.assertEqual('a', caching_iter.next())
self.assertEqual(b'a', next(caching_iter))
image_id = '1'
self.assertFalse(self.cache.is_cached(image_id))
@ -402,7 +402,7 @@ class ImageCacheTestCase(object):
self.assertTrue(os.path.exists(invalid_file_path))
def test_gate_caching_iter_good_checksum(self):
image = "12345678990abcdefghijklmnop"
image = b"12345678990abcdefghijklmnop"
image_id = 123
md5 = hashlib.md5()
@ -410,19 +410,19 @@ class ImageCacheTestCase(object):
checksum = md5.hexdigest()
cache = image_cache.ImageCache()
img_iter = cache.get_caching_iter(image_id, checksum, image)
img_iter = cache.get_caching_iter(image_id, checksum, [image])
for chunk in img_iter:
pass
# checksum is valid, fake image should be cached:
self.assertTrue(cache.is_cached(image_id))
def test_gate_caching_iter_bad_checksum(self):
image = "12345678990abcdefghijklmnop"
image = b"12345678990abcdefghijklmnop"
image_id = 123
checksum = "foobar" # bad.
cache = image_cache.ImageCache()
img_iter = cache.get_caching_iter(image_id, checksum, image)
img_iter = cache.get_caching_iter(image_id, checksum, [image])
def reader():
for chunk in img_iter:
@ -539,7 +539,7 @@ class TestImageCacheNoDep(test_utils.BaseTestCase):
self.driver = FailingFileDriver()
cache = image_cache.ImageCache()
data = ['a', 'b', 'c', 'Fail', 'd', 'e', 'f']
data = [b'a', b'b', b'c', b'Fail', b'd', b'e', b'f']
caching_iter = cache.get_caching_iter('dummy_id', None, iter(data))
self.assertEqual(data, list(caching_iter))
@ -557,7 +557,7 @@ class TestImageCacheNoDep(test_utils.BaseTestCase):
self.driver = OpenFailingDriver()
cache = image_cache.ImageCache()
data = ['a', 'b', 'c', 'd', 'e', 'f']
data = [b'a', b'b', b'c', b'd', b'e', b'f']
caching_iter = cache.get_caching_iter('dummy_id', None, iter(data))
self.assertEqual(data, list(caching_iter))

6
glance/tests/utils.py
View File

@ -392,17 +392,17 @@ def xattr_writes_supported(path):
return False
def set_xattr(path, key, value):
xattr.setxattr(path, "user.%s" % key, str(value))
xattr.setxattr(path, "user.%s" % key, value)
# We do a quick attempt to write a user xattr to a temporary file
# to check that the filesystem is even enabled to support xattrs
fake_filepath = os.path.join(path, 'testing-checkme')
result = True
with open(fake_filepath, 'wb') as fake_file:
fake_file.write("XXX")
fake_file.write(b"XXX")
fake_file.flush()
try:
set_xattr(fake_filepath, 'hits', '1')
set_xattr(fake_filepath, 'hits', b'1')
except IOError as e:
if e.errno == errno.EOPNOTSUPP:
result = False

1
tox.ini
View File

@ -36,6 +36,7 @@ commands =
glance.tests.unit.test_db_metadef \
glance.tests.unit.test_domain \
glance.tests.unit.test_domain_proxy \
glance.tests.unit.test_image_cache \
glance.tests.unit.test_image_cache_client \
glance.tests.unit.test_jsonpatchmixin \
glance.tests.unit.test_manage \