)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d568aa6882b9ec224f693a2579ff9b96642344b8","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check if device name is valid when adding to the ring"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Curently device names can be empty or start and/or end with spaces."},{"line_number":10,"context_line":"This can create unexpected results, for example these three commands"},{"line_number":11,"context_line":"are all valid:"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da9b358b_16861313","line":9,"updated":"2015-04-01 10:05:52.000000000","message":"s/Curently/Currently","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"1e4fdef60b8bbc6fdf6735d7e58bead6f3bd9fee","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Check if device name is valid when adding to the ring"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Curently device names can be empty or start and/or end with spaces."},{"line_number":10,"context_line":"This can create unexpected results, for example these three commands"},{"line_number":11,"context_line":"are all valid:"},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa041b7_00188683","line":9,"in_reply_to":"da9b358b_16861313","updated":"2015-04-01 14:34:54.000000000","message":"Done","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"}],"swift/cli/ringbuilder.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"59599ae9cb897798dc43be2fc3557d3d6e9a7113","unresolved":false,"context_lines":[{"line_number":240,"context_line":"                                \u0027replication_ip\u0027: replication_ip,"},{"line_number":241,"context_line":"                                \u0027replication_port\u0027: replication_port,"},{"line_number":242,"context_line":"                                \u0027weight\u0027: weight, \u0027meta\u0027: meta})"},{"line_number":243,"context_line":"    else:"},{"line_number":244,"context_line":"        parsed_devs.append(build_dev_from_opts(opts))"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    return parsed_devs"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9b358b_fea9617c","line":243,"updated":"2015-03-31 17:29:27.000000000","message":"What do you think about adding a same as above check here for opts.device?","commit_id":"0ad551c967536315df3d33b669d6f98d2bb8972d"}],"swift/common/ring/utils.py":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"4c2857e56dce7965447cc6179cddd269eef87252","unresolved":false,"context_lines":[{"line_number":516,"context_line":"    replication_port \u003d opts.replication_port or opts.port"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    if not validate_device_name(opts.device):"},{"line_number":519,"context_line":"        raise ValueError()"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    return {\u0027region\u0027: opts.region, \u0027zone\u0027: opts.zone, \u0027ip\u0027: ip,"},{"line_number":522,"context_line":"            \u0027port\u0027: opts.port, \u0027device\u0027: opts.device, \u0027meta\u0027: opts.meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_77932650","line":519,"updated":"2015-04-01 02:29:18.000000000","message":"no message with ValueError? I think it\u0027s caugh by the caller.","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"cda8860747635a2f8c2d02a187232fa336f59ae2","unresolved":false,"context_lines":[{"line_number":516,"context_line":"    replication_port \u003d opts.replication_port or opts.port"},{"line_number":517,"context_line":""},{"line_number":518,"context_line":"    if not validate_device_name(opts.device):"},{"line_number":519,"context_line":"        raise ValueError()"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    return {\u0027region\u0027: opts.region, \u0027zone\u0027: opts.zone, \u0027ip\u0027: ip,"},{"line_number":522,"context_line":"            \u0027port\u0027: opts.port, \u0027device\u0027: opts.device, \u0027meta\u0027: opts.meta,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_ccba6023","line":519,"in_reply_to":"da9b358b_77932650","updated":"2015-04-01 06:34:54.000000000","message":"Done, thx Pete!","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"}],"test/unit/cli/test_ringbuilder.py":[{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d369e5de356b585d7e803de47d4181d8cc937fb8","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"            ringbuilder.main(argv)"},{"line_number":1715,"context_line":"        except SystemExit as e:"},{"line_number":1716,"context_line":"            err \u003d e"},{"line_number":1717,"context_line":"        self.assertEquals(err.code, 1)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_268b32c5","line":1717,"updated":"2015-04-01 00:22:52.000000000","message":"This code works because the err always fulfill with e but I prefer to declare error_code before try statement then put e.code to the error_code.\nI wrote comments for my code image in test_invalid_device_name.","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"cda8860747635a2f8c2d02a187232fa336f59ae2","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"            ringbuilder.main(argv)"},{"line_number":1715,"context_line":"        except SystemExit as e:"},{"line_number":1716,"context_line":"            err \u003d e"},{"line_number":1717,"context_line":"        self.assertEquals(err.code, 1)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_6ced9424","line":1717,"in_reply_to":"da9b358b_268b32c5","updated":"2015-04-01 06:34:54.000000000","message":"Hisashi: agreed, that would make it a little bit more clear.\n\nHowever, I would like to avoid refactoring all the code in this patch - actually this style is used in many more places in this file, and I would like to keep it in the same style for now.\n\nA follow up patch could refactor this if we think it is required.","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"f147ab6965eff065b58eedca026e48a1df74bb3a","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"            ringbuilder.main(argv)"},{"line_number":1715,"context_line":"        except SystemExit as e:"},{"line_number":1716,"context_line":"            err \u003d e"},{"line_number":1717,"context_line":"        self.assertEquals(err.code, 1)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"}],"source_content_type":"text/x-python","patch_set":2,"id":"baa041b7_7295cbe2","line":1717,"in_reply_to":"da9b358b_6ced9424","updated":"2015-04-02 03:35:26.000000000","message":"Please look again, all of the existing code sets err to None ahead of the test.","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d369e5de356b585d7e803de47d4181d8cc937fb8","unresolved":false,"context_lines":[{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"},{"line_number":1721,"context_line":"        for device_name in [\"\", \" sda1\", \"sda1 \", \" meta \"]:"},{"line_number":1722,"context_line":"            err \u003d 0"},{"line_number":1723,"context_line":""},{"line_number":1724,"context_line":"            argv \u003d [\"\","},{"line_number":1725,"context_line":"                    self.tmpfile,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_2628d2e1","line":1722,"updated":"2015-04-01 00:22:52.000000000","message":"I prefer to declare error_code \u003d 0 instead of err\u003d0 here.","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d369e5de356b585d7e803de47d4181d8cc937fb8","unresolved":false,"context_lines":[{"line_number":1729,"context_line":"            try:"},{"line_number":1730,"context_line":"                ringbuilder.main(argv)"},{"line_number":1731,"context_line":"            except SystemExit as exc:"},{"line_number":1732,"context_line":"                err \u003d exc"},{"line_number":1733,"context_line":"            self.assertEquals(err.code, 2)"},{"line_number":1734,"context_line":""},{"line_number":1735,"context_line":"            argv \u003d [\"\","}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_86643e49","line":1732,"updated":"2015-04-01 00:22:52.000000000","message":"error_code \u003d exc.code","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"d369e5de356b585d7e803de47d4181d8cc937fb8","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                ringbuilder.main(argv)"},{"line_number":1731,"context_line":"            except SystemExit as exc:"},{"line_number":1732,"context_line":"                err \u003d exc"},{"line_number":1733,"context_line":"            self.assertEquals(err.code, 2)"},{"line_number":1734,"context_line":""},{"line_number":1735,"context_line":"            argv \u003d [\"\","},{"line_number":1736,"context_line":"                    self.tmpfile,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da9b358b_06238e06","line":1733,"updated":"2015-04-01 00:22:52.000000000","message":"self.assertEquals(error_code)","commit_id":"2b78b09009d4894ecbd633b9794551f733d2ac1f"},{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d568aa6882b9ec224f693a2579ff9b96642344b8","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"            ringbuilder.main(argv)"},{"line_number":1715,"context_line":"        except SystemExit as e:"},{"line_number":1716,"context_line":"            err \u003d e"},{"line_number":1717,"context_line":"        self.assertEquals(err.code, 1)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"}],"source_content_type":"text/x-python","patch_set":3,"id":"da9b358b_b14c1171","line":1717,"updated":"2015-04-01 10:05:52.000000000","message":"Right now if the SystemExit won\u0027t occur the test will fail (that\u0027s good), but the result message won\u0027t be clear to the user. It will simply says that variable `err` is not defined in current context. Better solution for this would be:\n\n\nwith self.assertRaises(SystemExit) as context:\n    ringbuilder.main(argv)\nself.assertEqual(context.exception.code, 1)","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"1e4fdef60b8bbc6fdf6735d7e58bead6f3bd9fee","unresolved":false,"context_lines":[{"line_number":1714,"context_line":"            ringbuilder.main(argv)"},{"line_number":1715,"context_line":"        except SystemExit as e:"},{"line_number":1716,"context_line":"            err \u003d e"},{"line_number":1717,"context_line":"        self.assertEquals(err.code, 1)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_603e32ba","line":1717,"in_reply_to":"da9b358b_b14c1171","updated":"2015-04-01 14:34:54.000000000","message":"This would be nice, but won\u0027t work on Python 2.6, and we need to support 2.6 testing for now.\n\nLike in the previous patchset said - I\u0027m fine to refactor all tests in this file, but in a separare patch to keep a single style of testing in this file currently.","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d568aa6882b9ec224f693a2579ff9b96642344b8","unresolved":false,"context_lines":[{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"},{"line_number":1721,"context_line":"        for device_name in [\"\", \" sda1\", \"sda1 \", \" meta \"]:"},{"line_number":1722,"context_line":"            err \u003d 0"},{"line_number":1723,"context_line":""},{"line_number":1724,"context_line":"            argv \u003d [\"\","}],"source_content_type":"text/x-python","patch_set":3,"id":"da9b358b_11729dad","line":1721,"updated":"2015-04-01 10:05:52.000000000","message":"nit: \" \" is a good candidate to check as well.","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"1e4fdef60b8bbc6fdf6735d7e58bead6f3bd9fee","unresolved":false,"context_lines":[{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"    def test_invalid_device_name(self):"},{"line_number":1720,"context_line":"        self.create_sample_ring()"},{"line_number":1721,"context_line":"        for device_name in [\"\", \" sda1\", \"sda1 \", \" meta \"]:"},{"line_number":1722,"context_line":"            err \u003d 0"},{"line_number":1723,"context_line":""},{"line_number":1724,"context_line":"            argv \u003d [\"\","}],"source_content_type":"text/x-python","patch_set":3,"id":"baa041b7_c0a4de09","line":1721,"in_reply_to":"da9b358b_11729dad","updated":"2015-04-01 14:34:54.000000000","message":"Good point, done","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d568aa6882b9ec224f693a2579ff9b96642344b8","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                ringbuilder.main(argv)"},{"line_number":1731,"context_line":"            except SystemExit as exc:"},{"line_number":1732,"context_line":"                err \u003d exc"},{"line_number":1733,"context_line":"            self.assertEquals(err.code, 2)"},{"line_number":1734,"context_line":""},{"line_number":1735,"context_line":"            argv \u003d [\"\","},{"line_number":1736,"context_line":"                    self.tmpfile,"}],"source_content_type":"text/x-python","patch_set":3,"id":"da9b358b_b163f1db","line":1733,"updated":"2015-04-01 10:05:52.000000000","message":"ditto like it test_warn_at_risk","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d568aa6882b9ec224f693a2579ff9b96642344b8","unresolved":false,"context_lines":[{"line_number":1745,"context_line":"                ringbuilder.main(argv)"},{"line_number":1746,"context_line":"            except SystemExit as exc:"},{"line_number":1747,"context_line":"                err \u003d exc"},{"line_number":1748,"context_line":"            self.assertEquals(err.code, 2)"},{"line_number":1749,"context_line":""},{"line_number":1750,"context_line":""},{"line_number":1751,"context_line":"class TestRebalanceCommand(unittest.TestCase, RunSwiftRingBuilderMixin):"}],"source_content_type":"text/x-python","patch_set":3,"id":"da9b358b_915ead10","line":1748,"updated":"2015-04-01 10:05:52.000000000","message":"ditto","commit_id":"ef4dbcafeef8117f644e5a4792e5c5d1c83cb454"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"ccd2c2fa434a8a6454bb724c88f6e9fe095c6bdd","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"        self.assertEquals(err.code, 2)"},{"line_number":1706,"context_line":""},{"line_number":1707,"context_line":"    def test_warn_at_risk(self):"},{"line_number":1708,"context_line":"        self.create_sample_ring()"},{"line_number":1709,"context_line":"        ring \u003d RingBuilder.load(self.tmpfile)"},{"line_number":1710,"context_line":"        ring.devs[0][\u0027weight\u0027] \u003d 10"},{"line_number":1711,"context_line":"        ring.save(self.tmpfile)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa041b7_b29353fa","line":1708,"updated":"2015-04-02 03:37:08.000000000","message":"I think Hisashi was right and you need err\u003dNone right about here. And it would not be considered refactoring.","commit_id":"72a52501b9a018383dd47509b5c695571d9792c5"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"50d30d410cb9696a616b40dc5a4d5d5ff8ceacf9","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"        self.assertEquals(err.code, 2)"},{"line_number":1706,"context_line":""},{"line_number":1707,"context_line":"    def test_warn_at_risk(self):"},{"line_number":1708,"context_line":"        self.create_sample_ring()"},{"line_number":1709,"context_line":"        ring \u003d RingBuilder.load(self.tmpfile)"},{"line_number":1710,"context_line":"        ring.devs[0][\u0027weight\u0027] \u003d 10"},{"line_number":1711,"context_line":"        ring.save(self.tmpfile)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa041b7_6cd23dd4","line":1708,"in_reply_to":"baa041b7_2320179b","updated":"2015-04-05 15:43:34.000000000","message":"I\u0027m buying Christian\u0027s defence that \"all existing tests do this\", too. So if he just adds None like in every other test, that\u0027d be fine by me. Refactoring the whole file would be another patch.","commit_id":"72a52501b9a018383dd47509b5c695571d9792c5"},{"author":{"_account_id":13161,"name":"Kamil Rykowski","email":"kamil.rykowski@gmail.com","username":"krykowski"},"change_message_id":"d65c33215c2da6f71ea7338cb09739161860eef5","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"        self.assertEquals(err.code, 2)"},{"line_number":1706,"context_line":""},{"line_number":1707,"context_line":"    def test_warn_at_risk(self):"},{"line_number":1708,"context_line":"        self.create_sample_ring()"},{"line_number":1709,"context_line":"        ring \u003d RingBuilder.load(self.tmpfile)"},{"line_number":1710,"context_line":"        ring.devs[0][\u0027weight\u0027] \u003d 10"},{"line_number":1711,"context_line":"        ring.save(self.tmpfile)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa041b7_2320179b","line":1708,"in_reply_to":"baa041b7_b29353fa","updated":"2015-04-02 05:31:54.000000000","message":"Even better, as Hisashi mention in #PS2 it could be written using error_code \u003d 0, so the code will look cleaner and no AttributeError (None.code) and NameError (when err is not defined) will be raised.","commit_id":"72a52501b9a018383dd47509b5c695571d9792c5"}]}