openstack/swift
Code Issues Proposed changes

Fix Python 3 issues in diskfile

Browse Source 
* Fix bytes vs Unicode issues
* On Python 3, encode JSON to UTF-8 and decode it from UTF-8
* Open files in binary mode to avoid Unicode issues
* test_auditor: use bytes for content, open files in binary mode

Change-Id: Ifa84001493cfb57975d3b140b0d7e09020504bca
This commit is contained in:
Victor Stinner 2016-07-26 12:36:50 +02:00
parent 25cdd52c6a
commit a81d60472f
2 changed files with 28 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
swift/obj/diskfile.py
View File

@ -49,6 +49,7 @@ from collections import defaultdict
from eventlet import Timeout from eventlet import Timeout
from eventlet.hubs import trampoline from eventlet.hubs import trampoline
import six
from swift import gettext_ as _ from swift import gettext_ as _
from swift.common.constraints import check_mount, check_dir from swift.common.constraints import check_mount, check_dir
@ -112,7 +113,7 @@ def read_metadata(fd):
:returns: dictionary of metadata :returns: dictionary of metadata
""" """
metadata = '' metadata = b''
key = 0 key = 0
try: try:
while True: while True:
@ -281,7 +282,7 @@ def consolidate_hashes(partition_dir):
# Now that all the invalidations are reflected in hashes.pkl, it's # Now that all the invalidations are reflected in hashes.pkl, it's
# safe to clear out the invalidations file. # safe to clear out the invalidations file.
try: try:
with open(invalidations_file, 'w') as inv_fh: with open(invalidations_file, 'wb') as inv_fh:
pass pass
except OSError as e: except OSError as e:
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
@ -416,7 +417,11 @@ def get_auditor_status(datadir_path, logger, auditor_type):
datadir_path, "auditor_status_%s.json" % auditor_type) datadir_path, "auditor_status_%s.json" % auditor_type)
status = {} status = {}
try: try:
with open(auditor_status) as statusfile: if six.PY3:
statusfile = open(auditor_status, encoding='utf8')
else:
statusfile = open(auditor_status, 'rb')
with statusfile:
status = statusfile.read() status = statusfile.read()
except (OSError, IOError) as e: except (OSError, IOError) as e:
if e.errno != errno.ENOENT and logger: if e.errno != errno.ENOENT and logger:
@ -435,6 +440,8 @@ def get_auditor_status(datadir_path, logger, auditor_type):
def update_auditor_status(datadir_path, logger, partitions, auditor_type): def update_auditor_status(datadir_path, logger, partitions, auditor_type):
status = json.dumps({'partitions': partitions}) status = json.dumps({'partitions': partitions})
if six.PY3:
status = status.encode('utf8')
auditor_status = os.path.join( auditor_status = os.path.join(
datadir_path, "auditor_status_%s.json" % auditor_type) datadir_path, "auditor_status_%s.json" % auditor_type)
try: try:
@ -2516,7 +2523,7 @@ class ECDiskFileWriter(BaseDiskFileWriter):
exc = None exc = None
try: try:
try: try:
with open(durable_file_path, 'w') as _fp: with open(durable_file_path, 'wb') as _fp:
fsync(_fp.fileno()) fsync(_fp.fileno())
fsync_dir(self._datadir) fsync_dir(self._datadir)
except (OSError, IOError) as err: except (OSError, IOError) as err:

36
test/unit/obj/test_auditor.py
View File

