)]}'
{"nova/virt/disk/api.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1d6aadba59ee17d2071cb8581d56ca2a12361d8e","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # For raw, we can directly inspect the file system"},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            utils.execute(\u0027e2label\u0027, image)"},{"line_number":153,"context_line":"            return True"},{"line_number":154,"context_line":"        except exception.ProcessExecutionError, e:"},{"line_number":155,"context_line":"            LOG.debug(_(\u0027Unable to determine label for image %(image)s with \u0027"},{"line_number":156,"context_line":"                        \u0027error %(errror)s. Cannot resize.\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2Fdbc%3D","line":153,"updated":"2013-01-21 19:49:09.000000000","message":"I don\u0027t understand this change here.  Consider the 4 cases of the original code:\n\n  1. use_cow \u003d\u003d True, no exception thrown: return True\n  2. use_cow \u003d\u003d True, exception thrown: return False\n  3. use_cow \u003d\u003d False, no exception thrown: return True\n  4. use_cow \u003d\u003d False, exception thrown: return False\n\nWith this change, case 1 now returns None instead of True.  Is there some reason we want this behavior?","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"1d75987c4b8f60a0aa9b35304aa4c4ab1d802b83","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # For raw, we can directly inspect the file system"},{"line_number":151,"context_line":"        try:"},{"line_number":152,"context_line":"            utils.execute(\u0027e2label\u0027, image)"},{"line_number":153,"context_line":"            return True"},{"line_number":154,"context_line":"        except exception.ProcessExecutionError, e:"},{"line_number":155,"context_line":"            LOG.debug(_(\u0027Unable to determine label for image %(image)s with \u0027"},{"line_number":156,"context_line":"                        \u0027error %(errror)s. Cannot resize.\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2FcSI%3D","line":153,"in_reply_to":"AAAAM3%2F%2Fdbc%3D","updated":"2013-01-22 14:52:23.000000000","message":"Good catch. I was adjusting this function and then decided against it, but didn\u0027t revert fully.","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1d6aadba59ee17d2071cb8581d56ca2a12361d8e","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        finally:"},{"line_number":307,"context_line":"            fs.teardown()"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    return status"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"def setup_container(image, container_dir, use_cow\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2Fdas%3D","line":309,"updated":"2013-01-21 19:49:09.000000000","message":"It seems to me that the fiddling with this \u0027status\u0027 variable is unnecessary; everywhere you alter the value of \u0027status\u0027, you can instead do a return.  Consider the following:\n\n    try:\n        fs \u003d vfs.VFS.instance_for_image(image, fmt, partition)\n        fs.setup()\n    except Exception as e:\n        for inject in mandatory:\n            inject_val \u003d locals()[inject]\n            if inject_val:\n                raise\n        LOG.warn(…)\n        return False\n\n    try:\n        return inject_data_into_fs(…)\n    finally:\n        fs.teardown()\n\nI also don\u0027t understand the purpose of the loop in the exception handler; could you go over what it is doing?","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"1d75987c4b8f60a0aa9b35304aa4c4ab1d802b83","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        finally:"},{"line_number":307,"context_line":"            fs.teardown()"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    return status"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"def setup_container(image, container_dir, use_cow\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2FcQg%3D","line":309,"in_reply_to":"AAAAM3%2F%2Fdas%3D","updated":"2013-01-22 14:52:23.000000000","message":"I prefer your equivalent without the status variable.\n\nThe loop is used to detect if a mandatory argument is passed,\nand raise an exception in that case. I\u0027ve added a comment to the code.","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"1d6aadba59ee17d2071cb8581d56ca2a12361d8e","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    status \u003d True"},{"line_number":352,"context_line":"    for inject in (\u0027key\u0027, \u0027net\u0027, \u0027metadata\u0027, \u0027admin_password\u0027, \u0027files\u0027):"},{"line_number":353,"context_line":"        inject_val \u003d locals()[inject]"},{"line_number":354,"context_line":"        inject_func \u003d globals()[\u0027_inject_%s_into_fs\u0027 % inject]"},{"line_number":355,"context_line":"        if inject_val:"},{"line_number":356,"context_line":"            try:"},{"line_number":357,"context_line":"                inject_func(inject_val, fs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2Fdac%3D","line":354,"updated":"2013-01-21 19:49:09.000000000","message":"Ugh.  Surely there\u0027s a better way of doing this?","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"1d75987c4b8f60a0aa9b35304aa4c4ab1d802b83","unresolved":false,"context_lines":[{"line_number":351,"context_line":"    status \u003d True"},{"line_number":352,"context_line":"    for inject in (\u0027key\u0027, \u0027net\u0027, \u0027metadata\u0027, \u0027admin_password\u0027, \u0027files\u0027):"},{"line_number":353,"context_line":"        inject_val \u003d locals()[inject]"},{"line_number":354,"context_line":"        inject_func \u003d globals()[\u0027_inject_%s_into_fs\u0027 % inject]"},{"line_number":355,"context_line":"        if inject_val:"},{"line_number":356,"context_line":"            try:"},{"line_number":357,"context_line":"                inject_func(inject_val, fs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAM3%2F%2FcQU%3D","line":354,"in_reply_to":"AAAAM3%2F%2Fdac%3D","updated":"2013-01-22 14:52:23.000000000","message":"Didn\u0027t think it was too bad myself to map the function names to the inject item","commit_id":"ed1a8da6c1a4cec55c12952772a6bdcb7a4589b9"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"8be33f62bc3d6541730e5ecda2df139a8240d88e","unresolved":false,"context_lines":[{"line_number":352,"context_line":"    status \u003d True"},{"line_number":353,"context_line":"    for inject in (\u0027key\u0027, \u0027net\u0027, \u0027metadata\u0027, \u0027admin_password\u0027, \u0027files\u0027):"},{"line_number":354,"context_line":"        inject_val \u003d locals()[inject]"},{"line_number":355,"context_line":"        inject_func \u003d globals()[\u0027_inject_%s_into_fs\u0027 % inject]"},{"line_number":356,"context_line":"        if inject_val:"},{"line_number":357,"context_line":"            try:"},{"line_number":358,"context_line":"                inject_func(inject_val, fs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAM3%2F%2Fb5A%3D","line":355,"updated":"2013-01-22 17:18:24.000000000","message":"Yeah, I can\u0027t point to any specific reason that this raises an odor in my nostrils, but it does.  I won\u0027t down-check the patch just for this, but I keep thinking there should be a better way of handling it.  One alternative that comes to mind would be to group all these _inject_*_into_fs() functions into a class, then using getattr() on that class, but I\u0027m not certain that would be better…","commit_id":"e91e6c07d9e34b79114ecac29b3669084e331f5a"}]}