)]}'
{"swift/common/db_replicator.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"087f2e85a04ce4c2715b1e831345eeff5af9c81f","unresolved":false,"context_lines":[{"line_number":107,"context_line":"                        yield (partition, object_file, node_id)"},{"line_number":108,"context_line":"                    else:"},{"line_number":109,"context_line":"                        try:"},{"line_number":110,"context_line":"                            os.rmdir(hash_dir)"},{"line_number":111,"context_line":"                        except OSError as e:"},{"line_number":112,"context_line":"                            if e.errno is not errno.ENOTEMPTY:"},{"line_number":113,"context_line":"                                raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_6999666e","line":110,"updated":"2016-12-01 21:47:02.000000000","message":"Now if you remove the try/catch here and just do the rmdir, then no unit tests fail.  The tempdir added in the related patch was added to show how this part could be tested.  The unit test created a file in the hash dir that did not end in .db - and it was asserted that no error was raised from the rmdir called on that directory.","commit_id":"2ce139bfeaf318f30e9ec184163a1e4ade2f9380"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dd14f16ed6562332cfd3f3cc55ecf823edc5f2c9","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            if not suffixes:"},{"line_number":91,"context_line":"                os.rmdir(part_dir)"},{"line_number":92,"context_line":"                continue"},{"line_number":93,"context_line":"            for suffix in suffixes:"},{"line_number":94,"context_line":"                suff_dir \u003d os.path.join(part_dir, suffix)"},{"line_number":95,"context_line":"                if not os.path.isdir(suff_dir):"},{"line_number":96,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_0fd9d3f3","line":93,"updated":"2017-01-13 20:06:17.000000000","message":"I feel like it\u0027s not super obvious in this diff that the removed code below was `if False` because of the continue above - I think the intent was if we os.rmdir(suff_dir) below on *all* the suffixes we *could* try:rmdir;except:ENOTEMPTY - but it\u0027s probably better/easier/safer just to wait to the next pass.","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"}],"test/unit/common/test_db_replicator.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dd14f16ed6562332cfd3f3cc55ecf823edc5f2c9","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"        db_path \u003d (\u0027450/afd/7089ab48d955ab0851fc51cc17a34afd/\u0027"},{"line_number":1056,"context_line":"                   \u00277089ab48d955ab0851fc51cc17a34afd.db\u0027)"},{"line_number":1057,"context_line":"        random_file \u003d (\u00271060/xyz/1234ab48d955ab0851fc51cc17a34xyz/\u0027"},{"line_number":1058,"context_line":"                       \u00271234ab48d955ab0851fc51cc17a34xyz.abc\u0027)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        paths \u003d ["},{"line_number":1061,"context_line":"            # empty part dir"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_cfec2bca","line":1058,"updated":"2017-01-13 20:06:17.000000000","message":"I think *this* random_file addresses Janie\u0027s earlier spot on the missed coverage for the ENOTEMPTY branch\n\ntests pops if you break the code:\n\n    diff --git a/swift/common/db_replicator.py b/swift/common/db_replicator.py\n    index 4c0082a..5874735 100644\n    --- a/swift/common/db_replicator.py\n    +++ b/swift/common/db_replicator.py\n    @@ -106,8 +106,9 @@ def roundrobin_datadirs(datadirs):\n                         if os.path.exists(object_file):\n                             yield (partition, object_file, node_id)\n                         else:\n    +                        os.rmdir(hash_dir)\n                             try:\n    -                            os.rmdir(hash_dir)\n    +                            pass\n                             except OSError as e:\n                                 if e.errno is not errno.ENOTEMPTY:\n                                     raise\n\nerror lgtm:\n\n    ubuntu@saio:~$ nosetests swift/test/unit/common/test_db_replicator.py\n    .............E................................................\n    \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n    ERROR: test_empty_suffix_and_hash_dirs_get_cleanedup (test.unit.common.test_db_replicator.TestDBReplicator)\n    ----------------------------------------------------------------------\n    Traceback (most recent call last):\n      File \"/home/ubuntu/swift/test/unit/__init__.py\", line 445, in wrapped\n        return f(*args, **kwargs)\n      File \"/home/ubuntu/swift/test/unit/common/test_db_replicator.py\", line 1085, in test_empty_suffix_and_hash_dirs_get_cleanedup\n        results \u003d list(db_replicator.roundrobin_datadirs([(datadir, node_id)]))\n      File \"/home/ubuntu/swift/swift/common/db_replicator.py\", line 120, in roundrobin_datadirs\n        yield next(it)\n      File \"/home/ubuntu/swift/swift/common/db_replicator.py\", line 109, in walk_datadir\n        os.rmdir(hash_dir)\n    OSError: [Errno 39] Directory not empty: \u0027/tmp/tmpYcc3gD/containers/1060/xyz/1234ab48d955ab0851fc51cc17a34xyz\u0027\n\n    ----------------------------------------------------------------------\n    Ran 62 tests in 0.109s\n\n    FAILED (errors\u003d1)","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"701f2fcb4bd9549c03b78c4bbed10175f181723f","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"        db_path \u003d (\u0027450/afd/7089ab48d955ab0851fc51cc17a34afd/\u0027"},{"line_number":1056,"context_line":"                   \u00277089ab48d955ab0851fc51cc17a34afd.db\u0027)"},{"line_number":1057,"context_line":"        random_file \u003d (\u00271060/xyz/1234ab48d955ab0851fc51cc17a34xyz/\u0027"},{"line_number":1058,"context_line":"                       \u00271234ab48d955ab0851fc51cc17a34xyz.abc\u0027)"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        paths \u003d ["},{"line_number":1061,"context_line":"            # empty part dir"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_f541b83a","line":1058,"in_reply_to":"7a3c09a3_cfec2bca","updated":"2017-01-13 22:12:31.000000000","message":"yep","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"701f2fcb4bd9549c03b78c4bbed10175f181723f","unresolved":false,"context_lines":[{"line_number":1090,"context_line":""},{"line_number":1091,"context_line":"        # all the empty leaf dirs are cleaned up"},{"line_number":1092,"context_line":"        for path in paths:"},{"line_number":1093,"context_line":"            if os.path.basename(path):"},{"line_number":1094,"context_line":"                check \u003d self.assertTrue"},{"line_number":1095,"context_line":"            else:"},{"line_number":1096,"context_line":"                check \u003d self.assertFalse"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_8a304e58","line":1093,"updated":"2017-01-13 22:12:31.000000000","message":"This all depends on the fact that the directory names specified in \u0027paths\u0027 end in \u0027/\u0027.  That seems a little brittle, but if that is the best we can do without a lot of work, it would be ok if there is a comment about this in the definition of \u0027paths\u0027.","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"3b5ef878391d592f7043f7c86b018e858e8fee85","unresolved":false,"context_lines":[{"line_number":1090,"context_line":""},{"line_number":1091,"context_line":"        # all the empty leaf dirs are cleaned up"},{"line_number":1092,"context_line":"        for path in paths:"},{"line_number":1093,"context_line":"            if os.path.basename(path):"},{"line_number":1094,"context_line":"                check \u003d self.assertTrue"},{"line_number":1095,"context_line":"            else:"},{"line_number":1096,"context_line":"                check \u003d self.assertFalse"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_9d833acc","line":1093,"in_reply_to":"7a3c09a3_1dbf4afc","updated":"2017-01-13 22:38:53.000000000","message":"yeah, or just use os.path.isfile()","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"afc46c70883390a11ab880aa1d08cba2e992c5a6","unresolved":false,"context_lines":[{"line_number":1090,"context_line":""},{"line_number":1091,"context_line":"        # all the empty leaf dirs are cleaned up"},{"line_number":1092,"context_line":"        for path in paths:"},{"line_number":1093,"context_line":"            if os.path.basename(path):"},{"line_number":1094,"context_line":"                check \u003d self.assertTrue"},{"line_number":1095,"context_line":"            else:"},{"line_number":1096,"context_line":"                check \u003d self.assertFalse"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_1dbf4afc","line":1093,"in_reply_to":"7a3c09a3_8a304e58","updated":"2017-01-13 22:27:08.000000000","message":"or maybe just a comment here that says \"if path ends in \u0027/\u0027, then check that it no longer exists.\" to make it a little more clear what is happening.","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"dd14f16ed6562332cfd3f3cc55ecf823edc5f2c9","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"                mock.patch(base + \u0027os.path.isdir\u0027, _isdir), \\"},{"line_number":1188,"context_line":"                mock.patch(base + \u0027os.path.exists\u0027, _exists), \\"},{"line_number":1189,"context_line":"                mock.patch(base + \u0027random.shuffle\u0027, _shuffle), \\"},{"line_number":1190,"context_line":"                mock.patch(base + \u0027os.rmdir\u0027, _rmdir):"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"            datadirs \u003d [(\u0027/srv/node/sda/containers\u0027, 1),"},{"line_number":1193,"context_line":"                        (\u0027/srv/node/sdb/containers\u0027, 2)]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_efd1e7fc","line":1190,"updated":"2017-01-13 20:06:17.000000000","message":"I think the whole change is worth it just for this!","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"701f2fcb4bd9549c03b78c4bbed10175f181723f","unresolved":false,"context_lines":[{"line_number":1187,"context_line":"                mock.patch(base + \u0027os.path.isdir\u0027, _isdir), \\"},{"line_number":1188,"context_line":"                mock.patch(base + \u0027os.path.exists\u0027, _exists), \\"},{"line_number":1189,"context_line":"                mock.patch(base + \u0027random.shuffle\u0027, _shuffle), \\"},{"line_number":1190,"context_line":"                mock.patch(base + \u0027os.rmdir\u0027, _rmdir):"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"            datadirs \u003d [(\u0027/srv/node/sda/containers\u0027, 1),"},{"line_number":1193,"context_line":"                        (\u0027/srv/node/sdb/containers\u0027, 2)]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_b520109d","line":1190,"in_reply_to":"7a3c09a3_efd1e7fc","updated":"2017-01-13 22:12:31.000000000","message":"yes, it\u0027s much better","commit_id":"ed59696de309f54dea03b917ade67a1b5d924697"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5d05b05b7d2eef2d200ae6226fe805cb1f0a14e9","unresolved":false,"context_lines":[{"line_number":1093,"context_line":""},{"line_number":1094,"context_line":"        # all the empty leaf dirs are cleaned up"},{"line_number":1095,"context_line":"        for path in paths:"},{"line_number":1096,"context_line":"            if os.path.basename(path):"},{"line_number":1097,"context_line":"                check \u003d self.assertTrue"},{"line_number":1098,"context_line":"            else:"},{"line_number":1099,"context_line":"                check \u003d self.assertFalse"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a3c09a3_d6683fbd","line":1096,"updated":"2017-01-14 00:28:44.000000000","message":"ok I mis-read this\n\nI was reading os.path.basename(path) as this path represents a file\n\n...","commit_id":"f0122b69c41ccedadca602f5e0edf7df644c9118"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5d05b05b7d2eef2d200ae6226fe805cb1f0a14e9","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"            else:"},{"line_number":1099,"context_line":"                check \u003d self.assertFalse"},{"line_number":1100,"context_line":"            dirpath \u003d os.path.join(datadir, os.path.dirname(path))"},{"line_number":1101,"context_line":"            isdir \u003d os.path.isdir(dirpath)"},{"line_number":1102,"context_line":"            check(isdir, \u0027%r is%s a directory!\u0027 % ("},{"line_number":1103,"context_line":"                dirpath, \u0027\u0027 if isdir else \u0027 not\u0027))"},{"line_number":1104,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a3c09a3_f6ac5bef","line":1101,"updated":"2017-01-14 00:28:44.000000000","message":"but then I missed we got up a path segment from there and make assertions about the parent.\n\nEssentially if the the path defined a file - make sure it\u0027s parent directory exists.  If we created an empty directory - the dirname is the empty directory and it should not exist.\n\nSo... that\u0027s pretty ^\u0026*ing trixy :\\","commit_id":"f0122b69c41ccedadca602f5e0edf7df644c9118"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"c3f351819f51523f576f68aa8ebb31be7cb62cd1","unresolved":false,"context_lines":[{"line_number":1098,"context_line":"            else:"},{"line_number":1099,"context_line":"                check \u003d self.assertFalse"},{"line_number":1100,"context_line":"            dirpath \u003d os.path.join(datadir, os.path.dirname(path))"},{"line_number":1101,"context_line":"            isdir \u003d os.path.isdir(dirpath)"},{"line_number":1102,"context_line":"            check(isdir, \u0027%r is%s a directory!\u0027 % ("},{"line_number":1103,"context_line":"                dirpath, \u0027\u0027 if isdir else \u0027 not\u0027))"},{"line_number":1104,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7a3c09a3_d6891fcc","line":1101,"in_reply_to":"7a3c09a3_f6ac5bef","updated":"2017-01-14 01:19:07.000000000","message":"yes, I had to get cozy with it before seeing the whole picture :-)","commit_id":"f0122b69c41ccedadca602f5e0edf7df644c9118"}]}