)]}'
{"swift/cli/info.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a48ea69417839eb430ef14991e1c2c3d195e744b","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"    # try to extract policy index from datafile disk path"},{"line_number":368,"context_line":"    fullpath \u003d os.path.abspath(datafile)"},{"line_number":369,"context_line":"    policy_index \u003d int(extract_policy(fullpath) or POLICIES.legacy)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"    try:"},{"line_number":372,"context_line":"        if policy_index:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_c2087430","line":369,"updated":"2015-07-24 08:01:21.000000000","message":"This seem weird a bit because the document for extract_policy describes that the function takes a device relative path (not abspath).\n\nI wrote a test[1] for a device named \"object-0\", that failed... :(\n\nIMO, we should build the path from the directory actual data stored in like [2], though it should be good to make a util function for that.\n\n1: http://paste.openstack.org/show/405039/\n\n2: https://github.com/openstack/swift/blob/master/swift/obj/diskfile.py#L322-L324","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":8859,"name":"Daisuke Morita","email":"morita.daisuke@ntti3.com","username":"morrisson"},"change_message_id":"27138309ab9b32229e047465db7dfdd83e2d1397","unresolved":false,"context_lines":[{"line_number":366,"context_line":""},{"line_number":367,"context_line":"    # try to extract policy index from datafile disk path"},{"line_number":368,"context_line":"    fullpath \u003d os.path.abspath(datafile)"},{"line_number":369,"context_line":"    policy_index \u003d int(extract_policy(fullpath) or POLICIES.legacy)"},{"line_number":370,"context_line":""},{"line_number":371,"context_line":"    try:"},{"line_number":372,"context_line":"        if policy_index:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_5ab3d90a","line":369,"in_reply_to":"3a50d1a3_c2087430","updated":"2015-07-24 13:29:41.000000000","message":"Thanks. I defined \u0027obj_path\u0027 by following extract_policy method\u0027s description. In addition, I added your tests to test_info.py .","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"731d5014ee601d894ca1e11d0d4519987b76f18b","unresolved":false,"context_lines":[{"line_number":370,"context_line":"        # obj_path should be device-relative path of an object"},{"line_number":371,"context_line":"        obj_path \u003d fullpath[fullpath.rindex(\u0027objects\u0027):]"},{"line_number":372,"context_line":"    except ValueError:"},{"line_number":373,"context_line":"        obj_path \u003d fullpath"},{"line_number":374,"context_line":"    policy_index \u003d int(extract_policy(obj_path) or POLICIES.legacy)"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a50d1a3_f5166ca6","line":373,"updated":"2015-07-24 18:03:34.000000000","message":"hrmm... I think passing the full path to extract_policy was a simple and obvious fix to cli.info.  Since extract_policy was expecting to be called with the full path, abspath fix was obviously correct.\n\nThis change... \n\nwell if it\u0027s a good idea to make this change it should just be in extract_policy - what did the tests think about doing it there?","commit_id":"3d9b2d077a157279fb6cd107a0c83727106af3ef"}],"test/unit/cli/test_info.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"68b0556fc9a6c139df01453f9c8efefcf0eef4d0","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        # Check an output of policy index when current directory is in"},{"line_number":391,"context_line":"        # object-* directory"},{"line_number":392,"context_line":"        out \u003d StringIO()"},{"line_number":393,"context_line":"        dir_and_file \u003d self.datafile.rsplit(\u0027/\u0027, 1)"},{"line_number":394,"context_line":"        cwd \u003d os.getcwd()"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        # Change working directory to object hash dir"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_d4acb237","line":393,"updated":"2015-07-24 05:53:44.000000000","message":"i might have just used:\n\n    dir_, file \u003d os.path.dirname(self.datafile), os.path.basename(self.datafile)","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":8859,"name":"Daisuke Morita","email":"morita.daisuke@ntti3.com","username":"morrisson"},"change_message_id":"27138309ab9b32229e047465db7dfdd83e2d1397","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        # Check an output of policy index when current directory is in"},{"line_number":391,"context_line":"        # object-* directory"},{"line_number":392,"context_line":"        out \u003d StringIO()"},{"line_number":393,"context_line":"        dir_and_file \u003d self.datafile.rsplit(\u0027/\u0027, 1)"},{"line_number":394,"context_line":"        cwd \u003d os.getcwd()"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        # Change working directory to object hash dir"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_9aa081a7","line":393,"in_reply_to":"3a50d1a3_621c006b","updated":"2015-07-24 13:29:41.000000000","message":"Done","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a48ea69417839eb430ef14991e1c2c3d195e744b","unresolved":false,"context_lines":[{"line_number":390,"context_line":"        # Check an output of policy index when current directory is in"},{"line_number":391,"context_line":"        # object-* directory"},{"line_number":392,"context_line":"        out \u003d StringIO()"},{"line_number":393,"context_line":"        dir_and_file \u003d self.datafile.rsplit(\u0027/\u0027, 1)"},{"line_number":394,"context_line":"        cwd \u003d os.getcwd()"},{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        # Change working directory to object hash dir"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_621c006b","line":393,"in_reply_to":"3a50d1a3_d4acb237","updated":"2015-07-24 08:01:21.000000000","message":"yup, that seems more meaningful. +1.","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"a48ea69417839eb430ef14991e1c2c3d195e744b","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        os.chdir(dir_and_file[0])"},{"line_number":398,"context_line":"        with mock.patch(\u0027sys.stdout\u0027, out):"},{"line_number":399,"context_line":"            print_obj(dir_and_file[1], swift_dir\u003dself.testdir)"},{"line_number":400,"context_line":"        os.chdir(cwd)"},{"line_number":401,"context_line":"        self.assertTrue(\u0027X-Backend-Storage-Policy-Index: 1\u0027 in out.getvalue())"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"    def test_print_obj_meta_and_ts_files(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_82ed8cf1","line":400,"updated":"2015-07-24 08:01:21.000000000","message":"I guess, you should wrap try/except/finally for os.chdir like the code from L277, otherwise if something wrong occurred between the directory change, unit test will loose the chance to get back the original correct directly.","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":8859,"name":"Daisuke Morita","email":"morita.daisuke@ntti3.com","username":"morrisson"},"change_message_id":"27138309ab9b32229e047465db7dfdd83e2d1397","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        os.chdir(dir_and_file[0])"},{"line_number":398,"context_line":"        with mock.patch(\u0027sys.stdout\u0027, out):"},{"line_number":399,"context_line":"            print_obj(dir_and_file[1], swift_dir\u003dself.testdir)"},{"line_number":400,"context_line":"        os.chdir(cwd)"},{"line_number":401,"context_line":"        self.assertTrue(\u0027X-Backend-Storage-Policy-Index: 1\u0027 in out.getvalue())"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"    def test_print_obj_meta_and_ts_files(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_7aafb594","line":400,"in_reply_to":"3a50d1a3_82ed8cf1","updated":"2015-07-24 13:29:41.000000000","message":"Done","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"68b0556fc9a6c139df01453f9c8efefcf0eef4d0","unresolved":false,"context_lines":[{"line_number":398,"context_line":"        with mock.patch(\u0027sys.stdout\u0027, out):"},{"line_number":399,"context_line":"            print_obj(dir_and_file[1], swift_dir\u003dself.testdir)"},{"line_number":400,"context_line":"        os.chdir(cwd)"},{"line_number":401,"context_line":"        self.assertTrue(\u0027X-Backend-Storage-Policy-Index: 1\u0027 in out.getvalue())"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"    def test_print_obj_meta_and_ts_files(self):"},{"line_number":404,"context_line":"        # verify that print_obj will also read from meta and ts files"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_94a62a52","line":401,"updated":"2015-07-24 05:53:44.000000000","message":"so this is from down in the curl commands - when I tested it just used the default 0 if you did it too close to the file :\u0027(","commit_id":"013b919f9c8611f66fd4cfc089983c33e85e3905"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"731d5014ee601d894ca1e11d0d4519987b76f18b","unresolved":false,"context_lines":[{"line_number":401,"context_line":"                print_obj(file_name, swift_dir\u003dself.testdir)"},{"line_number":402,"context_line":"            os.chdir(cwd)"},{"line_number":403,"context_line":"        except OSError:  # Failure case of os.chdir"},{"line_number":404,"context_line":"            self.fail(\"Unexpected exception raised\")"},{"line_number":405,"context_line":"        self.assertTrue(\u0027X-Backend-Storage-Policy-Index: 1\u0027 in out.getvalue())"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"    def test_print_obj_meta_and_ts_files(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a50d1a3_1503d8b9","line":404,"updated":"2015-07-24 18:03:34.000000000","message":"wouldn\u0027t it be more helpful to see the traceback from the OSError?\n\n... oh you wanted a finally\n\n    diff --git a/test/unit/cli/test_info.py b/test/unit/cli/test_info.py\n    index 856a568..c76b2ea 100644\n    --- a/test/unit/cli/test_info.py\n    +++ b/test/unit/cli/test_info.py\n    @@ -399,9 +399,8 @@ class TestPrintObjFullMeta(TestCliInfoBase):\n                 os.chdir(hash_dir)\n                 with mock.patch(\u0027sys.stdout\u0027, out):\n                     print_obj(file_name, swift_dir\u003dself.testdir)\n    +        finally:\n                 os.chdir(cwd)\n    -        except OSError:  # Failure case of os.chdir\n    -            self.fail(\"Unexpected exception raised\")\n             self.assertTrue(\u0027X-Backend-Storage-Policy-Index: 1\u0027 in out.getvalue())\n     \n         def test_print_obj_meta_and_ts_files(self):","commit_id":"3d9b2d077a157279fb6cd107a0c83727106af3ef"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"731d5014ee601d894ca1e11d0d4519987b76f18b","unresolved":false,"context_lines":[{"line_number":635,"context_line":"        super(TestPrintObjWeirdPath, self).setUp()"},{"line_number":636,"context_line":"        # device name is objects-0 instead of sda, this is weird."},{"line_number":637,"context_line":"        self.datafile \u003d os.path.join(self.testdir,"},{"line_number":638,"context_line":"                                     \u0027objects-0\u0027, \u0027objects-1\u0027,"},{"line_number":639,"context_line":"                                     \u00271\u0027, \u0027ea8\u0027,"},{"line_number":640,"context_line":"                                     \u0027db4449e025aca992307c7c804a67eea8\u0027,"},{"line_number":641,"context_line":"                                     \u00271402017884.18202.data\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a50d1a3_b55ea474","line":638,"updated":"2015-07-24 18:03:34.000000000","message":"VERY STRANGE INDEED!  What if the *partition* dir is named object-2!?  It might never end!?\n\nHonestly I\u0027m on the fence about this test - from a maintenance perspective it might be confusing to future me.  If this test case were to break at some point, I\u0027m pretty sure future me would be very confused - \"Why is there a test for this path!?  Did someone really see this?!  Does all of the diskfile code need to be tested against this case?!  What *else* could go wrong with random names in our paths?! Agh!  Freak out!?\"","commit_id":"3d9b2d077a157279fb6cd107a0c83727106af3ef"}]}
