)]}'
{"nova/storage/linuxscsi.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"6e6dbfcf5cab212cc73f9bdc1fcf1be73bb18a03","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"MULTIPATH_ERROR_REGEX \u003d re.compile(\"\\w{3} \\d+ \\d\\d:\\d\\d:\\d\\d \\|.*$\")"},{"line_number":30,"context_line":"MULTIPATH_WWID_REGEX \u003d re.compile(\"\\((?P\u003cwwid\u003e.+)\\)\")"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_a18662b7","line":29,"updated":"2015-04-12 17:00:55.000000000","message":"funny, we have the same regex in another review:\nhttps://review.openstack.org/#/c/165560/1/nova/virt/libvirt/volume.py,cm\n\nworth splitting into a separate utility method?","commit_id":"f5fa901f312bc458fc6113e337587a48eb063aec"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"28754dcc0b0f4d8b010177ad3d52b1d07b3d9fb6","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"MULTIPATH_ERROR_REGEX \u003d re.compile(\"\\w{3} \\d+ \\d\\d:\\d\\d:\\d\\d \\|.*$\")"},{"line_number":30,"context_line":"MULTIPATH_WWID_REGEX \u003d re.compile(\"\\((?P\u003cwwid\u003e.+)\\)\")"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_16ad50e9","line":29,"in_reply_to":"7aaa499b_a18662b7","updated":"2015-04-13 10:01:13.000000000","message":"Nice catch, thanks! IMHO the above change should be calling linuxscsi.find_multipath_device(device) to avoid duplicating this logic.","commit_id":"f5fa901f312bc458fc6113e337587a48eb063aec"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"69eb79cad7a47e6d3fce182b83d8d10a2c1330b6","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        lines \u003d out.strip()"},{"line_number":113,"context_line":"        lines \u003d lines.split(\"\\n\")"},{"line_number":114,"context_line":"        lines \u003d [line for line in lines"},{"line_number":115,"context_line":"                 if not re.match(MULTIPATH_ERROR_REGEX, line)]"},{"line_number":116,"context_line":"        if lines:"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"            # Use the device name, be it the WWID, mpathN or custom alias of"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_2c9f9a04","line":115,"updated":"2015-05-28 15:51:08.000000000","message":"You compiled the regex when it was declared, so you should call \u0027match\u0027 on the compiled expression. Also, lines 111-115 could be simplified.\n\n    if out:\n        lines \u003d [line for line in out.strip().splitlines()\n                 if not MULTIPATH_ERROR_REGEX.match(line)]","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7beec6234b8909917bb2c4e3dcb985163a75ce46","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        lines \u003d out.strip()"},{"line_number":113,"context_line":"        lines \u003d lines.split(\"\\n\")"},{"line_number":114,"context_line":"        lines \u003d [line for line in lines"},{"line_number":115,"context_line":"                 if not re.match(MULTIPATH_ERROR_REGEX, line)]"},{"line_number":116,"context_line":"        if lines:"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"            # Use the device name, be it the WWID, mpathN or custom alias of"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afb71cf_6556e05d","line":115,"in_reply_to":"7a016987_2c9f9a04","updated":"2015-06-08 17:33:09.000000000","message":"Nice idea, I\u0027ll respin the patch to include this now.","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"15a98019d7c2d1aeac93aae50415d207c74a6ed8","unresolved":false,"context_lines":[{"line_number":102,"context_line":"    out \u003d None"},{"line_number":103,"context_line":"    try:"},{"line_number":104,"context_line":"        (out, err) \u003d utils.execute(\u0027multipath\u0027, \u0027-l\u0027, device,"},{"line_number":105,"context_line":"                                   run_as_root\u003dTrue)"},{"line_number":106,"context_line":"    except processutils.ProcessExecutionError as exc:"},{"line_number":107,"context_line":"        LOG.warning(_LW(\"Multipath call failed exit (%(code)s)\"),"},{"line_number":108,"context_line":"                    {\u0027code\u0027: exc.exit_code})"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_05c30939","line":105,"updated":"2015-06-24 19:23:50.000000000","message":"Yes, this looks better, but it is an unrelated change, and should be in a separate patchset.","commit_id":"a5085052449e77bf5b3c55772ff86b16770cf7ad"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"15a98019d7c2d1aeac93aae50415d207c74a6ed8","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"                dev \u003d {\u0027device\u0027: \u0027/dev/%s\u0027 % dev_info[1],"},{"line_number":153,"context_line":"                       \u0027host\u0027: address[0], \u0027channel\u0027: address[1],"},{"line_number":154,"context_line":"                       \u0027id\u0027: address[2], \u0027lun\u0027: address[3]}"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"                devices.append(dev)"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"fa32b979_25be4dae","line":154,"updated":"2015-06-24 19:23:50.000000000","message":"Unrelated change","commit_id":"a5085052449e77bf5b3c55772ff86b16770cf7ad"}],"nova/tests/unit/test_linuxscsi.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4ac0645b6d7ba8b9247aab36c864fb64b5e57b08","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                   )"},{"line_number":187,"context_line":"            return out, None"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_b71fc408","line":189,"updated":"2015-05-26 08:35:31.000000000","message":"why not use mock here?","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e14ca177d12af1f6e92aed1bc817679967c64800","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                   )"},{"line_number":187,"context_line":"            return out, None"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_9f872b12","line":189,"in_reply_to":"7a016987_b71fc408","updated":"2015-05-26 18:27:34.000000000","message":"I was just sticking to the method template used with all other tests in this file..","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"9bbe0c19ca4b2dc8fa2bf5094bcae84ed7f952f7","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"id\"])"},{"line_number":195,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"name\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_42e35ed9","line":192,"updated":"2015-05-22 18:30:14.000000000","message":"Why log the return value? It seems like tests shouldn\u0027t log anything although I see that pattern in other test cases in this file.","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e14ca177d12af1f6e92aed1bc817679967c64800","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"id\"])"},{"line_number":195,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"name\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_3f86f713","line":192,"in_reply_to":"7a016987_37d2b435","updated":"2015-05-26 18:27:34.000000000","message":"Again, this is the same as the other tests in this file. IMHO changing _all_ of these tests to use mock and not log is out of the scope of this patch and I\u0027d rather it not be blocked for this. Happy to follow up with additional patches to clean this up after etc.","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"69eb79cad7a47e6d3fce182b83d8d10a2c1330b6","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"id\"])"},{"line_number":195,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"name\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_6c1232b6","line":192,"in_reply_to":"7a016987_3f86f713","updated":"2015-05-28 15:51:08.000000000","message":"Agree that cleaning up the other tests is not in scope for this patch. However, since this is a new test, it should use mock and should not log anything. It can then be used as a template for cleaning up the other tests in a later patch.","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"4ac0645b6d7ba8b9247aab36c864fb64b5e57b08","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"id\"])"},{"line_number":195,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"name\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a016987_37d2b435","line":192,"in_reply_to":"7a016987_42e35ed9","updated":"2015-05-26 08:35:31.000000000","message":"i agree - you should remove the log here","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7beec6234b8909917bb2c4e3dcb985163a75ce46","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self.stubs.Set(utils, \u0027execute\u0027, fake_execute)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        info \u003d linuxscsi.find_multipath_device(\u0027/dev/sdd\u0027)"},{"line_number":192,"context_line":"        LOG.error(\"info \u003d %s\" % info)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"id\"])"},{"line_number":195,"context_line":"        self.assertEqual(\"36005076303ffc48e0000000000000101\", info[\"name\"])"}],"source_content_type":"text/x-python","patch_set":4,"id":"3afb71cf_85879cd6","line":192,"in_reply_to":"7a016987_6c1232b6","updated":"2015-06-08 17:33:09.000000000","message":"Again I really think this is a waste of time in this patchset. os-brick has already ported all of these tests over to using mock [1]. Porting individual tests over is just a waste of time when I could easily follow up with a single patch to move all of these tests over to mock, clean up logging etc.\n\n[1]https://github.com/openstack/os-brick/blob/master/os_brick/tests/initiator/test_linuxscsi.py","commit_id":"d731901f06f61413f222395b12cac3c3b3e425de"}]}