)]}'
{"swift/common/ring/builder.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"88ab7f7f913a33854a3871f3cea3bc31877985ea","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        # Add holes to self.devs to ensure self.devs[dev[\u0027id\u0027]] will be the dev"},{"line_number":368,"context_line":"        while dev[\u0027id\u0027] \u003e\u003d len(self.devs):"},{"line_number":369,"context_line":"            self.devs.append(None)"},{"line_number":370,"context_line":"        required_keys \u003d (\u0027ip\u0027, \u0027port\u0027, \u0027weight\u0027)"},{"line_number":371,"context_line":"        if any(required not in dev for required in required_keys):"},{"line_number":372,"context_line":"            raise ValueError("},{"line_number":373,"context_line":"                \u0027%r is missing at least one the required key %r\u0027 % ("}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_82695c8c","line":370,"updated":"2017-04-26 06:58:02.000000000","message":"I\u0027m wondering why \u0027region\u0027, \u0027zone\u0027, and \u0027device\u0027 are not in required_keys?\n\nLooking at swift.common.ring.utils.parse_add_value and swift.cli.ringbuilder, \u0027zone\u0027 and \u0027device\u0027 seems required. For \u0027region\u0027, i think client tool maintains it (i.e. client tool insert region 1 if not defined so that those keys should be in here, shouldn\u0027t them?","commit_id":"6be5196fbef179691898187f6ccadfa5e17d2805"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b9f4d9c64e89e3c61eedf4b5e5d7b4d49afee145","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        # Add holes to self.devs to ensure self.devs[dev[\u0027id\u0027]] will be the dev"},{"line_number":368,"context_line":"        while dev[\u0027id\u0027] \u003e\u003d len(self.devs):"},{"line_number":369,"context_line":"            self.devs.append(None)"},{"line_number":370,"context_line":"        required_keys \u003d (\u0027ip\u0027, \u0027port\u0027, \u0027weight\u0027)"},{"line_number":371,"context_line":"        if any(required not in dev for required in required_keys):"},{"line_number":372,"context_line":"            raise ValueError("},{"line_number":373,"context_line":"                \u0027%r is missing at least one the required key %r\u0027 % ("}],"source_content_type":"text/x-python","patch_set":1,"id":"5ff73747_62f9ab4b","line":370,"in_reply_to":"5ff73747_82695c8c","updated":"2017-04-26 10:11:31.000000000","message":"Technically you can call add_dev with only ip, port and weight, and even save the .builder file - at least when you\u0027re calling the method directly:\n\n from swift.common.ring.builder import RingBuilder\n rb \u003d RingBuilder(8,1,1)\n rb.add_dev({\u0027ip\u0027:\u0027127.0.0.1\u0027, \u0027port\u0027: 6000, \u0027weight\u0027: 100})\n rb.to_dict()\n {\u0027dispersion\u0027: 0.0, \u0027_replica2part2dev\u0027: None, \u0027version\u0027: 1, \u0027replicas\u0027: 1, \u0027_dispersion_graph\u0027: {}, \u0027min_part_hours\u0027: 1, \u0027_last_part_gather_start\u0027: 0, \u0027overload\u0027: 0.0, \u0027part_power\u0027: 8, \u0027devs\u0027: [{\u0027weight\u0027: 100.0, \u0027ip\u0027: \u0027127.0.0.1\u0027, \u0027id\u0027: 0, \u0027parts\u0027: 0, \u0027meta\u0027: \u0027\u0027, \u0027port\u0027: 6000}], \u0027devs_changed\u0027: True, \u0027parts\u0027: 256, \u0027_last_part_moves_epoch\u0027: 0, \u0027id\u0027: None, \u0027_last_part_moves\u0027: None, \u0027_remove_devs\u0027: []}\n\nYou\u0027re right of course, it makes more sense to ensure region, zone and devicename do exist too, especially when using the swift-ring-builder CLI.\n\nTo me the fix looked ok because it doesn\u0027t change current behavior - there might be tools that use add_dev directly, and will set regions, zones, names later on? Or do things different than the CLI?\n\nHowever, I would be good with a follow up to check all required keys to rebalance exist.","commit_id":"6be5196fbef179691898187f6ccadfa5e17d2805"}]}