)]}'
{"swift/obj/diskfile.py":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"d4e7f9d3301bf2d05f3eff789a50e817572f0606","unresolved":false,"context_lines":[{"line_number":878,"context_line":"        # If put is successful, avoid unnecessary os.unlink() of tempfile"},{"line_number":879,"context_line":"        # later as the rename would have succeeded. Hence, the tempfile"},{"line_number":880,"context_line":"        # wouldn\u0027t exist at its original location."},{"line_number":881,"context_line":"        self._tmppath \u003d None"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"class DiskFileReader(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_59841bb1","line":881,"updated":"2014-12-29 07:44:41.000000000","message":"Is this maybe prone to a race condition error? If - for whatever reason - self._finalize_put has not finished yet (run in a thread?), it won\u0027t be able to finish ever.\n\nI think it is more safe to set self._tmppath to None in self._finalize_put. Just to be sure...","commit_id":"f079bd0cfaf466029377b9ffc9a2efcad51d56e7"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"f29a70162d50512bce16c56506ef55a38df4b448","unresolved":false,"context_lines":[{"line_number":878,"context_line":"        # If put is successful, avoid unnecessary os.unlink() of tempfile"},{"line_number":879,"context_line":"        # later as the rename would have succeeded. Hence, the tempfile"},{"line_number":880,"context_line":"        # wouldn\u0027t exist at its original location."},{"line_number":881,"context_line":"        self._tmppath \u003d None"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"class DiskFileReader(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_bc4de29b","line":881,"updated":"2014-12-29 19:10:14.000000000","message":"The threadpool calls are synchronous, so I don\u0027t think there\u0027s a safety problem here. However, I think moving this line into _finalize_put is easier to read; the closer this is to the rename() call, the more obvious its purpose becomes.","commit_id":"f079bd0cfaf466029377b9ffc9a2efcad51d56e7"},{"author":{"_account_id":8542,"name":"Prashanth Pai","email":"ppai@redhat.com","username":"pai"},"change_message_id":"b5614c321f335899e2b940c97ecb98f544448e0d","unresolved":false,"context_lines":[{"line_number":878,"context_line":"        # If put is successful, avoid unnecessary os.unlink() of tempfile"},{"line_number":879,"context_line":"        # later as the rename would have succeeded. Hence, the tempfile"},{"line_number":880,"context_line":"        # wouldn\u0027t exist at its original location."},{"line_number":881,"context_line":"        self._tmppath \u003d None"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":""},{"line_number":884,"context_line":"class DiskFileReader(object):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a961159_5cb877e5","line":881,"in_reply_to":"3a961159_bc4de29b","updated":"2014-12-30 07:27:46.000000000","message":"Done","commit_id":"f079bd0cfaf466029377b9ffc9a2efcad51d56e7"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"ad71c2d45b44ab7bcb24d0597a761ac979b3a0ed","unresolved":false,"context_lines":[{"line_number":857,"context_line":"        # After the rename completes, this object will be available for other"},{"line_number":858,"context_line":"        # requests to reference."},{"line_number":859,"context_line":"        renamer(self._tmppath, target_path)"},{"line_number":860,"context_line":"        # If rename is successful, flag put as succeeded. This is done to avoid"},{"line_number":861,"context_line":"        # unnecessary os.unlink() of tempfile later. As renamer() has"},{"line_number":862,"context_line":"        # succeeded, the tempfile would no longer exist at its original path."},{"line_number":863,"context_line":"        self._put_succeeded \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_2bca59a1","line":860,"updated":"2015-01-02 18:48:21.000000000","message":"I think I\u0027m probably missing something, renamer() just tries twice but give no indication of whether it really succeeded or not, right?  Wouldn\u0027t you need a return value from renamer or something (and maybe set the flag directly from that)?","commit_id":"266fb4e17686a9197334de1ad252eb2656762366"},{"author":{"_account_id":2622,"name":"Samuel Merritt","email":"spam+launchpad@andcheese.org","username":"torgomatic"},"change_message_id":"192336b6d84da31c7cdbd84694234808d72b5017","unresolved":false,"context_lines":[{"line_number":857,"context_line":"        # After the rename completes, this object will be available for other"},{"line_number":858,"context_line":"        # requests to reference."},{"line_number":859,"context_line":"        renamer(self._tmppath, target_path)"},{"line_number":860,"context_line":"        # If rename is successful, flag put as succeeded. This is done to avoid"},{"line_number":861,"context_line":"        # unnecessary os.unlink() of tempfile later. As renamer() has"},{"line_number":862,"context_line":"        # succeeded, the tempfile would no longer exist at its original path."},{"line_number":863,"context_line":"        self._put_succeeded \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_1185a6ce","line":860,"in_reply_to":"3a961159_2bca59a1","updated":"2015-01-05 22:20:41.000000000","message":"I think the lack of OSError counts as success.","commit_id":"266fb4e17686a9197334de1ad252eb2656762366"},{"author":{"_account_id":8542,"name":"Prashanth Pai","email":"ppai@redhat.com","username":"pai"},"change_message_id":"549c4ee87c495382692846c3b826072a693ac884","unresolved":false,"context_lines":[{"line_number":857,"context_line":"        # After the rename completes, this object will be available for other"},{"line_number":858,"context_line":"        # requests to reference."},{"line_number":859,"context_line":"        renamer(self._tmppath, target_path)"},{"line_number":860,"context_line":"        # If rename is successful, flag put as succeeded. This is done to avoid"},{"line_number":861,"context_line":"        # unnecessary os.unlink() of tempfile later. As renamer() has"},{"line_number":862,"context_line":"        # succeeded, the tempfile would no longer exist at its original path."},{"line_number":863,"context_line":"        self._put_succeeded \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_543e552a","line":860,"in_reply_to":"3a961159_2bca59a1","updated":"2015-01-06 03:49:55.000000000","message":"If no exception is raised, I think it is safe to assume that it was successful. os.rename() raises OSError if something goes wrong, does not return anything if successful.","commit_id":"266fb4e17686a9197334de1ad252eb2656762366"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"ad71c2d45b44ab7bcb24d0597a761ac979b3a0ed","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"                try:"},{"line_number":1628,"context_line":"                    os.unlink(tmppath)"},{"line_number":1629,"context_line":"                except OSError:"},{"line_number":1630,"context_line":"                    logging.exception(_(\u0027Error removing tempfile: %s\u0027),"},{"line_number":1631,"context_line":"                                      tmppath)"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"    def write_metadata(self, metadata):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_cbe1d52b","line":1630,"updated":"2015-01-02 18:48:21.000000000","message":"this needs unit test coverage","commit_id":"266fb4e17686a9197334de1ad252eb2656762366"},{"author":{"_account_id":8542,"name":"Prashanth Pai","email":"ppai@redhat.com","username":"pai"},"change_message_id":"549c4ee87c495382692846c3b826072a693ac884","unresolved":false,"context_lines":[{"line_number":1627,"context_line":"                try:"},{"line_number":1628,"context_line":"                    os.unlink(tmppath)"},{"line_number":1629,"context_line":"                except OSError:"},{"line_number":1630,"context_line":"                    logging.exception(_(\u0027Error removing tempfile: %s\u0027),"},{"line_number":1631,"context_line":"                                      tmppath)"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"    def write_metadata(self, metadata):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_517004a4","line":1630,"in_reply_to":"3a961159_cbe1d52b","updated":"2015-01-06 03:49:55.000000000","message":"Done","commit_id":"266fb4e17686a9197334de1ad252eb2656762366"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":8542,"name":"Prashanth Pai","email":"ppai@redhat.com","username":"pai"},"change_message_id":"0c8a93b6204da6cc6bda7e9297d44a12368e9ed7","unresolved":false,"context_lines":[{"line_number":2414,"context_line":"                    self.fail(\"Expected OSError exception\")"},{"line_number":2415,"context_line":"        self.assertFalse(writer.put_succeeded)"},{"line_number":2416,"context_line":"        self.assertTrue(_m_renamer.called)"},{"line_number":2417,"context_line":"        self.assertTrue(_m_unlink.called)"},{"line_number":2418,"context_line":"        self.assert_(len(self.df_mgr.logger.log_dict[\u0027exception\u0027]) \u003d\u003d 0)"},{"line_number":2419,"context_line":""},{"line_number":2420,"context_line":"    def test_create_unlink_cleanup_logging(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_99bfcecf","line":2417,"updated":"2015-01-07 09:23:33.000000000","message":"I just noticed that as actual unlink is mocked and rename does not happen, the tempfile might remain. I only see cleanup of self.testdir in tearDown(). Is something like this necessary ?\n\n    try:\n        ......\n    finally:\n        try:\n            os.unlink(_m_unlink.call_args[0][0])\n        except OSError as e:\n            if e.errno !\u003d errno.ENOENT:\n                raise","commit_id":"42c790d04b85e2d2665da7c13f800d03b263a22f"},{"author":{"_account_id":8542,"name":"Prashanth Pai","email":"ppai@redhat.com","username":"pai"},"change_message_id":"6dfa22895d2b726a930be5b4dbaf311797c828e2","unresolved":false,"context_lines":[{"line_number":2414,"context_line":"                    self.fail(\"Expected OSError exception\")"},{"line_number":2415,"context_line":"        self.assertFalse(writer.put_succeeded)"},{"line_number":2416,"context_line":"        self.assertTrue(_m_renamer.called)"},{"line_number":2417,"context_line":"        self.assertTrue(_m_unlink.called)"},{"line_number":2418,"context_line":"        self.assert_(len(self.df_mgr.logger.log_dict[\u0027exception\u0027]) \u003d\u003d 0)"},{"line_number":2419,"context_line":""},{"line_number":2420,"context_line":"    def test_create_unlink_cleanup_logging(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_a83d1290","line":2417,"in_reply_to":"3a961159_6d189c84","updated":"2015-01-07 12:25:39.000000000","message":"Thanks Christian :)","commit_id":"42c790d04b85e2d2665da7c13f800d03b263a22f"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b9dd154d4c0d5d5f4346de1037cc008369c862fc","unresolved":false,"context_lines":[{"line_number":2414,"context_line":"                    self.fail(\"Expected OSError exception\")"},{"line_number":2415,"context_line":"        self.assertFalse(writer.put_succeeded)"},{"line_number":2416,"context_line":"        self.assertTrue(_m_renamer.called)"},{"line_number":2417,"context_line":"        self.assertTrue(_m_unlink.called)"},{"line_number":2418,"context_line":"        self.assert_(len(self.df_mgr.logger.log_dict[\u0027exception\u0027]) \u003d\u003d 0)"},{"line_number":2419,"context_line":""},{"line_number":2420,"context_line":"    def test_create_unlink_cleanup_logging(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a961159_6d189c84","line":2417,"in_reply_to":"3a961159_99bfcecf","updated":"2015-01-07 12:15:17.000000000","message":"I think this is not needed - I quickly tested this, on my SAIO the tempfile is removed by tearDown().","commit_id":"42c790d04b85e2d2665da7c13f800d03b263a22f"}]}