Skip to content

Commit

Permalink
py3: Port more CLI tools
Browse files Browse the repository at this point in the history
Bring under test

 - test/unit/cli/test_dispersion_report.py
 - test/unit/cli/test_info.py and
 - test/unit/cli/test_relinker.py

I've verified that swift-*-info (at least) behave reasonably under
py3, even swift-object-info when there's non-utf8 metadata on the
data/meta file.

Change-Id: Ifed4b8059337c395e56f5e9f8d939c34fe4ff8dd
  • Loading branch information
tipabu committed Feb 28, 2018
1 parent 624b531 commit 36c4297
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 33 deletions.
8 changes: 8 additions & 0 deletions bin/swift-object-info
Expand Up @@ -14,15 +14,23 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import codecs
import sys
from optparse import OptionParser

import six

from swift.common.storage_policy import reload_storage_policies
from swift.common.utils import set_swift_dir
from swift.cli.info import print_obj, InfoSystemExit


if __name__ == '__main__':
if not six.PY2:
# Make stdout able to write escaped bytes
sys.stdout = codecs.getwriter("utf-8")(
sys.stdout.detach(), errors='surrogateescape')

parser = OptionParser('%prog [options] OBJECT_FILE')
parser.add_option(
'-n', '--no-check-etag', default=True,
Expand Down
4 changes: 2 additions & 2 deletions swift/cli/info.py
Expand Up @@ -352,8 +352,8 @@ def print_obj_metadata(metadata):
def print_metadata(title, items):
print(title)
if items:
for meta_key in sorted(items):
print(' %s: %s' % (meta_key, items[meta_key]))
for key, value in sorted(items.items()):
print(' %s: %s' % (key, value))
else:
print(' No metadata found')

Expand Down
23 changes: 15 additions & 8 deletions swift/common/db.py
Expand Up @@ -56,12 +56,19 @@ def utf8encode(*args):
for s in args]


def utf8encodekeys(metadata):
uni_keys = [k for k in metadata if isinstance(k, six.text_type)]
for k in uni_keys:
sv = metadata[k]
del metadata[k]
metadata[k.encode('utf-8')] = sv
def native_str_keys(metadata):
if six.PY2:
uni_keys = [k for k in metadata if isinstance(k, six.text_type)]
for k in uni_keys:
sv = metadata[k]
del metadata[k]
metadata[k.encode('utf-8')] = sv
else:
bin_keys = [k for k in metadata if isinstance(k, six.binary_type)]
for k in bin_keys:
sv = metadata[k]
del metadata[k]
metadata[k.decode('utf-8')] = sv


def _db_timeout(timeout, db_file, call):
Expand Down Expand Up @@ -741,7 +748,7 @@ def metadata(self):
metadata = self.get_raw_metadata()
if metadata:
metadata = json.loads(metadata)
utf8encodekeys(metadata)
native_str_keys(metadata)
else:
metadata = {}
return metadata
Expand Down Expand Up @@ -803,7 +810,7 @@ def update_metadata(self, metadata_updates, validate_metadata=False):
self.db_type)
md = row[0]
md = json.loads(md) if md else {}
utf8encodekeys(md)
native_str_keys(md)
except sqlite3.OperationalError as err:
if 'no such column: metadata' not in str(err):
raise
Expand Down
4 changes: 2 additions & 2 deletions swift/common/swob.py
Expand Up @@ -290,8 +290,8 @@ def setter(self, value):
else:
if isinstance(value, six.text_type):
value = value.encode('utf-8')
self.status_int = int(value.split(' ', 1)[0])
self.explanation = self.title = value.split(' ', 1)[1]
self.status_int = int(value.split(b' ', 1)[0])
self.explanation = self.title = value.split(b' ', 1)[1]

