)]}'
{"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f7d152e06f764baeb9d622619ac3f71970b1c497","unresolved":false,"context_lines":[{"line_number":716,"context_line":"        self.assertFalse(self.reconstructor.check_ring(obj_ring))"},{"line_number":717,"context_line":"        rmtree(testring, ignore_errors\u003d1)"},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"    def test_reconstruct(self):"},{"line_number":720,"context_line":"        # test reconstruct logs info when check_ring is false"},{"line_number":721,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ObjectReconstructor.\u0027"},{"line_number":722,"context_line":"                        \u0027check_ring\u0027, return_value\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_02ec328e","line":719,"updated":"2017-02-21 16:19:18.000000000","message":"Might be good to have a more-specific name; this would seem to imply that we\u0027re going to actually do some reconstruction ;-)","commit_id":"ef9c3c02eafeaaadcd885ce2bbae1735d1ce7f79"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"b672067c962288b60100672b1f17102f4086379b","unresolved":false,"context_lines":[{"line_number":716,"context_line":"        self.assertFalse(self.reconstructor.check_ring(obj_ring))"},{"line_number":717,"context_line":"        rmtree(testring, ignore_errors\u003d1)"},{"line_number":718,"context_line":""},{"line_number":719,"context_line":"    def test_reconstruct(self):"},{"line_number":720,"context_line":"        # test reconstruct logs info when check_ring is false"},{"line_number":721,"context_line":"        with mock.patch(\u0027swift.obj.reconstructor.ObjectReconstructor.\u0027"},{"line_number":722,"context_line":"                        \u0027check_ring\u0027, return_value\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_1e9c6f68","line":719,"in_reply_to":"da36d5c6_02ec328e","updated":"2017-02-22 12:57:22.000000000","message":"oops! I didn\u0027t think of it that way ;) nice catch, Done.","commit_id":"ef9c3c02eafeaaadcd885ce2bbae1735d1ce7f79"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f7d152e06f764baeb9d622619ac3f71970b1c497","unresolved":false,"context_lines":[{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        error_lines \u003d self.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":1610,"context_line":"        line \u003d error_lines[0]"},{"line_number":1611,"context_line":"        self.assertTrue(\u0027Unexpected entity in data dir\u0027, line)"},{"line_number":1612,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_part_count, 6)"},{"line_number":1613,"context_line":""},{"line_number":1614,"context_line":"    def test_collect_parts_overrides(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_423b8a38","line":1611,"range":{"start_line":1611,"start_character":13,"end_line":1611,"end_character":23},"updated":"2017-02-21 16:19:18.000000000","message":"Did you mean assertIn? Should we do this for more than just the one log line?","commit_id":"ef9c3c02eafeaaadcd885ce2bbae1735d1ce7f79"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"b672067c962288b60100672b1f17102f4086379b","unresolved":false,"context_lines":[{"line_number":1608,"context_line":""},{"line_number":1609,"context_line":"        error_lines \u003d self.logger.get_lines_for_level(\u0027warning\u0027)"},{"line_number":1610,"context_line":"        line \u003d error_lines[0]"},{"line_number":1611,"context_line":"        self.assertTrue(\u0027Unexpected entity in data dir\u0027, line)"},{"line_number":1612,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_part_count, 6)"},{"line_number":1613,"context_line":""},{"line_number":1614,"context_line":"    def test_collect_parts_overrides(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"da36d5c6_9e869f11","line":1611,"range":{"start_line":1611,"start_character":13,"end_line":1611,"end_character":23},"in_reply_to":"da36d5c6_423b8a38","updated":"2017-02-22 12:57:22.000000000","message":"Done","commit_id":"ef9c3c02eafeaaadcd885ce2bbae1735d1ce7f79"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e5c6e4d891a1e2799df7a7cdfafac3267cfce081","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            self.assertIn(\u0027Invalid response 400\u0027, line)"},{"line_number":680,"context_line":"        resp \u003d do_test(Exception())"},{"line_number":681,"context_line":"        for line in self.logger.get_lines_for_level(\u0027error\u0027):"},{"line_number":682,"context_line":"            self.assertIn(\u0027Trying to GET\u0027, line)"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"    def test_reconstructor_does_not_log_on_404(self):"},{"line_number":685,"context_line":"        part \u003d self.part_nums[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_4937abfe","line":682,"updated":"2017-02-24 20:03:37.000000000","message":"Could we add a Timeout case? it\u0027s exactly after http_connect though.","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"0fbde08724e47745fb555a1fd55f101e4f276c6f","unresolved":false,"context_lines":[{"line_number":679,"context_line":"            self.assertIn(\u0027Invalid response 400\u0027, line)"},{"line_number":680,"context_line":"        resp \u003d do_test(Exception())"},{"line_number":681,"context_line":"        for line in self.logger.get_lines_for_level(\u0027error\u0027):"},{"line_number":682,"context_line":"            self.assertIn(\u0027Trying to GET\u0027, line)"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"    def test_reconstructor_does_not_log_on_404(self):"},{"line_number":685,"context_line":"        part \u003d self.part_nums[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_a91b9d7f","line":682,"in_reply_to":"da36d5c6_4937abfe","updated":"2017-02-27 14:18:40.000000000","message":"Done","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e5c6e4d891a1e2799df7a7cdfafac3267cfce081","unresolved":false,"context_lines":[{"line_number":728,"context_line":"                        \u0027check_ring\u0027, return_value\u003dFalse):"},{"line_number":729,"context_line":"            self.reconstructor.reconstruct()"},{"line_number":730,"context_line":"        msgs \u003d self.reconstructor.logger.get_lines_for_level(\u0027info\u0027)"},{"line_number":731,"context_line":"        self.assertIn(\u0027Ring change detected. Aborting\u0027"},{"line_number":732,"context_line":"                      \u0027 current reconstruction pass.\u0027, msgs[0])"},{"line_number":733,"context_line":""},{"line_number":734,"context_line":"    def test_build_reconstruction_jobs(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_84598a37","line":731,"updated":"2017-02-24 20:03:37.000000000","message":"Ok, on the master, when i remove the code for the check_ring thing, nothing failed but this patch failed here.\n\nI\u0027d extent this test more to assert no jobs built and no reconstruct happens in this state but it does never block from previous +2 from Tim.","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"0fbde08724e47745fb555a1fd55f101e4f276c6f","unresolved":false,"context_lines":[{"line_number":728,"context_line":"                        \u0027check_ring\u0027, return_value\u003dFalse):"},{"line_number":729,"context_line":"            self.reconstructor.reconstruct()"},{"line_number":730,"context_line":"        msgs \u003d self.reconstructor.logger.get_lines_for_level(\u0027info\u0027)"},{"line_number":731,"context_line":"        self.assertIn(\u0027Ring change detected. Aborting\u0027"},{"line_number":732,"context_line":"                      \u0027 current reconstruction pass.\u0027, msgs[0])"},{"line_number":733,"context_line":""},{"line_number":734,"context_line":"    def test_build_reconstruction_jobs(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_4ce6af45","line":731,"in_reply_to":"da36d5c6_84598a37","updated":"2017-02-27 14:18:40.000000000","message":"Done","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e5c6e4d891a1e2799df7a7cdfafac3267cfce081","unresolved":false,"context_lines":[{"line_number":1077,"context_line":"                                         self.logger.all_log_lines())"},{"line_number":1078,"context_line":"        self.assertEqual(self.reconstructor.suffix_sync, 8)"},{"line_number":1079,"context_line":"        self.assertEqual(self.reconstructor.suffix_count, 8)"},{"line_number":1080,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_count, 6)"},{"line_number":1081,"context_line":"        self.assertEqual(len(found_jobs), 6)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def test_process_job_all_insufficient_storage(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_c4cc628b","line":1080,"updated":"2017-02-24 20:03:37.000000000","message":"This number depends on the setUp,\n\n        # part 0: 3C1/hash/xxx#1#d.data  \u003c-- job: sync_only - partners (FI 1)\n        #         061/hash/xxx#1#d.data  \u003c-- included in earlier job (FI 1)\n        #                 /xxx#2#d.data  \u003c-- job: sync_revert to index 2\n\n        # part 1: 3C1/hash/xxx#0#d.data  \u003c-- job: sync_only - partners (FI 0)\n        #                 /xxx#1#d.data  \u003c-- job: sync_revert to index 1\n        #         061/hash/xxx#1#d.data  \u003c-- included in earlier job (FI 1)\n\n        # part 2: 3C1/hash/xxx#2#d.data  \u003c-- job: sync_revert to index 2\n        #         061/hash/xxx#0#d.data  \u003c-- job: sync_revert to index 0\n\nat L181. Make sense.\n\n\nPerhaps, we need \"detect_lockup\" tests rather than asserting reconstruction_count here? Not sure. And this should still work.","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"0fbde08724e47745fb555a1fd55f101e4f276c6f","unresolved":false,"context_lines":[{"line_number":1077,"context_line":"                                         self.logger.all_log_lines())"},{"line_number":1078,"context_line":"        self.assertEqual(self.reconstructor.suffix_sync, 8)"},{"line_number":1079,"context_line":"        self.assertEqual(self.reconstructor.suffix_count, 8)"},{"line_number":1080,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_count, 6)"},{"line_number":1081,"context_line":"        self.assertEqual(len(found_jobs), 6)"},{"line_number":1082,"context_line":""},{"line_number":1083,"context_line":"    def test_process_job_all_insufficient_storage(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_4c54efc9","line":1080,"in_reply_to":"da36d5c6_c4cc628b","updated":"2017-02-27 14:18:40.000000000","message":"Agree that it depends on SetUp. But this value assertion is still relevant to this test, right?\n\nTesting detect_lockup sounds good, will add it in the next patchset if there no objections.","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"e5c6e4d891a1e2799df7a7cdfafac3267cfce081","unresolved":false,"context_lines":[{"line_number":1620,"context_line":"                      % os.path.join(datadir_path, \u0027junk\u0027), error_lines)"},{"line_number":1621,"context_line":"        self.assertIn(\u0027Unexpected entity %r is not a directory\u0027"},{"line_number":1622,"context_line":"                      % os.path.join(datadir_path, \u00271234\u0027), error_lines)"},{"line_number":1623,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_part_count, 6)"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"    def test_collect_parts_overrides(self):"},{"line_number":1626,"context_line":"        # setup multiple devices, with multiple parts"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_e457a623","line":1623,"updated":"2017-02-24 20:03:37.000000000","message":"Oh... wait???\n\nThis looks quit odd to me. That is because setUp has only 3 partition and *reconstruction_part_count* means that the dir has jobs to sync so that so we should not count up the number of invalid part dir here?\n\nWith my diff:\n\ndiff --git a/swift/obj/reconstructor.py b/swift/obj/reconstructor.py\nindex 5363da1..c03762f 100644\n--- a/swift/obj/reconstructor.py\n+++ b/swift/obj/reconstructor.py\n@@ -865,7 +865,7 @@ class ObjectReconstructor(Daemon):\n                         self.logger.warning(\n                             \u0027Unexpected entity in data dir: %r\u0027 % part_path)\n                         self.delete_partition(part_path)\n-                        self.reconstruction_part_count +\u003d 1\n+                        # self.reconstruction_part_count +\u003d 1\n                         continue\n                     partition \u003d int(partition)\n                     if override_partitions and (partition not in\n\nThis test fails wiht 4 !\u003d 6 but it still weird, it should be 3 !\u003d 6 if i understand the setup correctly? Am i missing something?","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"0fbde08724e47745fb555a1fd55f101e4f276c6f","unresolved":false,"context_lines":[{"line_number":1620,"context_line":"                      % os.path.join(datadir_path, \u0027junk\u0027), error_lines)"},{"line_number":1621,"context_line":"        self.assertIn(\u0027Unexpected entity %r is not a directory\u0027"},{"line_number":1622,"context_line":"                      % os.path.join(datadir_path, \u00271234\u0027), error_lines)"},{"line_number":1623,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_part_count, 6)"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"    def test_collect_parts_overrides(self):"},{"line_number":1626,"context_line":"        # setup multiple devices, with multiple parts"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_2c509b5e","line":1623,"in_reply_to":"da36d5c6_780a7a8b","updated":"2017-02-27 14:18:40.000000000","message":"Thanks Tim for the details. That was my conclusion too regarding why \"1234\" was passed on - [1] tests only for non digit partitions\n\n[1]https://github.com/openstack/swift/blob/2.13.0/swift/obj/reconstructor.py#L876-L880\n\nI\u0027m not entirely convinced of changing existing way of incrementing self.reconstruction_part_count, because this inconsistency doesn\u0027t seem harmful? Unless there are other stronger points which I\u0027m missing.","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"eb5febc089315cdda6efe4972a6dbfc2308b9411","unresolved":false,"context_lines":[{"line_number":1620,"context_line":"                      % os.path.join(datadir_path, \u0027junk\u0027), error_lines)"},{"line_number":1621,"context_line":"        self.assertIn(\u0027Unexpected entity %r is not a directory\u0027"},{"line_number":1622,"context_line":"                      % os.path.join(datadir_path, \u00271234\u0027), error_lines)"},{"line_number":1623,"context_line":"        self.assertEqual(self.reconstructor.reconstruction_part_count, 6)"},{"line_number":1624,"context_line":""},{"line_number":1625,"context_line":"    def test_collect_parts_overrides(self):"},{"line_number":1626,"context_line":"        # setup multiple devices, with multiple parts"}],"source_content_type":"text/x-python","patch_set":4,"id":"da36d5c6_780a7a8b","line":1623,"in_reply_to":"da36d5c6_e457a623","updated":"2017-02-24 20:27:43.000000000","message":"\"not\" and \"junk\" increment reconstruction_part_count there, but \"1234\" looks enough like a partition to make it through to reconstruct [1], where we figure out that it doesn\u0027t have any jobs associated with it and delete it.\n\nhttps://github.com/openstack/swift/blob/2.13.0/swift/obj/reconstructor.py#L944-L955\n\nSo I\u0027m a little torn, and I think there may have to live with a bit of a wart here. We *could* stop incrementing self.reconstruction_part_count there, but then we wouldn\u0027t get to 100% in the stats for the run. We could *instead* decrement self.part_count there, but then we\u0027d be inconsistent between different types of non-partitions. We could further switch to decrementing self.part_count in reconstruct, but then we\u0027re changing the total number of parts over the course of the run, which seems weird. I guess we were doing that before, too, though?","commit_id":"3946cada784ce847c69a19e44be34f5527c75554"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"2d848ead3bd5220cda61043f3ce34deb6afbad10","unresolved":false,"context_lines":[{"line_number":680,"context_line":"        for line in self.logger.get_lines_for_level(\u0027warning\u0027):"},{"line_number":681,"context_line":"            self.assertIn(\u0027Invalid response 400\u0027, line)"},{"line_number":682,"context_line":"        self.logger._clear()"},{"line_number":683,"context_line":"        resp \u003d do_test(Exception())"},{"line_number":684,"context_line":"        for line in self.logger.get_lines_for_level(\u0027error\u0027):"},{"line_number":685,"context_line":"            self.assertIn(\u0027Trying to GET\u0027, line)"},{"line_number":686,"context_line":"        self.logger._clear()"},{"line_number":687,"context_line":"        resp \u003d do_test(Timeout())"},{"line_number":688,"context_line":"        for line in self.logger.get_lines_for_level(\u0027error\u0027):"},{"line_number":689,"context_line":"            self.assertIn(\u0027Timeout (Nones)\u0027, line)"},{"line_number":690,"context_line":""},{"line_number":691,"context_line":"    def test_reconstructor_does_not_log_on_404(self):"},{"line_number":692,"context_line":"        part \u003d self.part_nums[0]"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba2be162_bb10a65a","line":689,"range":{"start_line":683,"start_character":0,"end_line":689,"end_character":50},"updated":"2017-03-03 07:03:24.000000000","message":"I\u0027m wondering why the assertions are different even though both exception will be in the same logger exception route. Exactly only Timeout case reports \"Timeout (Nones)\"\n\nAh... the suffix \"s\" means \"seconds\"... Hmm...","commit_id":"96f8b957eeb151272ded4e89c285a29bb9797ba1"}]}