From 6be5196fbef179691898187f6ccadfa5e17d2805 Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Tue, 25 Apr 2017 19:29:57 -0700 Subject: [PATCH] Make add_dev complain louder about missing keys ... and remove some cruft that couldn't possibly work Change-Id: I560f0a29f0a881c63ec3cb910dbf5476fe2a915a Related-Change-Id: I0d8928b55020592f8e75321d1f7678688301d797 --- swift/common/ring/builder.py | 11 +++++++---- test/unit/common/ring/test_builder.py | 7 +++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 88932e490d..c3d4d16b6e 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -367,6 +367,11 @@ class RingBuilder(object): # Add holes to self.devs to ensure self.devs[dev['id']] will be the dev while dev['id'] >= len(self.devs): self.devs.append(None) + required_keys = ('ip', 'port', 'weight') + if any(required not in dev for required in required_keys): + raise ValueError( + '%r is missing at least one the required key %r' % ( + dev, required_keys)) dev['weight'] = float(dev['weight']) dev['parts'] = 0 self.devs[dev['id']] = dev @@ -1657,10 +1662,8 @@ class RingBuilder(object): # NOTE(akscram): An old ring builder file don't contain # replication parameters. if dev: - if 'ip' in dev: - dev.setdefault('replication_ip', dev['ip']) - if 'port' in dev: - dev.setdefault('replication_port', dev['port']) + dev.setdefault('replication_ip', dev['ip']) + dev.setdefault('replication_port', dev['port']) return builder def save(self, builder_file): diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 8fdc4c9629..c6d6c22fd1 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -229,6 +229,13 @@ class TestRingBuilder(unittest.TestCase): 'ip': '127.0.0.1', 'port': 6200}) self.assertEqual(rb.devs[1]['id'], 1) self.assertEqual(dev_id, 1) + # some keys are required + self.assertRaises(ValueError, rb.add_dev, {}) + stub_dev = {'weight': 1, 'ip': '127.0.0.1', 'port': 7000} + for key in (stub_dev.keys()): + dev = stub_dev.copy() + dev.pop(key) + self.assertRaises(ValueError, rb.add_dev, dev) def test_set_dev_weight(self): rb = ring.RingBuilder(8, 3, 1)