)]}'
{"swift/cli/ringbuilder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bd2f68f59b6d477180a347ba9bcae7dce2156bcc","unresolved":false,"context_lines":[{"line_number":845,"context_line":"            exit(EXIT_ERROR)"},{"line_number":846,"context_line":"        if not (parts or options.force or removed_devs):"},{"line_number":847,"context_line":"            print(\u0027No partitions could be reassigned.\u0027)"},{"line_number":848,"context_line":"            if builder.min_part_seconds_left \u003e 0 and not options.force:"},{"line_number":849,"context_line":"                print(\u0027The time between rebalances must be at least \u0027"},{"line_number":850,"context_line":"                      \u0027min_part_hours: %s hours (%s remaining)\u0027 % ("},{"line_number":851,"context_line":"                          builder.min_part_hours,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a3c09a3_752b08ec","line":848,"updated":"2017-01-13 22:34:55.000000000","message":"_last_part_moves_epoch is getting updated inside of rebalance regardless [1] - but ultimately this guard will write it back to the builder.\n\nAfter pretend_min_part_hours_passed the _last_part_moves_epoch to disk makes min_part_seconds_left answer 0 - but *here* (after we *just* ran rebalance) it\u0027s effectively \u003d\u003d min_part_hours.\n\nYou can capture the value above and get the correct behavior:\n\n    diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py\n    index fed9efc..5570d87 100644\n    --- a/swift/cli/ringbuilder.py\n    +++ b/swift/cli/ringbuilder.py\n    @@ -831,6 +831,7 @@ swift-ring-builder \u003cbuilder_file\u003e rebalance [options]\n                 logger.addHandler(handler)\n     \n             devs_changed \u003d builder.devs_changed\n    +        min_part_seconds_left \u003d builder.min_part_seconds_left\n             try:\n                 last_balance \u003d builder.get_balance()\n                 parts, balance, removed_devs \u003d builder.rebalance(seed\u003dget_seed(3))\n    @@ -845,7 +846,7 @@ swift-ring-builder \u003cbuilder_file\u003e rebalance [options]\n                 exit(EXIT_ERROR)\n             if not (parts or options.force or removed_devs):\n                 print(\u0027No partitions could be reassigned.\u0027)\n    -            if builder.min_part_seconds_left \u003e 0 and not options.force:\n    +            if min_part_seconds_left \u003e 0 and not options.force:\n                     print(\u0027The time between rebalances must be at least \u0027\n                           \u0027min_part_hours: %s hours (%s remaining)\u0027 % (\n                               builder.min_part_hours,\n\n... otherwise you get this silliness:\n\n    ubuntu@saio:/tmp$ swift-ring-builder test.builder rebalance\n    No partitions could be reassigned.\n    The time between rebalances must be at least min_part_hours: 1 hours (0:54:52 remaining)\n    ubuntu@saio:/tmp$ swift-ring-builder test.builder pretend_min_part_hours_passed\n    ubuntu@saio:/tmp$ swift-ring-builder test.builder rebalance\n    No partitions could be reassigned.\n    The time between rebalances must be at least min_part_hours: 1 hours (1:00:00 remaining)\n\n1. I believe it always correct for rebalance to maintain/update that state - regardless of if the caller decides to serialize that state update to disk.","commit_id":"4a1aee05f12dcdc2901db3e1aab81841e2905d28"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"64f862174aa85bce5f3341d26df984b7e9a5c99a","unresolved":false,"context_lines":[{"line_number":845,"context_line":"            exit(EXIT_ERROR)"},{"line_number":846,"context_line":"        if not (parts or options.force or removed_devs):"},{"line_number":847,"context_line":"            print(\u0027No partitions could be reassigned.\u0027)"},{"line_number":848,"context_line":"            if builder.min_part_seconds_left \u003e 0 and not options.force:"},{"line_number":849,"context_line":"                print(\u0027The time between rebalances must be at least \u0027"},{"line_number":850,"context_line":"                      \u0027min_part_hours: %s hours (%s remaining)\u0027 % ("},{"line_number":851,"context_line":"                          builder.min_part_hours,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a3c09a3_c289f627","line":848,"in_reply_to":"7a3c09a3_752b08ec","updated":"2017-01-17 11:33:55.000000000","message":"Ah yes, good catch - thx for fixing this!","commit_id":"4a1aee05f12dcdc2901db3e1aab81841e2905d28"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e18ad36d5e573683aa58e3f42fc283b13a851c72","unresolved":false,"context_lines":[{"line_number":867,"context_line":"            exit(EXIT_ERROR)"},{"line_number":868,"context_line":"        if not (parts or options.force or removed_devs):"},{"line_number":869,"context_line":"            print(\u0027No partitions could be reassigned.\u0027)"},{"line_number":870,"context_line":"            if min_part_seconds_left \u003e 0 and not options.force:"},{"line_number":871,"context_line":"                print(\u0027The time between rebalances must be at least \u0027"},{"line_number":872,"context_line":"                      \u0027min_part_hours: %s hours (%s remaining)\u0027 % ("},{"line_number":873,"context_line":"                          builder.min_part_hours,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a3c09a3_47a294fe","line":870,"updated":"2017-01-16 19:56:52.000000000","message":"can you *ever* be in the block if options.force is True?","commit_id":"c468b598579d4cf6089ca59c4182c0892302641d"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"be225aefa412d8cf321e7d2fe174068574fe1b73","unresolved":false,"context_lines":[{"line_number":867,"context_line":"            exit(EXIT_ERROR)"},{"line_number":868,"context_line":"        if not (parts or options.force or removed_devs):"},{"line_number":869,"context_line":"            print(\u0027No partitions could be reassigned.\u0027)"},{"line_number":870,"context_line":"            if min_part_seconds_left \u003e 0 and not options.force:"},{"line_number":871,"context_line":"                print(\u0027The time between rebalances must be at least \u0027"},{"line_number":872,"context_line":"                      \u0027min_part_hours: %s hours (%s remaining)\u0027 % ("},{"line_number":873,"context_line":"                          builder.min_part_hours,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a3c09a3_2270fadb","line":870,"in_reply_to":"7a3c09a3_4228a6e2","updated":"2017-01-17 11:37:39.000000000","message":"Actually, this is a classic copy-n-paste error - it was part of the previous condition. Fixed.","commit_id":"c468b598579d4cf6089ca59c4182c0892302641d"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"64c375b9c6538bccc1e74e193d5403c6ea9e83c7","unresolved":false,"context_lines":[{"line_number":867,"context_line":"            exit(EXIT_ERROR)"},{"line_number":868,"context_line":"        if not (parts or options.force or removed_devs):"},{"line_number":869,"context_line":"            print(\u0027No partitions could be reassigned.\u0027)"},{"line_number":870,"context_line":"            if min_part_seconds_left \u003e 0 and not options.force:"},{"line_number":871,"context_line":"                print(\u0027The time between rebalances must be at least \u0027"},{"line_number":872,"context_line":"                      \u0027min_part_hours: %s hours (%s remaining)\u0027 % ("},{"line_number":873,"context_line":"                          builder.min_part_hours,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a3c09a3_4228a6e2","line":870,"in_reply_to":"7a3c09a3_47a294fe","updated":"2017-01-17 11:34:07.000000000","message":"Ah, good catch. No, you can\u0027t. So the second condition can be removed","commit_id":"c468b598579d4cf6089ca59c4182c0892302641d"}],"test/unit/cli/test_ringbuilder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"bd2f68f59b6d477180a347ba9bcae7dce2156bcc","unresolved":false,"context_lines":[{"line_number":1824,"context_line":"        argvs \u003d ["},{"line_number":1825,"context_line":"            [\"\", self.tmpfile, \"rebalance\", \"3\"],"},{"line_number":1826,"context_line":"            [\"\", self.tmpfile, \"remove\", \"d0\"],"},{"line_number":1827,"context_line":"            [\"\", self.tmpfile, \"rebalance\", \"3\"]]"},{"line_number":1828,"context_line":"        for argv in argvs:"},{"line_number":1829,"context_line":"            self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv)"},{"line_number":1830,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a3c09a3_272153fc","line":1827,"updated":"2017-01-13 22:34:55.000000000","message":"I like this test, if I \"fix\" it like this:\n\n    diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py\n    index 131c58d..5818535 100644\n    --- a/test/unit/cli/test_ringbuilder.py\n    +++ b/test/unit/cli/test_ringbuilder.py\n    @@ -1846,7 +1846,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):\n             argvs \u003d [\n                 [\"\", self.tmpfile, \"rebalance\", \"3\"],\n                 [\"\", self.tmpfile, \"remove\", \"d0\"],\n    -            [\"\", self.tmpfile, \"rebalance\", \"3\"]]\n    +            [\"\", self.tmpfile, \"rebalance\", \"3\", \"-f\"]]\n             for argv in argvs:\n                 self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv)\n     \n\nIt passes even on master.  This is a really clear indicator for me this change is addressing the open bug.","commit_id":"4a1aee05f12dcdc2901db3e1aab81841e2905d28"}]}