return property(getter, setter,
doc="Retrieve and set the Response status, e.g. '200 OK'")
Expand Down
47 changes: 36 additions & 11 deletions swift/obj/diskfile.py
Expand Up @@ -83,8 +83,8 @@
DEFAULT_RECLAIM_AGE = timedelta(weeks=1).total_seconds()
HASH_FILE = 'hashes.pkl'
HASH_INVALIDATIONS_FILE = 'hashes.invalid'
METADATA_KEY = 'user.swift.metadata'
METADATA_CHECKSUM_KEY = 'user.swift.metadata_checksum'
METADATA_KEY = b'user.swift.metadata'
METADATA_CHECKSUM_KEY = b'user.swift.metadata_checksum'
DROP_CACHE_WINDOW = 1024 * 1024
# These are system-set metadata keys that cannot be changed with a POST.
# They should be lowercase.
Expand Down Expand Up @@ -131,6 +131,26 @@ def encode_str(item):
return dict(((encode_str(k), encode_str(v)) for k, v in metadata.items()))


def _decode_metadata(metadata):
"""
Given a metadata dict from disk, convert keys and values to native strings.
:param metadata: a dict
"""
if six.PY2:
def to_str(item):
if isinstance(item, six.text_type):
return item.encode('utf8')
return item
else:
def to_str(item):
if isinstance(item, six.binary_type):
return item.decode('utf8', 'surrogateescape')
return item

return dict(((to_str(k), to_str(v)) for k, v in metadata.items()))


