)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"241e7656b23c024c0b6ad023ce18b0b1f95acfab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"49e42212_3d1325c5","updated":"2024-12-13 17:06:05.000000000","message":"the diffs with mock.ANY is a little hard to read, the problem is _last is missing from the expected:\n```\nE   + [{\u0027object_updater_last\u0027: 1734017398.438799,\nE   - [{\u0027object_updater_stats\u0027: {\u0027failures_account_container_count\u0027: 0,\n```\ntest/unit/obj/test_updater.py:705: in assert_and_reset_recon_dump","commit_id":"05e70117581829ef22d5a9a60b24e4e4b20e8ec2"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"12fe278bfccc48b8f3de88bc79843577d1d53041","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3bc76ec8_e7b825b2","in_reply_to":"49e42212_3d1325c5","updated":"2024-12-13 19:22:18.000000000","message":"Ahh, I didn\u0027t even have this test locally and took me too long before I realized that I needed to update my fork 🤦‍♂️","commit_id":"05e70117581829ef22d5a9a60b24e4e4b20e8ec2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b2faeab8ccee5ec9cf622fcfc7280bbebb3ce44e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b8358f6a_cca7aeae","updated":"2024-12-19 00:51:51.000000000","message":"recheck\n\nLooks like some errors transferring logs or something?","commit_id":"753f17d76b708aa252503fc3146303031141b83e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"6f7d5788_09d483c7","updated":"2024-12-20 16:46:13.000000000","message":"I need to look at this again after patchset 6","commit_id":"5281af5cf25f1a88b23441d1c45f0d86735395db"}],"swift/common/middleware/recon.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":83,"context_line":"        try:"},{"line_number":84,"context_line":"            with openr(cache_file, \u0027r\u0027) as f:"},{"line_number":85,"context_line":"                recondata \u003d json.load(f)"},{"line_number":86,"context_line":"                return {key: recondata.get(key) for key in cache_keys}"},{"line_number":87,"context_line":"        except IOError as err:"},{"line_number":88,"context_line":"            if err.errno \u003d\u003d errno.ENOENT and ignore_missing:"},{"line_number":89,"context_line":"                pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"0a37e97c_c19751d0","line":86,"updated":"2024-12-20 16:46:13.000000000","message":"I guess we *have* to specify which keys we want so we can return specific \"sets\" of cache keys for different services.","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"}],"swift/obj/updater.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0720460b820813be040f8acc676e734b3c26009c","unresolved":true,"context_lines":[{"line_number":576,"context_line":"            {"},{"line_number":577,"context_line":"                \u0027object_updater_sweep\u0027: elapsed,"},{"line_number":578,"context_line":"                \u0027object_updater_stats\u0027: object_updater_stats,"},{"line_number":579,"context_line":"                \u0027object_updater_last\u0027: self.begin"},{"line_number":580,"context_line":"            },"},{"line_number":581,"context_line":"            self.rcache,"},{"line_number":582,"context_line":"            self.logger,"}],"source_content_type":"text/x-python","patch_set":4,"id":"96fc0756_a0f16fea","line":579,"range":{"start_line":579,"start_character":44,"end_line":579,"end_character":49},"updated":"2024-12-19 06:11:34.000000000","message":"Looking at other `_last` times in recon, it seems like it\u0027s always the *end* time; why are we looking at **start** time here?","commit_id":"753f17d76b708aa252503fc3146303031141b83e"},{"author":{"_account_id":37271,"name":"Chinemerem Chigbo","display_name":"Chinemerem","email":"cchigbo@nvidia.com","username":"chinemerem"},"change_message_id":"5548f566e13be73c643656592752ffd584a688bb","unresolved":false,"context_lines":[{"line_number":576,"context_line":"            {"},{"line_number":577,"context_line":"                \u0027object_updater_sweep\u0027: elapsed,"},{"line_number":578,"context_line":"                \u0027object_updater_stats\u0027: object_updater_stats,"},{"line_number":579,"context_line":"                \u0027object_updater_last\u0027: self.begin"},{"line_number":580,"context_line":"            },"},{"line_number":581,"context_line":"            self.rcache,"},{"line_number":582,"context_line":"            self.logger,"}],"source_content_type":"text/x-python","patch_set":4,"id":"96176d68_64c16401","line":579,"range":{"start_line":579,"start_character":44,"end_line":579,"end_character":49},"in_reply_to":"96fc0756_a0f16fea","updated":"2024-12-19 09:28:51.000000000","message":"We shouldn\u0027t be. Updated.","commit_id":"753f17d76b708aa252503fc3146303031141b83e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":551,"context_line":"            (\u0027Object update single-threaded sweep completed: \u0027"},{"line_number":552,"context_line":"             \u0027%(elapsed).02fs, %(stats)s\u0027),"},{"line_number":553,"context_line":"            {\u0027elapsed\u0027: elapsed, \u0027stats\u0027: self.stats})"},{"line_number":554,"context_line":"        self.dump_recon(elapsed, now)"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"    def dump_recon(self, elapsed, now):"},{"line_number":557,"context_line":"        \"\"\"Gathers stats and dumps recon cache.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"05a48c20_09e581d6","line":554,"updated":"2024-12-20 16:46:13.000000000","message":"this is probably going to need a rebase on 937267: Aggregate per-disk recon stats | https://review.opendev.org/c/openstack/swift/+/937267\n\nwhen you re-spin I suggest you pass in `begin` and have `dump_recon` calculate `elapsed` and call `time.time` for the `last` stat.","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"}],"test/unit/common/middleware/test_recon.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def fake_from_recon_cache(self, *args, **kwargs):"},{"line_number":68,"context_line":"        self.fakeout_calls.append((args, kwargs))"},{"line_number":69,"context_line":"        return self.fakeout"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"class OpenAndReadTester(object):"}],"source_content_type":"text/x-python","patch_set":5,"id":"40885bcd_b1837d8e","line":69,"updated":"2024-12-20 16:46:13.000000000","message":"ok, so it\u0027s not a mock - it\u0027s a fake that has call tracking but always return the same stub","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":246,"context_line":"        utils.ismount \u003d self.mockos.fake_ismount"},{"line_number":247,"context_line":"        os.statvfs \u003d self.mockos.fake_statvfs"},{"line_number":248,"context_line":"        self.real_from_cache \u003d self.app._from_recon_cache"},{"line_number":249,"context_line":"        self.app._from_recon_cache \u003d self.fakecache.fake_from_recon_cache"},{"line_number":250,"context_line":"        self.frecon \u003d FakeRecon()"},{"line_number":251,"context_line":"        self.app.logger \u003d debug_logger()"},{"line_number":252,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2d365a46_f6fe8ceb","line":249,"updated":"2024-12-20 16:46:13.000000000","message":"so we mock the app\u0027s method to always return our fakecache stub","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":837,"context_line":""},{"line_number":838,"context_line":"    def test_get_updater_info_object(self):"},{"line_number":839,"context_line":"        from_cache_response \u003d {\"object_updater_sweep\": 0.79848217964172363,"},{"line_number":840,"context_line":"                               \"object_updater_last\": 1357969645.25}"},{"line_number":841,"context_line":"        self.fakecache.fakeout_calls \u003d []"},{"line_number":842,"context_line":"        self.fakecache.fakeout \u003d from_cache_response"},{"line_number":843,"context_line":"        rv \u003d self.app.get_updater_info(\u0027object\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"445906b8_0d9bfaff","line":840,"updated":"2024-12-20 16:46:13.000000000","message":"it\u0027s probably a good idea to update this stub to be more like the real; but I think for the purpose of this test it\u0027s *mostly* just saying \"whatever is in the stub is what you get\"","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":844,"context_line":"        self.assertEqual(self.fakecache.fakeout_calls,"},{"line_number":845,"context_line":"                         [(([\u0027object_updater_sweep\u0027, \u0027object_updater_stats\u0027,"},{"line_number":846,"context_line":"                             \u0027object_updater_last\u0027],"},{"line_number":847,"context_line":"                            self._full_recon_path(\u0027object\u0027)), {})])"},{"line_number":848,"context_line":"        self.assertEqual(rv, {\"object_updater_sweep\": 0.79848217964172363,"},{"line_number":849,"context_line":"                              \"object_updater_last\": 1357969645.25})"},{"line_number":850,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"15edea83_b17be9d8","line":847,"updated":"2024-12-20 16:46:13.000000000","message":"this assert is hard to read; I think the fake is capturing calls like `self.calls.append((args, kwargs))`\n\nso we\u0027re seeing the app only called `_from_recon_cache` once like:\n\n`app._from_recon_cache([\u0027sweep\u0027, \u0027stats\u0027, \u0027last\u0027], \u0027/path/to/object\u0027)`\n\n... which I guess looks right:\n\nhttps://github.com/NVIDIA/swift/blob/master/swift/common/middleware/recon.py#L192-L194\n\nso this is at least asserting the change in common.mw.recon to include last","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":846,"context_line":"                             \u0027object_updater_last\u0027],"},{"line_number":847,"context_line":"                            self._full_recon_path(\u0027object\u0027)), {})])"},{"line_number":848,"context_line":"        self.assertEqual(rv, {\"object_updater_sweep\": 0.79848217964172363,"},{"line_number":849,"context_line":"                              \"object_updater_last\": 1357969645.25})"},{"line_number":850,"context_line":""},{"line_number":851,"context_line":"    def test_get_updater_info_unrecognized(self):"},{"line_number":852,"context_line":"        rv \u003d self.app.get_updater_info(\u0027unrecognized_recon_type\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"d5d0bbf2_dc1ace80","line":849,"updated":"2024-12-20 16:46:13.000000000","message":"I don\u0027t think I understand exactly how `self.fakecache` interacts with `self.app.get_updater_info` ... but this seems like a really thin test i.e.\n\nassert the return value \u003d\u003d the mocked return value (??)","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"}],"test/unit/obj/test_updater.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":732,"context_line":"        }"},{"line_number":733,"context_line":"        assert_and_reset_recon_dump(exp_recon_dump)"},{"line_number":734,"context_line":""},{"line_number":735,"context_line":"        ts \u003d next(self.ts_iter)"},{"line_number":736,"context_line":"        ohash \u003d hash_path(\u0027a\u0027, \u0027c\u0027, \u0027o\u0027)"},{"line_number":737,"context_line":"        odir \u003d os.path.join(async_dir, ohash[-3:])"},{"line_number":738,"context_line":"        mkdirs(odir)"}],"source_content_type":"text/x-python","patch_set":5,"id":"2a7712cd_11b9ccdf","side":"PARENT","line":735,"updated":"2024-12-20 16:46:13.000000000","message":"not sure I understand the reasoning of pulling this up higher","commit_id":"fe7928ea8aa45b15fde3154cd8ccdfabc43053a0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":717,"context_line":"            \u0027concurrency\u0027: \u00271\u0027,"},{"line_number":718,"context_line":"            \u0027node_timeout\u0027: \u002715\u0027}, logger\u003dself.logger)"},{"line_number":719,"context_line":"        ts \u003d next(self.ts_iter)"},{"line_number":720,"context_line":"        now \u003d float(next(self.ts_iter))"},{"line_number":721,"context_line":"        with mock.patch(\u0027swift.obj.updater.time.time\u0027, return_value\u003dnow):"},{"line_number":722,"context_line":"            with mock.patch.object(ou, \u0027object_update\u0027,"},{"line_number":723,"context_line":"                                   return_value\u003d(False, \u0027node-id\u0027, None)):"}],"source_content_type":"text/x-python","patch_set":5,"id":"239ffe1e_0522a682","line":720,"updated":"2024-12-20 16:46:13.000000000","message":"oic, `ts_iter` returns a Timestamp so we have to convert to a float.\n\nmaybe better as:\n\n`now \u003d time.time()`","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":720,"context_line":"        now \u003d float(next(self.ts_iter))"},{"line_number":721,"context_line":"        with mock.patch(\u0027swift.obj.updater.time.time\u0027, return_value\u003dnow):"},{"line_number":722,"context_line":"            with mock.patch.object(ou, \u0027object_update\u0027,"},{"line_number":723,"context_line":"                                   return_value\u003d(False, \u0027node-id\u0027, None)):"},{"line_number":724,"context_line":"                ou.run_once()"},{"line_number":725,"context_line":"        exp_recon_dump \u003d {"},{"line_number":726,"context_line":"            \u0027object_updater_stats\u0027: {"}],"source_content_type":"text/x-python","patch_set":5,"id":"520bd430_8efa513d","line":723,"updated":"2024-12-20 16:46:13.000000000","message":"I think `with ctx(), ctx():` is the preferred way to spell multi-with statements:\n\nhttps://docs.python.org/3/reference/compound_stmts.html#the-with-statement","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"89307a59f17f9aede5bcc0fd70ba9d9aa81ac4f9","unresolved":true,"context_lines":[{"line_number":749,"context_line":"                        async_pending)"},{"line_number":750,"context_line":"        with mock.patch(\u0027swift.obj.updater.time.time\u0027, return_value\u003dnow):"},{"line_number":751,"context_line":"            with mock.patch.object(ou, \u0027object_update\u0027,"},{"line_number":752,"context_line":"                                   return_value\u003d(False, \u0027node-id\u0027, None)):"},{"line_number":753,"context_line":"                ou.run_once()"},{"line_number":754,"context_line":"        exp_recon_dump \u003d {"},{"line_number":755,"context_line":"            \u0027object_updater_stats\u0027: {"}],"source_content_type":"text/x-python","patch_set":5,"id":"898a8915_73af0437","line":752,"updated":"2024-12-20 16:46:13.000000000","message":"oh, maybe this code was written before python had multi-with","commit_id":"dc10a47f8f405c67161040ffdf61c9b0e4195de3"}]}