@ -153,7 +153,7 @@ class TestAuditor(unittest.TestCase):
def run_tests(disk_file): def run_tests(disk_file):
auditor_worker = auditor.AuditorWorker(self.conf, self.logger, auditor_worker = auditor.AuditorWorker(self.conf, self.logger,
self.rcache, self.devices) self.rcache, self.devices)
data = '0' * 1024 data = b'0' * 1024
etag = md5() etag = md5()
with disk_file.create() as writer: with disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -174,7 +174,7 @@ class TestAuditor(unittest.TestCase):
policy=disk_file.policy)) policy=disk_file.policy))
self.assertEqual(auditor_worker.quarantines, pre_quarantines) self.assertEqual(auditor_worker.quarantines, pre_quarantines)
os.write(writer._fd, 'extra_data') os.write(writer._fd, b'extra_data')
auditor_worker.object_audit( auditor_worker.object_audit(
AuditLocation(disk_file._datadir, 'sda', '0', AuditLocation(disk_file._datadir, 'sda', '0',
@ -188,7 +188,7 @@ class TestAuditor(unittest.TestCase):
def test_object_audit_diff_data(self): def test_object_audit_diff_data(self):
auditor_worker = auditor.AuditorWorker(self.conf, self.logger, auditor_worker = auditor.AuditorWorker(self.conf, self.logger,
self.rcache, self.devices) self.rcache, self.devices)
data = '0' * 1024 data = b'0' * 1024
etag = md5() etag = md5()
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
@ -212,9 +212,7 @@ class TestAuditor(unittest.TestCase):
AuditLocation(self.disk_file._datadir, 'sda', '0', AuditLocation(self.disk_file._datadir, 'sda', '0',
policy=POLICIES.legacy)) policy=POLICIES.legacy))
self.assertEqual(auditor_worker.quarantines, pre_quarantines) self.assertEqual(auditor_worker.quarantines, pre_quarantines)
etag = md5() etag = md5(b'1' + b'0' * 1023).hexdigest()
etag.update('1' + '0' * 1023)
etag = etag.hexdigest()
metadata['ETag'] = etag metadata['ETag'] = etag
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
@ -231,8 +229,8 @@ class TestAuditor(unittest.TestCase):
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
path = os.path.join(self.disk_file._datadir, timestamp + '.data') path = os.path.join(self.disk_file._datadir, timestamp + '.data')
mkdirs(self.disk_file._datadir) mkdirs(self.disk_file._datadir)
fp = open(path, 'w') fp = open(path, 'wb')
fp.write('0' * 1024) fp.write(b'0' * 1024)
fp.close() fp.close()
invalidate_hash(os.path.dirname(self.disk_file._datadir)) invalidate_hash(os.path.dirname(self.disk_file._datadir))
auditor_worker = auditor.AuditorWorker(self.conf, self.logger, auditor_worker = auditor.AuditorWorker(self.conf, self.logger,
@ -362,7 +360,7 @@ class TestAuditor(unittest.TestCase):
location = AuditLocation(self.disk_file._datadir, 'sda', '0', location = AuditLocation(self.disk_file._datadir, 'sda', '0',
policy=self.disk_file.policy) policy=self.disk_file.policy)
data = 'VERIFY' data = b'VERIFY'
etag = md5() etag = md5()
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
@ -440,7 +438,7 @@ class TestAuditor(unittest.TestCase):
auditor_worker.last_logged = time.time() auditor_worker.last_logged = time.time()
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
pre_errors = auditor_worker.errors pre_errors = auditor_worker.errors
data = '0' * 1024 data = b'0' * 1024
etag = md5() etag = md5()
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -464,7 +462,7 @@ class TestAuditor(unittest.TestCase):
auditor_worker.log_time = 0 auditor_worker.log_time = 0
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
pre_quarantines = auditor_worker.quarantines pre_quarantines = auditor_worker.quarantines
data = '0' * 1024 data = b'0' * 1024
def write_file(df): def write_file(df):
with df.create() as writer: with df.create() as writer:
@ -491,7 +489,7 @@ class TestAuditor(unittest.TestCase):
self.assertEqual(auditor_worker.stats_buckets[10240], 0) self.assertEqual(auditor_worker.stats_buckets[10240], 0)
# pick up some additional code coverage, large file # pick up some additional code coverage, large file
data = '0' * 1024 * 1024 data = b'0' * 1024 * 1024
for df in (self.disk_file, self.disk_file_ec): for df in (self.disk_file, self.disk_file_ec):
with df.create() as writer: with df.create() as writer:
writer.write(data) writer.write(data)
@ -545,7 +543,7 @@ class TestAuditor(unittest.TestCase):
pre_quarantines = auditor_worker.quarantines pre_quarantines = auditor_worker.quarantines
# pretend that we logged (and reset counters) just now # pretend that we logged (and reset counters) just now
auditor_worker.last_logged = time.time() auditor_worker.last_logged = time.time()
data = '0' * 1024 data = b'0' * 1024
etag = md5() etag = md5()
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -557,7 +555,7 @@ class TestAuditor(unittest.TestCase):
'Content-Length': str(os.fstat(writer._fd).st_size), 'Content-Length': str(os.fstat(writer._fd).st_size),
} }
writer.put(metadata) writer.put(metadata)
os.write(writer._fd, 'extra_data') os.write(writer._fd, b'extra_data')
writer.commit(Timestamp(timestamp)) writer.commit(Timestamp(timestamp))
auditor_worker.audit_all_objects() auditor_worker.audit_all_objects()
self.assertEqual(auditor_worker.quarantines, pre_quarantines + 1) self.assertEqual(auditor_worker.quarantines, pre_quarantines + 1)
@ -569,7 +567,7 @@ class TestAuditor(unittest.TestCase):
auditor_worker.last_logged = time.time() auditor_worker.last_logged = time.time()
timestamp = str(normalize_timestamp(time.time())) timestamp = str(normalize_timestamp(time.time()))
pre_quarantines = auditor_worker.quarantines pre_quarantines = auditor_worker.quarantines
data = '0' * 10 data = b'0' * 10
etag = md5() etag = md5()
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -585,7 +583,7 @@ class TestAuditor(unittest.TestCase):
auditor_worker.audit_all_objects() auditor_worker.audit_all_objects()
self.disk_file = self.df_mgr.get_diskfile('sda', '0', 'a', 'c', 'ob', self.disk_file = self.df_mgr.get_diskfile('sda', '0', 'a', 'c', 'ob',
policy=POLICIES.legacy) policy=POLICIES.legacy)
data = '1' * 10 data = b'1' * 10
etag = md5() etag = md5()
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -598,14 +596,14 @@ class TestAuditor(unittest.TestCase):
} }
writer.put(metadata) writer.put(metadata)
writer.commit(Timestamp(timestamp)) writer.commit(Timestamp(timestamp))
os.write(writer._fd, 'extra_data') os.write(writer._fd, b'extra_data')
auditor_worker.audit_all_objects() auditor_worker.audit_all_objects()
self.assertEqual(auditor_worker.quarantines, pre_quarantines + 1) self.assertEqual(auditor_worker.quarantines, pre_quarantines + 1)
def test_object_run_fast_track_non_zero(self): def test_object_run_fast_track_non_zero(self):
self.auditor = auditor.ObjectAuditor(self.conf) self.auditor = auditor.ObjectAuditor(self.conf)
self.auditor.log_time = 0 self.auditor.log_time = 0
data = '0' * 1024 data = b'0' * 1024
etag = md5() etag = md5()
with self.disk_file.create() as writer: with self.disk_file.create() as writer:
writer.write(data) writer.write(data)
@ -620,7 +618,7 @@ class TestAuditor(unittest.TestCase):
writer.put(metadata) writer.put(metadata)
writer.commit(Timestamp(timestamp)) writer.commit(Timestamp(timestamp))
etag = md5() etag = md5()
etag.update('1' + '0' * 1023) etag.update(b'1' + b'0' * 1023)
etag = etag.hexdigest() etag = etag.hexdigest()
metadata['ETag'] = etag metadata['ETag'] = etag
write_metadata(writer._fd, metadata) write_metadata(writer._fd, metadata)