)]}'
{"swift/obj/diskfile.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5c63452f41f0324e2838a509eda92be02a046360","unresolved":true,"context_lines":[{"line_number":403,"context_line":"    invalidations_file \u003d join(partition_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    if not os.path.exists(partition_dir):"},{"line_number":406,"context_line":"        return {\u0027valid\u0027: False}"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    with lock_path(partition_dir):"},{"line_number":409,"context_line":"        hashes \u003d read_hashes(partition_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"655e6e1a_d86c8f05","line":406,"updated":"2021-03-19 20:42:49.000000000","message":"I\u0027m not quite happy with how this bleeds over from read_hashes, but I needed to prevent the lock_path below from creating partition_dir.","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"72beade690e82c1fa5792c395a8534f647364534","unresolved":true,"context_lines":[{"line_number":403,"context_line":"    invalidations_file \u003d join(partition_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    if not os.path.exists(partition_dir):"},{"line_number":406,"context_line":"        return {\u0027valid\u0027: False}"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    with lock_path(partition_dir):"},{"line_number":409,"context_line":"        hashes \u003d read_hashes(partition_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"896057c5_d011834d","line":406,"in_reply_to":"5f414006_bcc77b31","updated":"2021-03-25 05:16:39.000000000","message":"I\u0027d rather avoid the warning getting logged down at L1276, so I guess we\u0027d want to update the error handling there, too. But that reminds me: we\u0027re *already* bleeding this sort of a return out in __get_hashes (L1268). At least here we\u0027re a lot closer to read_hashes.\n\nOn the whole, I think I\u0027m OK with this -- just wanted to flag it up.","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"139fb15f41f315ddb32142a8ab3e802a3255d686","unresolved":true,"context_lines":[{"line_number":403,"context_line":"    invalidations_file \u003d join(partition_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    if not os.path.exists(partition_dir):"},{"line_number":406,"context_line":"        return {\u0027valid\u0027: False}"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    with lock_path(partition_dir):"},{"line_number":409,"context_line":"        hashes \u003d read_hashes(partition_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd3d3c97_86bd2370","line":406,"in_reply_to":"655e6e1a_d86c8f05","updated":"2021-03-25 04:34:12.000000000","message":"Couldn\u0027t we just stop the lock_path from making the directory, something like: http://paste.openstack.org/show/803897/","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"59a8a577b8ec0ee2fa5b7e1bd488b32846b7cb32","unresolved":true,"context_lines":[{"line_number":403,"context_line":"    invalidations_file \u003d join(partition_dir, HASH_INVALIDATIONS_FILE)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    if not os.path.exists(partition_dir):"},{"line_number":406,"context_line":"        return {\u0027valid\u0027: False}"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    with lock_path(partition_dir):"},{"line_number":409,"context_line":"        hashes \u003d read_hashes(partition_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f414006_bcc77b31","line":406,"in_reply_to":"cd3d3c97_86bd2370","updated":"2021-03-25 04:48:49.000000000","message":"raise IOError(errno.ENOENT, \"Lcok directory doesn\u0027t exist\") really? Always include the path into the diagnostics, preferably as \u0027%r\u0027 if possible. You never know why you wish you did. And, well, \"L-cock\".","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f41053982a2dbe40b51b03946876a592bf93ee1","unresolved":true,"context_lines":[{"line_number":1552,"context_line":"        if not dev_path:"},{"line_number":1553,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1554,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1555,"context_line":"        if not os.path.exists(partition_path) and suffixes:"},{"line_number":1556,"context_line":"            mkdirs(partition_path)"},{"line_number":1557,"context_line":"        if skip_rehash:"},{"line_number":1558,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":1,"id":"c66c1403_c1c8a530","line":1555,"updated":"2021-03-25 12:44:42.000000000","message":"why not just return early here?\nwhy wait until we\u0027re all the way down in consolidate_hashes to check for existence of the dir again? Is there something I am missing that happens during the call path down to __get_hashes and consolidate_hashes?","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bda6d4a672bbbc0da78e23029e995fbb20f2104e","unresolved":true,"context_lines":[{"line_number":1552,"context_line":"        if not dev_path:"},{"line_number":1553,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1554,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1555,"context_line":"        if not os.path.exists(partition_path) and suffixes:"},{"line_number":1556,"context_line":"            mkdirs(partition_path)"},{"line_number":1557,"context_line":"        if skip_rehash:"},{"line_number":1558,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5187a930_ded7eb55","line":1555,"in_reply_to":"c66c1403_c1c8a530","updated":"2021-03-25 20:08:20.000000000","message":"Good call -- cleans all this up considerably.","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":1549,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1550,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1551,"context_line":"        if not os.path.exists(partition_path):"},{"line_number":1552,"context_line":"            if not suffixes:"},{"line_number":1553,"context_line":"                return None if skip_rehash else {}"},{"line_number":1554,"context_line":"            mkdirs(partition_path)"},{"line_number":1555,"context_line":"        if skip_rehash:"}],"source_content_type":"text/x-python","patch_set":4,"id":"324b2e4d_23a1dbda","line":1552,"range":{"start_line":1552,"start_character":12,"end_line":1552,"end_character":27},"updated":"2021-03-26 12:24:48.000000000","message":"ok. I checked and there are diskfile unit tests that fail of this condition goes missing (i.e. we do test that specifying suffixes will cause the part dir to be created)","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":1550,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1551,"context_line":"        if not os.path.exists(partition_path):"},{"line_number":1552,"context_line":"            if not suffixes:"},{"line_number":1553,"context_line":"                return None if skip_rehash else {}"},{"line_number":1554,"context_line":"            mkdirs(partition_path)"},{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baf6ba58_2ab87f3f","line":1553,"range":{"start_line":1553,"start_character":31,"end_line":1553,"end_character":42},"updated":"2021-03-26 12:24:48.000000000","message":"ok, no change needed...notes to self...\n\nAFAICT the only time skip_rehash is True is due to skip_rehash \u003d bool(suffixes) in obj REPLICATE, so currently this will never return None, but I guess that is under the control of the caller.\n\nSo skip_rehash -\u003e suffixes, but also I think suffixes -\u003e skip_rehash?\n\nSo, from a wider perspective, is the only time we want to create a non-existent partition when suffixes \u003d\u003d sync_rehash \u003d\u003d True ? With the current method signature we can\u0027t assume that in the body of this method though (other third-party callers may have different requirements), so this is just thinking aloud.","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9777f5973efceefd3c835937455687fc690eb1a4","unresolved":true,"context_lines":[{"line_number":1550,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1551,"context_line":"        if not os.path.exists(partition_path):"},{"line_number":1552,"context_line":"            if not suffixes:"},{"line_number":1553,"context_line":"                return None if skip_rehash else {}"},{"line_number":1554,"context_line":"            mkdirs(partition_path)"},{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":4,"id":"f4f3acb8_1ede4644","line":1553,"range":{"start_line":1553,"start_character":31,"end_line":1553,"end_character":42},"in_reply_to":"baf6ba58_2ab87f3f","updated":"2021-03-26 21:35:38.000000000","message":"Thinking longer term, I can absolutely imagine a reason I\u0027d want to make a call with skip_rehash\u003dTrue, suffixes\u003d[] -- basically as a way to get a feel for how dense a partition is without incurring the disk I/O of a proper rehash. So I suppose {} would be a reasonable return here.\n\nI\u0027m having a much harder time envisioning a use-case for skip_rehash\u003dFalse, suffixes\u003d[...], though.","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":1554,"context_line":"            mkdirs(partition_path)"},{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"},{"line_number":1557,"context_line":"                invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1558,"context_line":"            return None"},{"line_number":1559,"context_line":"        else:"},{"line_number":1560,"context_line":"            _junk, hashes \u003d tpool.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"366b8f48_f9c71a35","line":1557,"range":{"start_line":1557,"start_character":16,"end_line":1557,"end_character":31},"updated":"2021-03-26 12:24:48.000000000","message":"this will call mkdirs(partition_path) in lock_path()","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9777f5973efceefd3c835937455687fc690eb1a4","unresolved":true,"context_lines":[{"line_number":1554,"context_line":"            mkdirs(partition_path)"},{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"},{"line_number":1557,"context_line":"                invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1558,"context_line":"            return None"},{"line_number":1559,"context_line":"        else:"},{"line_number":1560,"context_line":"            _junk, hashes \u003d tpool.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"0a222cdc_52f549e9","line":1557,"range":{"start_line":1557,"start_character":16,"end_line":1557,"end_character":31},"in_reply_to":"366b8f48_f9c71a35","updated":"2021-03-26 21:35:38.000000000","message":"Yeah, I was noticing that recently, too... makes me think the mkdirs above is rather unnecessary.","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"},{"line_number":1557,"context_line":"                invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1558,"context_line":"            return None"},{"line_number":1559,"context_line":"        else:"},{"line_number":1560,"context_line":"            _junk, hashes \u003d tpool.execute("},{"line_number":1561,"context_line":"                self._get_hashes, device, partition, policy,"}],"source_content_type":"text/x-python","patch_set":4,"id":"cc163606_0c5155cd","line":1558,"range":{"start_line":1558,"start_character":19,"end_line":1558,"end_character":23},"updated":"2021-03-26 12:24:48.000000000","message":"a little off topic - I guess we return None here so that a caller can infer that no hashes have been calculated/read. But for the one use case, the caller ignores the result, and callers know they set skip_rehash, so I wonder if would simplify things by returning an empty dict?","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9777f5973efceefd3c835937455687fc690eb1a4","unresolved":true,"context_lines":[{"line_number":1555,"context_line":"        if skip_rehash:"},{"line_number":1556,"context_line":"            for suffix in suffixes or []:"},{"line_number":1557,"context_line":"                invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1558,"context_line":"            return None"},{"line_number":1559,"context_line":"        else:"},{"line_number":1560,"context_line":"            _junk, hashes \u003d tpool.execute("},{"line_number":1561,"context_line":"                self._get_hashes, device, partition, policy,"}],"source_content_type":"text/x-python","patch_set":4,"id":"32850186_fac873b9","line":1558,"range":{"start_line":1558,"start_character":19,"end_line":1558,"end_character":23},"in_reply_to":"cc163606_0c5155cd","updated":"2021-03-26 21:35:38.000000000","message":"I think it was a compromise that shook out from the comments aroun https://review.opendev.org/c/openstack/swift/+/758636/5/swift/obj/diskfile.py\n\nBasically, there is no *good* answer we can respond with, so pick an obviously nonsensical answer. Empty dict *isn\u0027t* nonsensical -- someone might reasonably think that it meant we read the pickle and it was empty (or there was no pickle to read).","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":344,"context_line":"def valid_suffix(value):"},{"line_number":345,"context_line":"    return len(value) \u003d\u003d 3 and all(c in \u00270123456789abcdef\u0027 for c in value)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"def read_hashes(partition_dir):"},{"line_number":349,"context_line":"    \"\"\""},{"line_number":350,"context_line":"    Read the existing hashes.pkl"}],"source_content_type":"text/x-python","patch_set":5,"id":"70208af1_b6233d81","line":347,"updated":"2021-03-29 09:43:09.000000000","message":"but...\n\n  df.valid_suffix((\u0027b\u0027,\u0027a\u0027,\u0027d\u0027))\n  True\n\nmaybe\n\n   int(value, 16) \u003c 4096\n\nwith appropriate exception handling?\n\nAlso, probably worth a unit test given its a new module level helper function","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee0a9171ebb5481919cd47fb3fb9129bc690f8cb","unresolved":true,"context_lines":[{"line_number":344,"context_line":"def valid_suffix(value):"},{"line_number":345,"context_line":"    return len(value) \u003d\u003d 3 and all(c in \u00270123456789abcdef\u0027 for c in value)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"def read_hashes(partition_dir):"},{"line_number":349,"context_line":"    \"\"\""},{"line_number":350,"context_line":"    Read the existing hashes.pkl"}],"source_content_type":"text/x-python","patch_set":5,"id":"c69225ce_91d2c9e2","line":347,"in_reply_to":"70208af1_b6233d81","updated":"2021-03-29 17:26:04.000000000","message":"IDK about using int(value, 16):\n\n \u003e\u003e\u003e int(\u0027 -0\u0027, 16)\n 0\n\nwhich just seems wrong.","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"9fc054ae0f892ce18fab0701cfe5c5612529c912","unresolved":true,"context_lines":[{"line_number":344,"context_line":"def valid_suffix(value):"},{"line_number":345,"context_line":"    return len(value) \u003d\u003d 3 and all(c in \u00270123456789abcdef\u0027 for c in value)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"def read_hashes(partition_dir):"},{"line_number":349,"context_line":"    \"\"\""},{"line_number":350,"context_line":"    Read the existing hashes.pkl"}],"source_content_type":"text/x-python","patch_set":5,"id":"dbb0ee19_504bfecb","line":347,"in_reply_to":"ade42cfe_bc5d7c97","updated":"2021-03-29 18:28:29.000000000","message":"I mean, if the goal is \"it should be watertight\" and we\u0027re worried about callers passing things like (\u0027b\u0027, \u0027a\u0027, \u0027d\u0027) -- I feel like I ought to be painstakingly clear in my input assumptions. I think the current patch that adds an\n\n isinstance(value, str)\n\ncheck is enough and a step in the right direction? *shrug*","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"af104660393bc8afd464c2da713bed26c493359f","unresolved":true,"context_lines":[{"line_number":344,"context_line":"def valid_suffix(value):"},{"line_number":345,"context_line":"    return len(value) \u003d\u003d 3 and all(c in \u00270123456789abcdef\u0027 for c in value)"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"def read_hashes(partition_dir):"},{"line_number":349,"context_line":"    \"\"\""},{"line_number":350,"context_line":"    Read the existing hashes.pkl"}],"source_content_type":"text/x-python","patch_set":5,"id":"ade42cfe_bc5d7c97","line":347,"in_reply_to":"c69225ce_91d2c9e2","updated":"2021-03-29 17:43:43.000000000","message":"I thought the promiscuity of int(,16) was acceptable for hashes. What\u0027s the scenario you\u0027re considering? A bug that produces invalid syntax, and then... extra invalidations? Or is there a possibility of corruption because int() eats it?","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":1554,"context_line":"        if not dev_path:"},{"line_number":1555,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1556,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1557,"context_line":"        suffixes \u003d [suf for suf in suffixes or [] if valid_suffix(suf)]"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":5,"id":"81d276aa_74e0adc4","line":1557,"updated":"2021-03-29 09:43:09.000000000","message":"is it best to silently ignore invalid suffixes or raise a ValueError? certainly a good idea not to create invalid suffixes though\n\nalso, the validation isn\u0027t covered by unit tests","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee0a9171ebb5481919cd47fb3fb9129bc690f8cb","unresolved":true,"context_lines":[{"line_number":1554,"context_line":"        if not dev_path:"},{"line_number":1555,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1556,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1557,"context_line":"        suffixes \u003d [suf for suf in suffixes or [] if valid_suffix(suf)]"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":5,"id":"e398f9ee_9e0b671b","line":1557,"in_reply_to":"81d276aa_74e0adc4","updated":"2021-03-29 17:26:04.000000000","message":"Given how object-server currently just passes through values from the (replication) client, I\u0027m inclined to ignore -- but that\u0027s mostly out of laziness and a disinclination to touch the object/server.py right now.","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f7e86bb16e3591f18da1b1f2c530eab8983cb40","unresolved":false,"context_lines":[{"line_number":1554,"context_line":"        if not dev_path:"},{"line_number":1555,"context_line":"            raise DiskFileDeviceUnavailable()"},{"line_number":1556,"context_line":"        partition_path \u003d get_part_path(dev_path, policy, partition)"},{"line_number":1557,"context_line":"        suffixes \u003d [suf for suf in suffixes or [] if valid_suffix(suf)]"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3732bacf_555629f4","line":1557,"in_reply_to":"e398f9ee_9e0b671b","updated":"2021-03-29 18:38:18.000000000","message":"Ack","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":1557,"context_line":"        suffixes \u003d [suf for suf in suffixes or [] if valid_suffix(suf)]"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"},{"line_number":1561,"context_line":"                self.invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1562,"context_line":"            hashes \u003d None"},{"line_number":1563,"context_line":"        elif not os.path.exists(partition_path):"}],"source_content_type":"text/x-python","patch_set":5,"id":"f9cf9a1d_26cea9ed","line":1560,"range":{"start_line":1560,"start_character":34,"end_line":1560,"end_character":40},"updated":"2021-03-29 09:43:09.000000000","message":"or [] is no longer necessary","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee0a9171ebb5481919cd47fb3fb9129bc690f8cb","unresolved":false,"context_lines":[{"line_number":1557,"context_line":"        suffixes \u003d [suf for suf in suffixes or [] if valid_suffix(suf)]"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"},{"line_number":1561,"context_line":"                self.invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1562,"context_line":"            hashes \u003d None"},{"line_number":1563,"context_line":"        elif not os.path.exists(partition_path):"}],"source_content_type":"text/x-python","patch_set":5,"id":"889214d5_a8634c47","line":1560,"range":{"start_line":1560,"start_character":34,"end_line":1560,"end_character":40},"in_reply_to":"f9cf9a1d_26cea9ed","updated":"2021-03-29 17:26:04.000000000","message":"Done","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a2bdbe4ec9ac48df07dee07f17edf712dc127d30","unresolved":true,"context_lines":[{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":"        if skip_rehash:"},{"line_number":1560,"context_line":"            for suffix in suffixes or []:"},{"line_number":1561,"context_line":"                self.invalidate_hash(os.path.join(partition_path, suffix))"},{"line_number":1562,"context_line":"            hashes \u003d None"},{"line_number":1563,"context_line":"        elif not os.path.exists(partition_path):"},{"line_number":1564,"context_line":"            hashes \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"707b670b_aecfda08","line":1561,"range":{"start_line":1561,"start_character":16,"end_line":1561,"end_character":20},"updated":"2021-03-26 21:38:13.000000000","message":"Pete\u0027s comment on https://review.opendev.org/c/openstack/swift/+/783089/ of\n\n\u003e I\u0027m okay with this but I have a question, why the heck do we need these class methods? Is this for out-of-tree users of DiskFile? It looks to me that the code can just call foo() instead of self.foo().\n\nmade me realize we must\u0027ve done it for lots-of-small-files\u0027s sake, and that we probably need more self.blahblahblah around.","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":1566,"context_line":"            _junk, hashes \u003d tpool.execute("},{"line_number":1567,"context_line":"                self._get_hashes, device, partition, policy,"},{"line_number":1568,"context_line":"                recalculate\u003dsuffixes)"},{"line_number":1569,"context_line":"        return hashes"},{"line_number":1570,"context_line":""},{"line_number":1571,"context_line":"    def _listdir(self, path):"},{"line_number":1572,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"bce9668f_1f977f10","line":1569,"updated":"2021-03-29 09:43:09.000000000","message":"+1 I prefer this now with one return point","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f7e86bb16e3591f18da1b1f2c530eab8983cb40","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"        hashes.update((suffix, None) for suffix in recalculate)"},{"line_number":1309,"context_line":"        for suffix, hash_ in list(hashes.items()):"},{"line_number":1310,"context_line":"            if suffix in (\u0027valid\u0027, \u0027updated\u0027):"},{"line_number":1311,"context_line":"                continue"},{"line_number":1312,"context_line":"            if not hash_:"},{"line_number":1313,"context_line":"                suffix_dir \u003d join(partition_path, suffix)"},{"line_number":1314,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"07589292_a5cde05c","line":1311,"updated":"2021-03-29 18:38:18.000000000","message":"interesting...before, if valid\u003d\u003dFalse, we\u0027d try to open a dir .../valid, presumably failed and then remove the key \u0027valid\u0027 at line  1319!! ...which is ok because in write_hashes it was then set to default of False... PHEW!","commit_id":"ade3b28636dd3be79152af4dedfcc7a039dff025"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"f0b1eadf01ffda3d0d60a5172d85f3cd79c142f0","unresolved":true,"context_lines":[{"line_number":1308,"context_line":"        hashes.update((suffix, None) for suffix in recalculate)"},{"line_number":1309,"context_line":"        for suffix, hash_ in list(hashes.items()):"},{"line_number":1310,"context_line":"            if suffix in (\u0027valid\u0027, \u0027updated\u0027):"},{"line_number":1311,"context_line":"                continue"},{"line_number":1312,"context_line":"            if not hash_:"},{"line_number":1313,"context_line":"                suffix_dir \u003d join(partition_path, suffix)"},{"line_number":1314,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"08da0bfc_805c5ed7","line":1311,"in_reply_to":"07589292_a5cde05c","updated":"2021-03-29 20:12:44.000000000","message":"Yeah, this cleanup started to get away from me... but I think I like where it landed *a lot* more than where it started.","commit_id":"ade3b28636dd3be79152af4dedfcc7a039dff025"}],"test/unit/obj/test_diskfile.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f41053982a2dbe40b51b03946876a592bf93ee1","unresolved":true,"context_lines":[{"line_number":7101,"context_line":"            with open(inv_file) as f:"},{"line_number":7102,"context_line":"                self.assertEqual(\u0027\u0027, f.read().strip(\u0027\\n\u0027))"},{"line_number":7103,"context_line":""},{"line_number":7104,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":7105,"context_line":"        for policy in self.iter_policies():"},{"line_number":7106,"context_line":"            df_mgr \u003d self.df_router[policy]"},{"line_number":7107,"context_line":"            part_path \u003d os.path.join(self.devices, \u0027sda1\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ea29aaf3_262b0625","line":7104,"updated":"2021-03-25 12:44:42.000000000","message":"I think the test no longer achieves its goal, which was to invalidate a suffix while the hashes.pkl exists - but now, as asserted the hashes.pkl does not exist when the diskfile is deleted.\n\nSo really, this test now needs to have mkdirs(part_path) and a new test added to verify the part_path not being created.","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f41053982a2dbe40b51b03946876a592bf93ee1","unresolved":true,"context_lines":[{"line_number":7226,"context_line":"                                     policy\u003dpolicy)"},{"line_number":7227,"context_line":"            if existing:"},{"line_number":7228,"context_line":"                df.delete(self.ts())"},{"line_number":7229,"context_line":"                # create hashes.pkl"},{"line_number":7230,"context_line":"                df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7231,"context_line":""},{"line_number":7232,"context_line":"            suffix_dir \u003d os.path.dirname(df._datadir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1d1afa0d_92cdbc93","line":7229,"range":{"start_line":7229,"start_character":16,"end_line":7229,"end_character":35},"updated":"2021-03-25 12:44:42.000000000","message":"the hashes.pkl will now have an entry here whereas it was empty before\n\nI\u0027d rather a mkdirs(part_dir) was added so that the test is otherwise unchanged","commit_id":"73401b25dce2dab57d3d34ff7eba2570d3b0e5d0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":7104,"context_line":"    def test_invalidate_hash_empty_file_exists(self):"},{"line_number":7105,"context_line":"        for policy in self.iter_policies():"},{"line_number":7106,"context_line":"            df_mgr \u003d self.df_router[policy]"},{"line_number":7107,"context_line":"            hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7108,"context_line":"            self.assertEqual(hashes, {})"},{"line_number":7109,"context_line":"            # create something to hash"},{"line_number":7110,"context_line":"            df \u003d df_mgr.get_diskfile(\u0027sda1\u0027, \u00270\u0027, \u0027a\u0027, \u0027c\u0027, \u0027o\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff276d02_e83513bf","line":7107,"updated":"2021-03-26 12:24:48.000000000","message":"but the empty file no longer exists as the test assumes","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":8353,"context_line":"            self.assertEqual(hashes, {})"},{"line_number":8354,"context_line":"            part_path \u003d os.path.join("},{"line_number":8355,"context_line":"                self.devices, \u0027sda1\u0027, diskfile.get_data_dir(policy), \u00270\u0027)"},{"line_number":8356,"context_line":"            self.assertFalse(os.path.exists(part_path))"},{"line_number":8357,"context_line":""},{"line_number":8358,"context_line":"    def test_get_hashes_creates_pkl(self):"},{"line_number":8359,"context_line":"        # like above, but -- if the partition already exists, make the pickle"}],"source_content_type":"text/x-python","patch_set":4,"id":"a2984095_5c445210","line":8356,"updated":"2021-03-26 12:24:48.000000000","message":"nice and clear","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":7197,"context_line":""},{"line_number":7198,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"},{"line_number":7199,"context_line":"                            mock_listdir):"},{"line_number":7200,"context_line":"                # creates pkl file"},{"line_number":7201,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7202,"context_line":""},{"line_number":7203,"context_line":"            # second suffix added after directory listing, it\u0027s added later"},{"line_number":7204,"context_line":"            self.assertIn(suffix, hashes)"}],"source_content_type":"text/x-python","patch_set":5,"id":"b055654c_831f3283","line":7201,"range":{"start_line":7200,"start_character":16,"end_line":7201,"end_character":67},"updated":"2021-03-29 09:43:09.000000000","message":"in this case the comment appears to be misleading in light of line 7175","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8f7e86bb16e3591f18da1b1f2c530eab8983cb40","unresolved":false,"context_lines":[{"line_number":7197,"context_line":""},{"line_number":7198,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"},{"line_number":7199,"context_line":"                            mock_listdir):"},{"line_number":7200,"context_line":"                # creates pkl file"},{"line_number":7201,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7202,"context_line":""},{"line_number":7203,"context_line":"            # second suffix added after directory listing, it\u0027s added later"},{"line_number":7204,"context_line":"            self.assertIn(suffix, hashes)"}],"source_content_type":"text/x-python","patch_set":5,"id":"efc7e064_cbcc581a","line":7201,"range":{"start_line":7200,"start_character":16,"end_line":7201,"end_character":67},"in_reply_to":"1cf149f8_3bb89d13","updated":"2021-03-29 18:38:18.000000000","message":"Ack","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee0a9171ebb5481919cd47fb3fb9129bc690f8cb","unresolved":true,"context_lines":[{"line_number":7197,"context_line":""},{"line_number":7198,"context_line":"            with mock.patch(\u0027swift.obj.diskfile.os.listdir\u0027,"},{"line_number":7199,"context_line":"                            mock_listdir):"},{"line_number":7200,"context_line":"                # creates pkl file"},{"line_number":7201,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7202,"context_line":""},{"line_number":7203,"context_line":"            # second suffix added after directory listing, it\u0027s added later"},{"line_number":7204,"context_line":"            self.assertIn(suffix, hashes)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1cf149f8_3bb89d13","line":7201,"range":{"start_line":7200,"start_character":16,"end_line":7201,"end_character":67},"in_reply_to":"b055654c_831f3283","updated":"2021-03-29 17:26:04.000000000","message":"I\u0027ll add a \"if not already there\" or something. If existing is False, the df.delete(...) creates the partition dir and hashes.invalid, then this creates hashes.pkl.","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee0a9171ebb5481919cd47fb3fb9129bc690f8cb","unresolved":true,"context_lines":[{"line_number":7239,"context_line":"            suffix_dir \u003d os.path.dirname(df._datadir)"},{"line_number":7240,"context_line":"            suffix \u003d os.path.basename(suffix_dir)"},{"line_number":7241,"context_line":"            part_dir \u003d os.path.dirname(suffix_dir)"},{"line_number":7242,"context_line":"            mkdirs(part_dir)"},{"line_number":7243,"context_line":"            invalidations_file \u003d os.path.join("},{"line_number":7244,"context_line":"                part_dir, diskfile.HASH_INVALIDATIONS_FILE)"},{"line_number":7245,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e0e1ae99_f8c2bbb6","line":7242,"updated":"2021-03-29 17:26:04.000000000","message":"This one shouldn\u0027t be necessary.","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":7274,"context_line":"                return result"},{"line_number":7275,"context_line":""},{"line_number":7276,"context_line":"            with mock.patch.object(df_mgr, \u0027_hash_suffix\u0027, mock_hash_suffix):"},{"line_number":7277,"context_line":"                # creates pkl file and repeats listing when pkl modified"},{"line_number":7278,"context_line":"                hashes \u003d df_mgr.get_hashes(\u0027sda1\u0027, \u00270\u0027, [], policy)"},{"line_number":7279,"context_line":""},{"line_number":7280,"context_line":"            # first get_hashes should complete with suffix1 state"},{"line_number":7281,"context_line":"            self.assertIn(suffix, hashes)"},{"line_number":7282,"context_line":"            # sanity check - the suffix hash has changed..."}],"source_content_type":"text/x-python","patch_set":5,"id":"d87f1081_8c4bb374","line":7279,"range":{"start_line":7277,"start_character":16,"end_line":7279,"end_character":0},"updated":"2021-03-29 09:43:09.000000000","message":"ditto: comment is misleading","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":7520,"context_line":"            self.assertEqual({suffix: \u0027new fake hash\u0027}, hashes)"},{"line_number":7521,"context_line":""},{"line_number":7522,"context_line":"            # sanity check hashes file"},{"line_number":7523,"context_line":"            part_path \u003d os.path.join(self.devices, \u0027sda1\u0027,"},{"line_number":7524,"context_line":"                                     diskfile.get_data_dir(policy), \u00270\u0027)"},{"line_number":7525,"context_line":"            hashes_file \u003d os.path.join(part_path, diskfile.HASH_FILE)"},{"line_number":7526,"context_line":"            with open(hashes_file, \u0027rb\u0027) as f:"},{"line_number":7527,"context_line":"                found_hashes \u003d pickle.load(f)"},{"line_number":7528,"context_line":"                found_hashes.pop(\u0027updated\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bb0e4361_87b06213","line":7525,"range":{"start_line":7523,"start_character":12,"end_line":7525,"end_character":69},"updated":"2021-03-29 09:43:09.000000000","message":"pre-existing duplication: these have already been defined","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c1dca06cc4bb29ba88397e993e463d4ad9990723","unresolved":true,"context_lines":[{"line_number":8210,"context_line":"            df_mgr \u003d self.df_router[policy]"},{"line_number":8211,"context_line":"            part_path \u003d os.path.join(self.devices, \u0027sda1\u0027,"},{"line_number":8212,"context_line":"                                     diskfile.get_data_dir(policy), \u00270\u0027)"},{"line_number":8213,"context_line":"            mkdirs(part_path)  # ensure we\u0027ll bother writing a pkl at all"},{"line_number":8214,"context_line":"            orig_listdir \u003d os.listdir"},{"line_number":8215,"context_line":"            listdir_calls \u003d []"},{"line_number":8216,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bd3597bd_72f10f59","line":8213,"updated":"2021-03-29 09:43:09.000000000","message":"+1","commit_id":"fc1e9a2da0b7091c99dca089a64aa0339ba2ae34"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"455dea083679bafa66a09645d0e23f750d523575","unresolved":true,"context_lines":[{"line_number":7162,"context_line":"                               fake_get_hashes), \\"},{"line_number":7163,"context_line":"                mock.patch.object(tpool, \u0027execute\u0027, my_tpool_execute), \\"},{"line_number":7164,"context_line":"                mock.patch(\u0027swift.obj.diskfile.os.path.exists\u0027,"},{"line_number":7165,"context_line":"                           return_value\u003dTrue):"},{"line_number":7166,"context_line":"            req \u003d Request.blank(\u0027/sda1/p/\u0027,"},{"line_number":7167,"context_line":"                                environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027REPLICATE\u0027},"},{"line_number":7168,"context_line":"                                headers\u003d{})"}],"source_content_type":"text/x-python","patch_set":4,"id":"88bb1bf5_5b10fa1f","line":7165,"updated":"2021-03-26 12:24:48.000000000","message":"nice cleanup","commit_id":"94cd4684e5a7f4ffda7a228da0df01e5915a7dc3"}]}