)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-27 09:11:32 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"replicator-tests, add listdir_on_corrupt_data"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9f6466ed_ce06696e","line":7,"updated":"2022-06-27 19:16:13.000000000","message":"I think we need to update the commit message\n\nMost specifically I think we want this change to highlight the fix in the replicator to be more robust in the face of ENODATA when doing clean-up (so that we can drain nodes with failing disks)","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-27 09:11:32 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"replicator-tests, add listdir_on_corrupt_data"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9b7c3c08_8d195e2c","line":7,"in_reply_to":"9f6466ed_ce06696e","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0b83e935d061965f6d1c049e9c64da098be2a677","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-27 18:40:18 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix replicator,auditor for ENODATA, related tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"046f7457_088facd2","line":7,"updated":"2022-06-30 07:28:10.000000000","message":"This commit message, oh as Clay mentioned needs to be updated. It isn\u0027t clear on what it does and what it effects.\nE,g, oh this patch effects all the replicators and auditors? nope just the object ones. Maybe we should change it to something like (this of off the top of my head without looking at the code too much yet):\n\n Make the object replicator and auditor more robust to ENODATA\n \n Currently when the replicator or auditor hits an ENODATA corruption we bail.\n This patch will allow them to skip and continue in an effort to make progress on the work that needs to be done.","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-27 18:40:18 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix replicator,auditor for ENODATA, related tests"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"ba763f47_c39d1570","line":7,"in_reply_to":"046f7457_088facd2","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2f1385080a1e196cb45423455538f1f606dfb7bf","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-30 08:55:38 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make the object replicator and auditor more robust to ENODATA"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7e5a55c7_544a7daa","line":7,"updated":"2022-07-01 01:53:20.000000000","message":"What about the rest of the commit message? how are you making it more robust? Doesn\u0027t have to be super detailed but unless it\u0027s a very obvious from the first line, we provide more details.\nUsually we want to cover the \"what\u0027s the issue\", and \"how it\u0027s been solved\". See: https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure\n\nIt\u0027s why I also suggested a body of the commit message in my previous comment:\n\n  Currently when the replicator or auditor hits an ENODATA corruption we bail.\n  This patch will allow them to skip and continue in an effort to make progress on the work that needs to be done.","commit_id":"5dfd53d4480dcd5a56888ca596413cc35f5b7cad"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     indianwhocodes \u003cnairashwin952013@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-06-30 08:55:38 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make the object replicator and auditor more robust to ENODATA"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"f5df0a68_a4d7ee0a","line":7,"in_reply_to":"7e5a55c7_544a7daa","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"5dfd53d4480dcd5a56888ca596413cc35f5b7cad"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"56a38bfa82ac7af11e7460d13db6a46e83e6e24d","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Make the object replicator and auditor more robust to ENODATA"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently when the replicator or auditor hits an ENODATA corruption we bail."},{"line_number":10,"context_line":"This patch will allow them to skip and continue in an effort to make progress"},{"line_number":11,"context_line":"on the work that needs to be done."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Id4c6e31332a93eb64a7660cec2a42ef686a84d22"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"dd7eb351_c656bab0","line":10,"updated":"2022-07-05 20:32:03.000000000","message":"recommended word wrap on commit messages is 72 chars","commit_id":"1738338c7a69fea1f4d397b1ed59bcd03a4c9c47"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28ec110c4dfbd6a744d6806c36e5465c5d684afe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6791ef89_9b389f1e","updated":"2022-06-08 00:17:28.000000000","message":"More tests are good!","commit_id":"b0ffaff34fa644c4256eba19e0dcc4ee32f2c711"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"33a61e05_c5fa2077","updated":"2022-06-13 22:05:15.000000000","message":"these tests are helping me get a picture of where we need to be thinking about ENODATA\n\nAFAIK, currently the only place we handle it \"correctly\" is hash_suffix_dir https://review.opendev.org/c/openstack/swift/+/832738\n\nI think we can \"ingore\" ENODATA in *most* places the same way we ignore ENOTDIR - but we\u0027re going to need at least ONE place where we actually quarantine it.  I\u0027m not sure if that one place should be in suffix rehashing, it seems like maybe the db/object audit location generators might be a reasonable target.  Can you add an ENODATA test for the object audit location generator?\n","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"21b9aee0_ec00dcbd","updated":"2022-06-24 15:37:59.000000000","message":"I think this test does a good job in some places, and needs more work in others.\n\nI like where you\u0027re going with the audit_loc_gen - let\u0027s get those changes tested really well.\n\nI like the new replicator test coverage for delete_partition - let\u0027s make sure we handle ENODATA when trying to drain a node.\n\nI tried to make some suggestions that might be helpful in a follow up:\n\nhttps://review.opendev.org/c/openstack/swift/+/847589","commit_id":"e45020f6cbb2751b4ae5fd286b75494d804064a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f43b9510_27442a2c","updated":"2022-06-27 19:16:13.000000000","message":"thanks for bring in some of those changes from the follow-up, but there\u0027s some more cleanup to do on this change before we can merge it.","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0b83e935d061965f6d1c049e9c64da098be2a677","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2dcf510a_1b222027","updated":"2022-06-30 07:28:10.000000000","message":"Great work Ash!\n\nThe commit message needs some work. I now it may sound like a NIT. But the repo will be our source of truth so future us would love to be sure they are more discriptive.\n\nWhile your rewording it and pushing another patchset, maybe fix the commit NIT that Tim commented about.","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3af954a559754a469599f0f782f383715b5ee455","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"71f911d7_d18531f6","updated":"2022-06-29 20:11:21.000000000","message":"I think this is looking pretty good!","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"beb8796912a446bb32ef8248fd79f26cc6da414d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cf958677_1f3cd270","updated":"2022-06-29 23:28:37.000000000","message":"LGTM -- Clay, is there something else you\u0027re looking for before merging? Just looking to try it out in prod for a bit?","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2f1385080a1e196cb45423455538f1f606dfb7bf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ba814ebc_4a81e3de","updated":"2022-07-01 01:53:20.000000000","message":"Sorry for being a commit message stickler. But landing it upstream means there is a bit of a high bar. Other then the commit message I think we\u0027re ready to land this!","commit_id":"5dfd53d4480dcd5a56888ca596413cc35f5b7cad"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"56a38bfa82ac7af11e7460d13db6a46e83e6e24d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"6d10fe89_026f7386","updated":"2022-07-05 20:32:03.000000000","message":"i think gerrit has the ability to mark some of these comment threds as \"resolved\" so it\u0027s easier to keep track and tell when all the issues that came up in review have been resolved.","commit_id":"1738338c7a69fea1f4d397b1ed59bcd03a4c9c47"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"927bae133580b3ef6d31591a2108b4ecfa75cdcc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"4b66f79f_09b56e00","updated":"2022-07-07 17:21:50.000000000","message":"well done","commit_id":"d7c08d8ea75ebeb1a9614d13e582b53ccef34ef7"}],"swift/obj/diskfile.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":true,"context_lines":[{"line_number":582,"context_line":"            try:"},{"line_number":583,"context_line":"                suffixes \u003d listdir(part_path)"},{"line_number":584,"context_line":"            except OSError as e:"},{"line_number":585,"context_line":"                if e.errno !\u003d errno.ENOTDIR and e.errno !\u003d errno.ENODATA:"},{"line_number":586,"context_line":"                    raise"},{"line_number":587,"context_line":"                continue"},{"line_number":588,"context_line":"            for asuffix in suffixes:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fbac371a_629ddef4","line":585,"updated":"2022-06-24 15:37:59.000000000","message":"FWIW we *do* cleanup a partdir that raises ENOTDIR \n\nhttps://github.com/openstack/swift/blob/master/swift/obj/replicator.py#L988-L990\n\n... but I can\u0027t find any such handling of when a suff_path raises ENOTDIR\n\n\tobject-6020: default_perms_for_dir: sys_acl_get_file(35a, ACL_TYPE_DEFAULT): Permission denied, falling back on umask\n\tobject-6020: rsync: [generator] recv_generator: mkdir \"/sdb3/objects/40/35a/a161102fba1710ef912af194b8d4635a\" (in object_sdb3) failed: Not a directory (20)\n\tobject-6020: *** Skipping any contents from this failed directory ***\n\tobject-6020: rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1333) [sender\u003d3.2.3]\n\tobject-6020: Bad rsync return code: 23 \u003c- [\u0027rsync\u0027, \u0027--recursive\u0027, \u0027--whole-file\u0027, \u0027--human-readable\u0027, \u0027--xattrs\u0027, \u0027--itemize-changes\u0027, \u0027--ignore-existing\u0027, \u0027--timeout\u003d30\u0027, \u0027--contimeout\u003d30\u0027, \u0027--bwlimit\u003d0\u0027, \u0027--exclude\u003d.*.[0-9a-zA-Z][0-9a-zA-Z][0-9a-zA-Z][0-9a-zA-Z][0-9a-zA-Z][0-9a-zA-Z]\u0027, \u0027/srv/node2/sdb2/objects/40/35a\u0027, \u0027127.0.0.3::object_sdb3/sdb3/objects/40\u0027]\n\n^ not great!","commit_id":"e45020f6cbb2751b4ae5fd286b75494d804064a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":true,"context_lines":[{"line_number":590,"context_line":"                try:"},{"line_number":591,"context_line":"                    hashes \u003d listdir(suff_path)"},{"line_number":592,"context_line":"                except OSError as e:"},{"line_number":593,"context_line":"                    if e.errno !\u003d errno.ENOTDIR and e.errno !\u003d errno.ENODATA:"},{"line_number":594,"context_line":"                        raise"},{"line_number":595,"context_line":"                    continue"},{"line_number":596,"context_line":"                for hsh in hashes:"}],"source_content_type":"text/x-python","patch_set":7,"id":"a18efddb_39599fb8","line":593,"updated":"2022-06-24 15:37:59.000000000","message":"this change makes sense, if the suffix dir raises ENODATA skip it.  Our auditor will get a bunch further.\n\nBesides, we can\u0027t move ENODATA dirs out of the way, and we\u0027re probably NOT going to quarantine the whole partition... so we skip it and hopefully we can drain the rest of the suffixes.\n\nNormally we spell this kind of \"raise on any error except \u003cwhitelist\u003e\" as:\n\n  -                    if e.errno !\u003d errno.ENOTDIR and e.errno !\u003d errno.ENODATA:\n  +                    if e.errno not in (errno.ENOTDIR, errno.ENODATA):","commit_id":"e45020f6cbb2751b4ae5fd286b75494d804064a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":true,"context_lines":[{"line_number":582,"context_line":"            try:"},{"line_number":583,"context_line":"                suffixes \u003d listdir(part_path)"},{"line_number":584,"context_line":"            except OSError as e:"},{"line_number":585,"context_line":"                if e.errno not in (errno.ENOTDIR, errno.ENODATA):"},{"line_number":586,"context_line":"                    raise"},{"line_number":587,"context_line":"                continue"},{"line_number":588,"context_line":"            for asuffix in suffixes:"}],"source_content_type":"text/x-python","patch_set":8,"id":"afeb9ca8_b43c3bf6","line":585,"updated":"2022-06-27 19:16:13.000000000","message":"are all four of these error conditions hit with the new test?\n\nI\u0027m worried this block never tests ENODATA, but we definately want the auditor to continue onto next part.","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":582,"context_line":"            try:"},{"line_number":583,"context_line":"                suffixes \u003d listdir(part_path)"},{"line_number":584,"context_line":"            except OSError as e:"},{"line_number":585,"context_line":"                if e.errno not in (errno.ENOTDIR, errno.ENODATA):"},{"line_number":586,"context_line":"                    raise"},{"line_number":587,"context_line":"                continue"},{"line_number":588,"context_line":"            for asuffix in suffixes:"}],"source_content_type":"text/x-python","patch_set":8,"id":"b66f73e6_d778ce78","line":585,"in_reply_to":"afeb9ca8_b43c3bf6","updated":"2023-08-17 16:16:07.000000000","message":"I think so since the trailing characters for the suffix and hash dir is the same","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"}],"swift/obj/replicator.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"beb8796912a446bb32ef8248fd79f26cc6da414d","unresolved":true,"context_lines":[{"line_number":608,"context_line":"        try:"},{"line_number":609,"context_line":"            tpool.execute(shutil.rmtree, path)"},{"line_number":610,"context_line":"        except OSError as e:"},{"line_number":611,"context_line":"            if e.errno not in (errno.ENOENT, errno.ENOTEMPTY, errno.ENODATA):"},{"line_number":612,"context_line":"                # If there was a race to create or delete, don\u0027t worry"},{"line_number":613,"context_line":"                raise"},{"line_number":614,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3201e382_17284bc7","line":611,"updated":"2022-06-29 23:28:37.000000000","message":"Huh -- sure enough, seen in prod. I\u0027m kinda surprised; would\u0027ve expected the ENODATA so come up during syncing. Maybe the suffix hash matched, so nothing needed to sync?","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"beb8796912a446bb32ef8248fd79f26cc6da414d","unresolved":true,"context_lines":[{"line_number":609,"context_line":"            tpool.execute(shutil.rmtree, path)"},{"line_number":610,"context_line":"        except OSError as e:"},{"line_number":611,"context_line":"            if e.errno not in (errno.ENOENT, errno.ENOTEMPTY, errno.ENODATA):"},{"line_number":612,"context_line":"                # If there was a race to create or delete, don\u0027t worry"},{"line_number":613,"context_line":"                raise"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"    def delete_handoff_objs(self, job, delete_objs):"}],"source_content_type":"text/x-python","patch_set":9,"id":"842195ad_9f2f46d5","line":612,"updated":"2022-06-29 23:28:37.000000000","message":"nit: Comment could use an update, since ENODATA has to do with corruption, not races.","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":609,"context_line":"            tpool.execute(shutil.rmtree, path)"},{"line_number":610,"context_line":"        except OSError as e:"},{"line_number":611,"context_line":"            if e.errno not in (errno.ENOENT, errno.ENOTEMPTY, errno.ENODATA):"},{"line_number":612,"context_line":"                # If there was a race to create or delete, don\u0027t worry"},{"line_number":613,"context_line":"                raise"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"    def delete_handoff_objs(self, job, delete_objs):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ef18e8f_f0b1355b","line":612,"in_reply_to":"057a6ed2_65cc5257","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0b83e935d061965f6d1c049e9c64da098be2a677","unresolved":true,"context_lines":[{"line_number":609,"context_line":"            tpool.execute(shutil.rmtree, path)"},{"line_number":610,"context_line":"        except OSError as e:"},{"line_number":611,"context_line":"            if e.errno not in (errno.ENOENT, errno.ENOTEMPTY, errno.ENODATA):"},{"line_number":612,"context_line":"                # If there was a race to create or delete, don\u0027t worry"},{"line_number":613,"context_line":"                raise"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"    def delete_handoff_objs(self, job, delete_objs):"}],"source_content_type":"text/x-python","patch_set":9,"id":"057a6ed2_65cc5257","line":612,"in_reply_to":"842195ad_9f2f46d5","updated":"2022-06-30 07:28:10.000000000","message":"+1","commit_id":"109583210bc9bf7cc143820825e197a09673a90f"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":6528,"context_line":"            self.assertRaises(OSError, audit)"},{"line_number":6529,"context_line":"        with patch(\u0027swift.common.utils.listdir\u0027, _mock_utils_listdir_on_corrupt_data):"},{"line_number":6530,"context_line":"            audit_again \u003d lambda: list(utils.audit_location_generator("},{"line_number":6531,"context_line":"                tmpdir, \"data\", mount_check\u003dFalse))"},{"line_number":6532,"context_line":"            self.assertRaises(OSError, audit_again)"},{"line_number":6533,"context_line":"        rmtree(tmpdir)"},{"line_number":6534,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"b6298f1d_f4beb410","line":6531,"updated":"2022-06-13 22:05:15.000000000","message":"FWIW the object auditor has it\u0027s own audit_location_generator https://github.com/openstack/swift/blob/master/swift/obj/diskfile.py#L534\n\nbut it might also not handle ENODATA","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":6575,"context_line":"        with patch(\u0027swift.common.utils.listdir\u0027, _mock_utils_listdir_on_corrupt_data):"},{"line_number":6576,"context_line":"            audit_again \u003d lambda: list(utils.audit_location_generator("},{"line_number":6577,"context_line":"                tmpdir, \"data\", mount_check\u003dFalse))"},{"line_number":6578,"context_line":"            self.assertRaises(OSError, audit_again)"},{"line_number":6579,"context_line":"        rmtree(tmpdir)"},{"line_number":6580,"context_line":""},{"line_number":6581,"context_line":"    def test_non_dir_drive(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"91128f4c_a4c3c7e0","line":6578,"updated":"2022-06-13 22:05:15.000000000","message":"I\u0027m not sure this adds any new coverage - it\u0027s basically just re-testing that we\u0027re only handling the ENOTDIR error in the account/container audit location generate\n\nThe object audit location generator ALSO handles ENOTDIR by skipping, which is kind of sketchy, but maybe better than blowing up...","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"}],"test/unit/obj/test_auditor.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":true,"context_lines":[{"line_number":466,"context_line":"                               \u0027get_diskfile_from_audit_location\u0027, blowup):"},{"line_number":467,"context_line":"            self.assertRaises(NameError, auditor_worker.object_audit,"},{"line_number":468,"context_line":"                              AuditLocation(os.path.dirname(path), \u0027sda\u0027, \u00270\u0027,"},{"line_number":469,"context_line":"                                            policy\u003dPOLICIES.legacy))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def test_object_audit_will_not_swallow_enodata_in_tests(self):"},{"line_number":472,"context_line":"        timestamp \u003d str(normalize_timestamp(time.time()))"}],"source_content_type":"text/x-python","patch_set":7,"id":"b009abf9_fef5bb3e","line":469,"updated":"2022-06-24 15:37:59.000000000","message":"I had to go back to the review to understand what this test was doing\n\nhttps://review.opendev.org/c/openstack/swift/+/46191\n\n... it really goes hand and hand with \u0027test_*failsafe*_object_audit_*will*_swallow_errors\u0027","commit_id":"e45020f6cbb2751b4ae5fd286b75494d804064a1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":true,"context_lines":[{"line_number":483,"context_line":"                               \u0027get_diskfile_from_audit_location\u0027, blowup):"},{"line_number":484,"context_line":"            self.assertRaises(OSError, auditor_worker.object_audit,"},{"line_number":485,"context_line":"                              AuditLocation(os.path.dirname(path), \u0027sda\u0027, \u00270\u0027,"},{"line_number":486,"context_line":"                                            policy\u003dPOLICIES.legacy))"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    def test_failsafe_object_audit_will_swallow_errors_in_tests(self):"},{"line_number":489,"context_line":"        timestamp \u003d str(normalize_timestamp(time.time()))"}],"source_content_type":"text/x-python","patch_set":7,"id":"bcb57d17_476b3671","line":486,"updated":"2022-06-24 15:37:59.000000000","message":"I\u0027m not sure what this new test is trying to demonstrate - i would suggest we remove it.\n\nIt\u0027s exercising `object_audit` - which should NOT raise ENODATA [1] - but the way you\u0027ve mocked it\u0027s dependencies we\u0027re asserting that it *does*\n\nI\u0027m not sure how `get_diskfile_from_audit_location` behaves when the hashdir raises ENODATA - I would think we\u0027d *want* it to raise a QuarantineError (which `object_audit` should catch)","commit_id":"e45020f6cbb2751b4ae5fd286b75494d804064a1"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import errno"},{"line_number":17,"context_line":"import unittest"},{"line_number":18,"context_line":"import json"},{"line_number":19,"context_line":"import mock"}],"source_content_type":"text/x-python","patch_set":8,"id":"2a660cb3_fca47698","line":16,"in_reply_to":"1b220931_b3c59218","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":true,"context_lines":[{"line_number":13,"context_line":"# See the License for the specific language governing permissions and"},{"line_number":14,"context_line":"# limitations under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import errno"},{"line_number":17,"context_line":"import unittest"},{"line_number":18,"context_line":"import json"},{"line_number":19,"context_line":"import mock"}],"source_content_type":"text/x-python","patch_set":8,"id":"1b220931_b3c59218","line":16,"in_reply_to":"4e67ff1c_30ea8f1e","updated":"2022-06-27 19:16:13.000000000","message":"\u003e pep8: F401 \u0027errno\u0027 imported but unused\n\nPlease fix.","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":true,"context_lines":[{"line_number":8609,"context_line":"                self.assertEqual(expected, list_locations(tmpdir, \u0027objects\u0027))"},{"line_number":8610,"context_line":"            diskfile.clear_auditor_status(tmpdir, \u0027objects\u0027)"},{"line_number":8611,"context_line":"            with mock.patch(\u0027os.listdir\u0027, splode_if_endswith("},{"line_number":8612,"context_line":"                    \"afd\", errno.ENODATA)):"},{"line_number":8613,"context_line":"                self.assertEqual(expected, list_locations(tmpdir, \u0027objects\u0027))"},{"line_number":8614,"context_line":"            diskfile.clear_auditor_status(tmpdir, \u0027objects\u0027)"},{"line_number":8615,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1724ddf4_86367f05","line":8612,"updated":"2022-06-27 19:16:13.000000000","message":"i feel like we\u0027d get better coverage if this was ENOTDIR and the 2809 part failure was ENODATA","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":8609,"context_line":"                self.assertEqual(expected, list_locations(tmpdir, \u0027objects\u0027))"},{"line_number":8610,"context_line":"            diskfile.clear_auditor_status(tmpdir, \u0027objects\u0027)"},{"line_number":8611,"context_line":"            with mock.patch(\u0027os.listdir\u0027, splode_if_endswith("},{"line_number":8612,"context_line":"                    \"afd\", errno.ENODATA)):"},{"line_number":8613,"context_line":"                self.assertEqual(expected, list_locations(tmpdir, \u0027objects\u0027))"},{"line_number":8614,"context_line":"            diskfile.clear_auditor_status(tmpdir, \u0027objects\u0027)"},{"line_number":8615,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"f9cd4155_13570bc3","line":8612,"in_reply_to":"1724ddf4_86367f05","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"}],"test/unit/obj/test_replicator.py":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"c292ef811629fc8f3290a499fbf82ab2f85998c9","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import errno"},{"line_number":17,"context_line":"import io"},{"line_number":18,"context_line":"import json"},{"line_number":19,"context_line":"import pdb"},{"line_number":20,"context_line":"import unittest"},{"line_number":21,"context_line":"import os"},{"line_number":22,"context_line":"import mock"}],"source_content_type":"text/x-python","patch_set":1,"id":"87115bce_be9514a5","line":19,"in_reply_to":"6fd988e1_8fccea23","updated":"2022-06-07 23:14:47.000000000","message":"\u003e pep8: F401 \u0027pdb\u0027 imported but unused\n\nPlease fix.","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28ec110c4dfbd6a744d6806c36e5465c5d684afe","unresolved":true,"context_lines":[{"line_number":1468,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1469,"context_line":"            osexec \u003d OSError()"},{"line_number":1470,"context_line":"            osexec.errno \u003d errno.ENOTEMPTY"},{"line_number":1471,"context_line":"            mockrmtree.side_effect \u003d osexec"},{"line_number":1472,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1473,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1474,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027], override_partitions\u003d[1], override_policies\u003d[3])"}],"source_content_type":"text/x-python","patch_set":1,"id":"260bd9ca_ad66ecfb","line":1471,"updated":"2022-06-08 00:17:28.000000000","message":"nit: Can roll it up a bit with something like\n\n mockrmtree.side_effect \u003d OSError(errno.ENOTEMPTY, \u0027Directory not empty\u0027)","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":false,"context_lines":[{"line_number":1468,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1469,"context_line":"            osexec \u003d OSError()"},{"line_number":1470,"context_line":"            osexec.errno \u003d errno.ENOTEMPTY"},{"line_number":1471,"context_line":"            mockrmtree.side_effect \u003d osexec"},{"line_number":1472,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1473,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1474,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027], override_partitions\u003d[1], override_policies\u003d[3])"}],"source_content_type":"text/x-python","patch_set":1,"id":"73417ade_7894aa3f","line":1471,"in_reply_to":"260bd9ca_ad66ecfb","updated":"2022-06-24 15:37:59.000000000","message":"Done","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":1468,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1469,"context_line":"            osexec \u003d OSError()"},{"line_number":1470,"context_line":"            osexec.errno \u003d errno.ENOTEMPTY"},{"line_number":1471,"context_line":"            mockrmtree.side_effect \u003d osexec"},{"line_number":1472,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1473,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1474,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027], override_partitions\u003d[1], override_policies\u003d[3])"}],"source_content_type":"text/x-python","patch_set":1,"id":"cade4720_82d7f6c5","line":1471,"in_reply_to":"260bd9ca_ad66ecfb","updated":"2023-08-17 16:16:07.000000000","message":"Done","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28ec110c4dfbd6a744d6806c36e5465c5d684afe","unresolved":true,"context_lines":[{"line_number":1474,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027], override_partitions\u003d[1], override_policies\u003d[3])"},{"line_number":1475,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1476,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1477,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1478,"context_line":""},{"line_number":1479,"context_line":"    def test_delete_partition_override_params_os_no_entity_error(self):"},{"line_number":1480,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"}],"source_content_type":"text/x-python","patch_set":1,"id":"55262fc7_f860d3c8","line":1477,"updated":"2022-06-08 00:17:28.000000000","message":"We should probably have an assertion that our mock got called. This assertion is just saying that the real *wasn\u0027t* called, yeah?","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":1474,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027], override_partitions\u003d[1], override_policies\u003d[3])"},{"line_number":1475,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1476,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1477,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1478,"context_line":""},{"line_number":1479,"context_line":"    def test_delete_partition_override_params_os_no_entity_error(self):"},{"line_number":1480,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"}],"source_content_type":"text/x-python","patch_set":1,"id":"30188947_9cce9d05","line":1477,"in_reply_to":"55262fc7_f860d3c8","updated":"2023-08-17 16:16:07.000000000","message":"Done","commit_id":"005bef2332f2011fea3882cc97156f7410215afe"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28ec110c4dfbd6a744d6806c36e5465c5d684afe","unresolved":true,"context_lines":[{"line_number":1472,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1473,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027],"},{"line_number":1474,"context_line":"                                      override_partitions\u003d[1],"},{"line_number":1475,"context_line":"                                      override_policies\u003d[3])"},{"line_number":1476,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1477,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1478,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"}],"source_content_type":"text/x-python","patch_set":3,"id":"905cce49_de7be717","line":1475,"range":{"start_line":1475,"start_character":57,"end_line":1475,"end_character":58},"updated":"2022-06-08 00:17:28.000000000","message":"Wait -- we patch policies to have 0 and 1 -- where\u0027d 3 come from?","commit_id":"b0ffaff34fa644c4256eba19e0dcc4ee32f2c711"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":1472,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1473,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027],"},{"line_number":1474,"context_line":"                                      override_partitions\u003d[1],"},{"line_number":1475,"context_line":"                                      override_policies\u003d[3])"},{"line_number":1476,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1477,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1478,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"}],"source_content_type":"text/x-python","patch_set":3,"id":"42517960_7ef25a2f","line":1475,"range":{"start_line":1475,"start_character":57,"end_line":1475,"end_character":58},"in_reply_to":"905cce49_de7be717","updated":"2023-08-17 16:16:07.000000000","message":"Retrieved it from here: https://github.com/openstack/swift/blob/master/test/unit/obj/test_replicator.py#L2554 ,looking it a bit more closely, made me realize that the aforementioned unit-test was just a means to validate arguments for override policies for replication. Will incorporate your feedback.","commit_id":"b0ffaff34fa644c4256eba19e0dcc4ee32f2c711"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"28ec110c4dfbd6a744d6806c36e5465c5d684afe","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1487,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sdb\u0027],"},{"line_number":1488,"context_line":"                                      override_partitions\u003d[9],"},{"line_number":1489,"context_line":"                                      override_policies\u003d[3])"},{"line_number":1490,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1491,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1492,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"}],"source_content_type":"text/x-python","patch_set":3,"id":"ebf30d7f_d5ac31cc","line":1489,"updated":"2022-06-08 00:17:28.000000000","message":"Similar to above -- I\u0027m not sure where any of these overrides are coming from.","commit_id":"b0ffaff34fa644c4256eba19e0dcc4ee32f2c711"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":1486,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1487,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sdb\u0027],"},{"line_number":1488,"context_line":"                                      override_partitions\u003d[9],"},{"line_number":1489,"context_line":"                                      override_policies\u003d[3])"},{"line_number":1490,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1491,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1492,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"}],"source_content_type":"text/x-python","patch_set":3,"id":"ad4d2efc_b4daf957","line":1489,"in_reply_to":"ebf30d7f_d5ac31cc","updated":"2023-08-17 16:16:07.000000000","message":"Done","commit_id":"b0ffaff34fa644c4256eba19e0dcc4ee32f2c711"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def test_delete_partition_override_params_os_not_empty_error(self):"},{"line_number":1467,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1468,"context_line":"            mockrmtree.side_effect \u003d OSError(errno.ENOTEMPTY, \u0027Directory not empty\u0027)"},{"line_number":1469,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1470,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1471,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"43ab901b_1989f6a7","line":1468,"updated":"2022-06-13 22:05:15.000000000","message":"in delete_partition we already handle ENOENT and ENOTEMPTY, they are \"just\" a race \n\nwe do *not* handle ENOTDIR OR ENODATA - which would both be surprising file system corrutpion errors - but we still want to degrade gracefully.\n\nIt looks like the error handling in update_deleted allows the replicator to continue after an ENODATA.  So I guess the only behaviorial difference between ENOENT and ENODATA is the traceback in the logs.","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":false,"context_lines":[{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"    def test_delete_partition_override_params_os_not_empty_error(self):"},{"line_number":1467,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1468,"context_line":"            mockrmtree.side_effect \u003d OSError(errno.ENOTEMPTY, \u0027Directory not empty\u0027)"},{"line_number":1469,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1470,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1471,"context_line":"            self.replicator.replicate(override_devices\u003d[\u0027sda\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"c104a434_68a76431","line":1468,"in_reply_to":"43ab901b_1989f6a7","updated":"2022-06-24 15:37:59.000000000","message":"Done","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":1477,"context_line":"            mockrmtree.assert_called_once()"},{"line_number":1478,"context_line":""},{"line_number":1479,"context_line":"    def test_delete_partition_override_params_os_no_entity_error(self):"},{"line_number":1480,"context_line":"        with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:"},{"line_number":1481,"context_line":"            mockrmtree.side_effect \u003d OSError(errno.ENOENT, \u0027Entity not found\u0027)"},{"line_number":1482,"context_line":"            part_path \u003d os.path.join(self.objects, \u00271\u0027)"},{"line_number":1483,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"}],"source_content_type":"text/x-python","patch_set":4,"id":"9c002b79_b26919e3","line":1480,"updated":"2022-06-13 22:05:15.000000000","message":"because I\u0027ve seen broad mocking create surprising side-effects with test setup/teardown I normally recommned you make the mock scope as tight as possible:\n\n\tdiff --git a/test/unit/obj/test_replicator.py b/test/unit/obj/test_replicator.py\n\tindex 86239d2aa..3c701cd4d 100644\n\t--- a/test/unit/obj/test_replicator.py\n\t+++ b/test/unit/obj/test_replicator.py\n\t@@ -1477,17 +1477,17 @@ class TestObjectReplicator(unittest.TestCase):\n\t\t     mockrmtree.assert_called_once()\n\t \n\t     def test_delete_partition_override_params_os_no_entity_error(self):\n\t+        part_path \u003d os.path.join(self.objects, \u00271\u0027)\n\t+        self.assertTrue(os.access(part_path, os.F_OK))\n\t\t with mock.patch(\u0027swift.obj.replicator.shutil.rmtree\u0027) as mockrmtree:\n\t\t     mockrmtree.side_effect \u003d OSError(errno.ENOENT, \u0027Entity not found\u0027)\n\t-            part_path \u003d os.path.join(self.objects, \u00271\u0027)\n\t-            self.assertTrue(os.access(part_path, os.F_OK))\n\t\t     self.replicator.replicate(override_devices\u003d[\u0027sda\u0027],\n\t\t\t\t\t       override_partitions\u003d[1],\n\t\t\t\t\t       override_policies\u003d[0])\n\t-            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)\n\t-            self.assertFalse(error_lines)\n\t-            self.assertTrue(os.access(part_path, os.F_OK))\n\t-            mockrmtree.assert_called_once()\n\t+        error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)\n\t+        self.assertFalse(error_lines)\n\t+        self.assertTrue(os.access(part_path, os.F_OK))\n\t+        mockrmtree.assert_called_once()\n\t \n\t     def test_delete_policy_override_params(self):\n\t\t df0 \u003d self.df_mgr.get_diskfile(\u0027sda\u0027, \u002799\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"                                      override_policies\u003d[0])"},{"line_number":1487,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1488,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1489,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1490,"context_line":"            mockrmtree.assert_called_once()"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"    def test_delete_policy_override_params(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"c0a4f962_f6a9058b","line":1489,"updated":"2022-06-13 22:05:15.000000000","message":"I think this is more clear as \"assertTrue(os.path.exists(part_path))\"","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"7d42fa182eb1cfc4b901fd67866da2e24901cb84","unresolved":false,"context_lines":[{"line_number":1486,"context_line":"                                      override_policies\u003d[0])"},{"line_number":1487,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1488,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1489,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1490,"context_line":"            mockrmtree.assert_called_once()"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"    def test_delete_policy_override_params(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ad71297a_a9938f98","line":1489,"in_reply_to":"c0a4f962_f6a9058b","updated":"2022-06-24 15:37:59.000000000","message":"Done","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"99fdd701156090015bff40aecd36d75a4a2c4341","unresolved":true,"context_lines":[{"line_number":1487,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1488,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1489,"context_line":"            self.assertTrue(os.access(part_path, os.F_OK))"},{"line_number":1490,"context_line":"            mockrmtree.assert_called_once()"},{"line_number":1491,"context_line":""},{"line_number":1492,"context_line":"    def test_delete_policy_override_params(self):"},{"line_number":1493,"context_line":"        df0 \u003d self.df_mgr.get_diskfile(\u0027sda\u0027, \u002799\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"271fdbf7_8b9c66ce","line":1490,"updated":"2022-06-13 22:05:15.000000000","message":"I think we can make a stronger assertion than \"called_once\"\n\n\t@@ -1487,7 +1490,10 @@ class TestObjectReplicator(unittest.TestCase):\n\t\t     error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)\n\t\t     self.assertFalse(error_lines)\n\t\t     self.assertTrue(os.access(part_path, os.F_OK))\n\t-            mockrmtree.assert_called_once()\n\t+        self.assertEqual([\n\t+            mock.call(os.path.join(self.testdir, \u0027node/sda/objects/1\u0027)),\n\t+       ], mockrmtree.call_args_list)\n\t+","commit_id":"75db5db832dfaf329daa7d3d8b96752a7e109b87"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa628a00696ea70aab6df500f633976a387a1317","unresolved":true,"context_lines":[{"line_number":1498,"context_line":"                                      override_partitions\u003d[1],"},{"line_number":1499,"context_line":"                                      override_policies\u003d[0])"},{"line_number":1500,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1501,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1502,"context_line":"            self.assertTrue(os.path.exists(part_path))"},{"line_number":1503,"context_line":"            self.assertEqual([mock.call(part_path)], mockrmtree.call_args_list)"},{"line_number":1504,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"cd746bb2_107b7299","line":1501,"updated":"2022-06-27 19:16:13.000000000","message":"this test does not pass for me","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"2aae922b54495a82031c1854e28805f875db25ed","unresolved":false,"context_lines":[{"line_number":1498,"context_line":"                                      override_partitions\u003d[1],"},{"line_number":1499,"context_line":"                                      override_policies\u003d[0])"},{"line_number":1500,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1501,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1502,"context_line":"            self.assertTrue(os.path.exists(part_path))"},{"line_number":1503,"context_line":"            self.assertEqual([mock.call(part_path)], mockrmtree.call_args_list)"},{"line_number":1504,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"996e181a_e359782b","line":1501,"in_reply_to":"cd746bb2_107b7299","updated":"2023-08-17 16:16:07.000000000","message":"Ack","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"beb8796912a446bb32ef8248fd79f26cc6da414d","unresolved":true,"context_lines":[{"line_number":1498,"context_line":"                                      override_partitions\u003d[1],"},{"line_number":1499,"context_line":"                                      override_policies\u003d[0])"},{"line_number":1500,"context_line":"            error_lines \u003d self.replicator.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1501,"context_line":"            self.assertFalse(error_lines)"},{"line_number":1502,"context_line":"            self.assertTrue(os.path.exists(part_path))"},{"line_number":1503,"context_line":"            self.assertEqual([mock.call(part_path)], mockrmtree.call_args_list)"},{"line_number":1504,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"4f2a4bfb_0644f4ea","line":1501,"in_reply_to":"cd746bb2_107b7299","updated":"2022-06-29 23:28:37.000000000","message":"Passes now, and this is where things bomb out without the fix. Note that the other two new tests here pass on master.","commit_id":"9c41f96a4654b8580dcfea2febca37c3c4792e5c"}]}