def read_metadata(fd, add_missing_checksum=False):
"""
Helper function to read the pickled metadata from an object file.
Expand All @@ -144,8 +164,8 @@ def read_metadata(fd, add_missing_checksum=False):
key = 0
try:
while True:
metadata += xattr.getxattr(fd, '%s%s' % (METADATA_KEY,
(key or '')))
metadata += xattr.getxattr(
fd, METADATA_KEY + str(key or '').encode('ascii'))
key += 1
except (IOError, OSError) as e:
if errno.errorcode.get(e.errno) in ('ENOTSUP', 'EOPNOTSUPP'):
Expand Down Expand Up @@ -173,7 +193,7 @@ def read_metadata(fd, add_missing_checksum=False):
logging.error("Error adding metadata: %s" % e)

if metadata_checksum:
computed_checksum = hashlib.md5(metadata).hexdigest()
computed_checksum = hashlib.md5(metadata).hexdigest().encode('ascii')
if metadata_checksum != computed_checksum:
raise DiskFileBadMetadataChecksum(
"Metadata checksum mismatch for %s: "
Expand All @@ -183,7 +203,11 @@ def read_metadata(fd, add_missing_checksum=False):
# strings are utf-8 encoded when written, but have not always been
# (see https://bugs.launchpad.net/swift/+bug/1678018) so encode them again
# when read
return _encode_metadata(pickle.loads(metadata))
if six.PY2:
metadata = pickle.loads(metadata)
else:
metadata = pickle.loads(metadata, encoding='bytes')
return _decode_metadata(metadata)


def write_metadata(fd, metadata, xattr_size=65536):
Expand All @@ -194,11 +218,11 @@ def write_metadata(fd, metadata, xattr_size=65536):
:param metadata: metadata to write
"""
metastr = pickle.dumps(_encode_metadata(metadata), PICKLE_PROTOCOL)
metastr_md5 = hashlib.md5(metastr).hexdigest()
metastr_md5 = hashlib.md5(metastr).hexdigest().encode('ascii')
key = 0
try:
while metastr:
xattr.setxattr(fd, '%s%s' % (METADATA_KEY, key or ''),
xattr.setxattr(fd, METADATA_KEY + str(key or '').encode('ascii'),
metastr[:xattr_size])
metastr = metastr[xattr_size:]
key += 1
Expand Down Expand Up @@ -368,9 +392,10 @@ def invalidate_hash(suffix_dir):
suffix = basename(suffix_dir)
partition_dir = dirname(suffix_dir)
invalidations_file = join(partition_dir, HASH_INVALIDATIONS_FILE)
with lock_path(partition_dir):
with open(invalidations_file, 'ab') as inv_fh:
inv_fh.write(suffix + "\n")
if not isinstance(suffix, bytes):
suffix = suffix.encode('utf-8')
with lock_path(partition_dir), open(invalidations_file, 'ab') as inv_fh:
inv_fh.write(suffix + b"\n")


def relink_paths(target_path, new_target_path, check_existing=False):
Expand Down
2 changes: 1 addition & 1 deletion test/unit/__init__.py
Expand Up @@ -1292,7 +1292,7 @@ def xattr_supported_check():
# assume the worst -- xattrs aren't supported
supports_xattr_cached_val = False

big_val = 'x' * (4096 + 1) # more than 4k of metadata
big_val = b'x' * (4096 + 1) # more than 4k of metadata
try:
fd, tmppath = mkstemp()
xattr.setxattr(fd, 'user.swift.testing_key', big_val)
Expand Down
6 changes: 3 additions & 3 deletions test/unit/cli/test_info.py
Expand Up @@ -42,8 +42,8 @@ class TestCliInfoBase(unittest.TestCase):
def setUp(self):
skip_if_no_xattrs()
self.orig_hp = utils.HASH_PATH_PREFIX, utils.HASH_PATH_SUFFIX
utils.HASH_PATH_PREFIX = 'info'
utils.HASH_PATH_SUFFIX = 'info'
utils.HASH_PATH_PREFIX = b'info'
utils.HASH_PATH_SUFFIX = b'info'
self.testdir = os.path.join(mkdtemp(), 'tmp_test_cli_info')
utils.mkdirs(self.testdir)
rmtree(self.testdir)
Expand Down Expand Up @@ -875,7 +875,7 @@ def test_print_obj_invalid(self):
self.assertRaises(InfoSystemExit, print_obj, datafile)

with open(datafile, 'wb') as fp:
fp.write('1234')
fp.write(b'1234')

out = StringIO()
with mock.patch('sys.stdout', out):
Expand Down
2 changes: 1 addition & 1 deletion test/unit/cli/test_recon.py
Expand Up @@ -511,7 +511,7 @@ def mock_scout_umount(app, host):
self.recon_instance.umount_check(hosts)

output = stdout.getvalue()
r = re.compile("\Not mounted:|Device errors: .*")
r = re.compile("^Not mounted:|Device errors: .*")
lines = output.splitlines()
self.assertTrue(lines)
for line in lines:
Expand Down
4 changes: 2 additions & 2 deletions test/unit/cli/test_relinker.py
Expand Up @@ -62,7 +62,7 @@ def setUp(self):
self.object_fname = "1278553064.00000.data"
self.objname = os.path.join(self.objdir, self.object_fname)
with open(self.objname, "wb") as dummy:
dummy.write("Hello World!")
dummy.write(b"Hello World!")
write_metadata(dummy, {'name': '/a/c/o', 'Content-Length': '12'})

test_policies = [StoragePolicy(0, 'platin', True)]
Expand Down Expand Up @@ -164,7 +164,7 @@ def test_cleanup_quarantined(self):
self._common_test_cleanup()
# Pretend the object in the new place got corrupted
with open(self.expected_file, "wb") as obj:
obj.write('trash')
obj.write(b'trash')

self.assertEqual(
1, relinker.cleanup(self.testdir, self.devices, True, self.logger))
Expand Down
6 changes: 3 additions & 3 deletions test/unit/obj/test_diskfile.py
Expand Up @@ -326,16 +326,16 @@ def check_metadata():
check_metadata()

# simulate a legacy diskfile that might have persisted unicode metadata
with mock.patch.object(diskfile, '_encode_metadata', lambda x: x):
with mock.patch.object(diskfile, '_decode_metadata', lambda x: x):
with open(path, 'wb') as fd:
diskfile.write_metadata(fd, metadata)
# sanity check, while still mocked, that we did persist unicode
with open(path, 'rb') as fd:
actual = diskfile.read_metadata(fd)
for k, v in actual.items():
if k == u'X-Object-Meta-Strange':
self.assertIsInstance(k, six.text_type)
self.assertIsInstance(v, six.text_type)
self.assertIsInstance(k, str)
self.assertIsInstance(v, str)
break
else:
self.fail('Did not find X-Object-Meta-Strange')
Expand Down
3 changes: 3 additions & 0 deletions tox.ini
Expand Up @@ -29,6 +29,9 @@ setenv = VIRTUAL_ENV={envdir}
[testenv:py34]
commands =
nosetests \
test/unit/cli/test_dispersion_report.py \
test/unit/cli/test_info.py \
test/unit/cli/test_relinker.py \
test/unit/cli/test_ring_builder_analyzer.py \
test/unit/cli/test_ringbuilder.py \
test/unit/common/ring \
Expand Down

0 comments on commit 36c4297

Please sign in to comment.