)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"10000c3a3f547f30633b8e6c5d4a7c1abcc37499","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds a task for image conversion. It uses `qemu-img` to"},{"line_number":10,"context_line":"convert images. This tool has support for several image formats."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The current implementation converts images iff the operator has"},{"line_number":13,"context_line":"configured glance to do so. That is, the `convert_to_format` option has"},{"line_number":14,"context_line":"been set."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_1ac37b39","line":12,"updated":"2015-02-27 02:55:38.000000000","message":"NIT: :%s/iff/if/","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"9adbf678befeddf244ccb954c52e28de07b2fecd","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch adds a task for image conversion. It uses `qemu-img` to"},{"line_number":10,"context_line":"convert images. This tool has support for several image formats."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The current implementation converts images iff the operator has"},{"line_number":13,"context_line":"configured glance to do so. That is, the `convert_to_format` option has"},{"line_number":14,"context_line":"been set."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_78a9cd23","line":12,"in_reply_to":"ba7be1f8_1ac37b39","updated":"2015-02-27 12:51:14.000000000","message":"iff means \"if and only if\" https://en.wikipedia.org/wiki/If_and_only_if \n\nI\u0027ll extend this","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"10000c3a3f547f30633b8e6c5d4a7c1abcc37499","unresolved":false,"context_lines":[{"line_number":25,"context_line":"management in general, I\u0027ve decided to let the fix for a follow-up"},{"line_number":26,"context_line":"patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DocImpact"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Partially-implements blueprint: new-upload-workflow"},{"line_number":31,"context_line":"Partially-implements blueprint: basic-import-conversion"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_5adc1319","line":28,"updated":"2015-02-27 02:55:38.000000000","message":"APIImpact?\nSecurityImpact (calling external binary)?","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"9adbf678befeddf244ccb954c52e28de07b2fecd","unresolved":false,"context_lines":[{"line_number":25,"context_line":"management in general, I\u0027ve decided to let the fix for a follow-up"},{"line_number":26,"context_line":"patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DocImpact"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Partially-implements blueprint: new-upload-workflow"},{"line_number":31,"context_line":"Partially-implements blueprint: basic-import-conversion"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_f843fd69","line":28,"in_reply_to":"ba7be1f8_5adc1319","updated":"2015-02-27 12:51:14.000000000","message":"mmh, not sure about ApiImpact since we\u0027re not changing the api. I\u0027ll add the SecurityImpact, though.","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"6dd4071b7d1333b3b06685b157073ae2592a478f","unresolved":false,"context_lines":[{"line_number":22,"context_line":"enable/disable tasks."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Since both things mentioned in the previous paragraph affect the task"},{"line_number":25,"context_line":"management in general, I\u0027ve decided to let the fix for a follow-up"},{"line_number":26,"context_line":"patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"ba7be1f8_dd9a96c4","line":25,"updated":"2015-02-27 19:21:58.000000000","message":"nit: let -\u003e leave","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f20fae283aa92ffaba05c1ee1524cecffb4815a9","unresolved":false,"context_lines":[{"line_number":22,"context_line":"enable/disable tasks."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Since both things mentioned in the previous paragraph affect the task"},{"line_number":25,"context_line":"management in general, I\u0027ve decided to let the fix for a follow-up"},{"line_number":26,"context_line":"patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"ba7be1f8_e1384cf0","line":25,"in_reply_to":"ba7be1f8_dd9a96c4","updated":"2015-03-02 21:04:54.000000000","message":"thanks :)","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"c9b9b9dc2cea5ed842e25b47e35947588090cb72","unresolved":false,"context_lines":[{"line_number":22,"context_line":"enable/disable tasks."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Since both things mentioned in the previous paragraph affect the task"},{"line_number":25,"context_line":"management in general, I\u0027ve decided to let the fix for a follow-up"},{"line_number":26,"context_line":"patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"9a80dd14_73574bcb","line":25,"updated":"2015-03-10 17:03:34.000000000","message":"let -\u003e leave (as Joel Coffman had noted)","commit_id":"a7e66d6706acb483bbc7590d3b40c51968ee7ebe"}],"glance/async/flows/convert.py":[{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"7222634aed3ebaa4b269804f7c54df1438a3aff5","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"convert_task_opts \u003d ["},{"line_number":31,"context_line":"    cfg.StrOpt(\u0027convertion_format\u0027,"},{"line_number":32,"context_line":"               default\u003dNone,"},{"line_number":33,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":34,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_6cc7a164","line":31,"updated":"2015-02-26 00:13:36.000000000","message":"Spelling: \u0027conversion_format\u0027","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"b4215c579f8d48e62d9b6e59f9305b243b5c58de","unresolved":false,"context_lines":[{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"convert_task_opts \u003d ["},{"line_number":31,"context_line":"    cfg.StrOpt(\u0027convertion_format\u0027,"},{"line_number":32,"context_line":"               default\u003dNone,"},{"line_number":33,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":34,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_b282b9c5","line":31,"in_reply_to":"ba7be1f8_6cc7a164","updated":"2015-02-26 13:02:38.000000000","message":"(facepalm)","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"7222634aed3ebaa4b269804f7c54df1438a3aff5","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    cfg.StrOpt(\u0027convertion_format\u0027,"},{"line_number":32,"context_line":"               default\u003dNone,"},{"line_number":33,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":34,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""},{"line_number":35,"context_line":"                      \"Can be \u0027serial\u0027 or \u0027parallel\u0027.\")),"},{"line_number":36,"context_line":"]"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_6cac018d","line":34,"updated":"2015-02-26 00:13:36.000000000","message":"This help string seems to have been copied from somewhere else.  Suggestion: \"The format to which images will be automatically converted. \"\n\"Can be \u0027qcow2\u0027 or \u0027raw\u0027.\"","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"b4215c579f8d48e62d9b6e59f9305b243b5c58de","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    cfg.StrOpt(\u0027convertion_format\u0027,"},{"line_number":32,"context_line":"               default\u003dNone,"},{"line_number":33,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":34,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""},{"line_number":35,"context_line":"                      \"Can be \u0027serial\u0027 or \u0027parallel\u0027.\")),"},{"line_number":36,"context_line":"]"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_d2856dcf","line":34,"in_reply_to":"ba7be1f8_6cac018d","updated":"2015-02-26 13:02:38.000000000","message":"doh, u got me","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"7222634aed3ebaa4b269804f7c54df1438a3aff5","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        if convertion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        dest_path \u003d \"%s.converted\""},{"line_number":68,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":69,"context_line":"                                       convertion_format, file_path, dest_path,"},{"line_number":70,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_0c154548","line":67,"updated":"2015-02-26 00:13:36.000000000","message":"Can we check whether the image is already in the desired format, and skip the conversion process altogether if it is?","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"b4215c579f8d48e62d9b6e59f9305b243b5c58de","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        if convertion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        dest_path \u003d \"%s.converted\""},{"line_number":68,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":69,"context_line":"                                       convertion_format, file_path, dest_path,"},{"line_number":70,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_7234c1fb","line":67,"in_reply_to":"ba7be1f8_0c154548","updated":"2015-02-26 13:02:38.000000000","message":"Yes, we can and should. Since this is just the first step, I\u0027d prefer to just write a comment for now and make sure we do that in a follow-up patch (before the Kilo release). The reason is that we can split the remaining optimizations among other people willing to help here and, for now, just focus on the whole tasks interaction.","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"7222634aed3ebaa4b269804f7c54df1438a3aff5","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        dest_path \u003d \"%s.converted\""},{"line_number":68,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":69,"context_line":"                                       convertion_format, file_path, dest_path,"},{"line_number":70,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        if stderr:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_8ca4d54b","line":69,"updated":"2015-02-26 00:13:36.000000000","message":"Don\u0027t we need to tell \"someone\" about the dest_path, or rename dest_path to file_path so that the surrounding code can find the converted image in place of the original one?","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"b4215c579f8d48e62d9b6e59f9305b243b5c58de","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        dest_path \u003d \"%s.converted\""},{"line_number":68,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":69,"context_line":"                                       convertion_format, file_path, dest_path,"},{"line_number":70,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        if stderr:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba7be1f8_324de9c4","line":69,"in_reply_to":"ba7be1f8_8ca4d54b","updated":"2015-02-26 13:02:38.000000000","message":"yes and no. Yes we want, no we can\u0027t do it right now. The ImportToStore task should take care of this, pls refer to the patch this patch depends on for more info about the path of this implementation.","commit_id":"2e4273a24f5e7123c887adb8dcff8e5908faa792"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9ef8b33dbc9ccf8a1759adef74dbc85e5d4a6b85","unresolved":false,"context_lines":[{"line_number":38,"context_line":"CONF \u003d cfg.CONF"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# NOTE(flaper87): Registering under the taskflow_executor section"},{"line_number":41,"context_line":"# for now. It seems a waste to hae a whole section dedidcated to a"},{"line_number":42,"context_line":"# single task with a single option."},{"line_number":43,"context_line":"CONF.register_opts(convert_task_opts, group\u003d\u0027taskflow_executor\u0027)"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_ef6343db","line":41,"updated":"2015-02-27 07:27:53.000000000","message":"hae \u003e\u003e have?","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"10000c3a3f547f30633b8e6c5d4a7c1abcc37499","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # because the dest format may work differently depending"},{"line_number":62,"context_line":"        # on the environment OpenStack is running in."},{"line_number":63,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":64,"context_line":"        if conversion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # TODO(flaper87): Check whether the image is in the desired"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_7a1637a9","line":64,"updated":"2015-02-27 02:55:38.000000000","message":"Please log this @ WARN. The conversion should not be enabled if there is no target format set. Will cause lots of confusion if we just ignore.","commit_id":"df1a0566a2064a74d8b1e535cf96e3902831f0fa"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"6dd4071b7d1333b3b06685b157073ae2592a478f","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # because the dest format may work differently depending"},{"line_number":62,"context_line":"        # on the environment OpenStack is running in."},{"line_number":63,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":64,"context_line":"        if conversion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # TODO(flaper87): Check whether the image is in the desired"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_5d714676","line":64,"updated":"2015-02-27 19:21:58.000000000","message":"See Erno\u0027s comment on patch set 8 about logging. It seems like not setting the conversion format should be detected immediately rather than during execution.","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f20fae283aa92ffaba05c1ee1524cecffb4815a9","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # because the dest format may work differently depending"},{"line_number":62,"context_line":"        # on the environment OpenStack is running in."},{"line_number":63,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":64,"context_line":"        if conversion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # TODO(flaper87): Check whether the image is in the desired"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_ac756b99","line":64,"in_reply_to":"ba7be1f8_5d714676","updated":"2015-03-02 21:04:54.000000000","message":"Mind pointing me to the exact comment?\n\nI tend to agree with your comment about detecting this right away, however, it\u0027ll be a bit hard to do it right now due to the way tasks work.\n\nI\u0027ve a re-factor planned to some parts of the task implementation that will add these checks and also \"task execution requirements\"\n\nWhile I agree this is needed, I think we should do it in follow-up patches.","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"ba4af42ef83822a1017327256d0948d88826d3c6","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # because the dest format may work differently depending"},{"line_number":62,"context_line":"        # on the environment OpenStack is running in."},{"line_number":63,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":64,"context_line":"        if conversion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # TODO(flaper87): Check whether the image is in the desired"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_48067bcf","line":64,"in_reply_to":"ba7be1f8_ac756b99","updated":"2015-03-03 13:06:13.000000000","message":"Same line (number 64), just on patch set 8:\n\n  Please log this @ WARN. The conversion should not be enabled if there is no target format set. Will cause lots of confusion if we just ignore.","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6896,"name":"Loganathan Parthipan","email":"parthi@hpe.com","username":"parthipan"},"change_message_id":"c2e78d4fec9cb38943fcef017c01eae5d0fe3c05","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        # format already. Probably using `qemu-img` just like the"},{"line_number":69,"context_line":"        # `Introspection` task."},{"line_number":70,"context_line":"        dest_path \u003d \"%s.converted\""},{"line_number":71,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":72,"context_line":"                                       conversion_format, file_path, dest_path,"},{"line_number":73,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a80dd14_bf76e2d6","line":71,"updated":"2015-03-04 12:20:55.000000000","message":"There has to be some sanity test on the image metadata before one calls qemu-img this way. For instance the virtual size could be set very large and end up with a raw file (though sparse). This can then be used to claim real storage by copying into a volume and bring down a service node quite easily.","commit_id":"ec06b3653f471c4317eb1089983637d68974375d"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"c9b9b9dc2cea5ed842e25b47e35947588090cb72","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        # on the environment OpenStack is running in."},{"line_number":63,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":64,"context_line":"        if conversion_format is None:"},{"line_number":65,"context_line":"            return"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        # TODO(flaper87): Check whether the image is in the desired"},{"line_number":68,"context_line":"        # format already. Probably using `qemu-img` just like the"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a80dd14_139b6f73","line":65,"updated":"2015-03-10 17:03:34.000000000","message":"Erno Kuvaja suggested that this situation should be logged @WARN, because it should never occur.  I agree.","commit_id":"a7e66d6706acb483bbc7590d3b40c51968ee7ebe"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"376c1347c7748d5a5d6b0dd0302cbddf850d0ff1","unresolved":false,"context_lines":[{"line_number":33,"context_line":"               default\u003dNone,"},{"line_number":34,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":35,"context_line":"               help\u003d_(\"The format to which images will be automatically \""},{"line_number":36,"context_line":"                      \"converted. \" \"Can be \u0027qcow2\u0027 or \u0027raw\u0027.\")),"},{"line_number":37,"context_line":"]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_a3413a0d","line":36,"updated":"2015-03-11 17:35:42.000000000","message":"You can drop the double quotes in the middle here\n\n    \"converted. Can be \u0027qcow2\u0027 or \u0027raw\u0027.\"","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1baa4bae9520a3fe40a03109e18f174d94383041","unresolved":false,"context_lines":[{"line_number":33,"context_line":"               default\u003dNone,"},{"line_number":34,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":35,"context_line":"               help\u003d_(\"The format to which images will be automatically \""},{"line_number":36,"context_line":"                      \"converted. \" \"Can be \u0027qcow2\u0027 or \u0027raw\u0027.\")),"},{"line_number":37,"context_line":"]"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_e4da4e16","line":36,"in_reply_to":"9a80dd14_a3413a0d","updated":"2015-03-11 22:17:03.000000000","message":"Done","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"376c1347c7748d5a5d6b0dd0302cbddf850d0ff1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"CONF \u003d cfg.CONF"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# NOTE(flaper87): Registering under the taskflow_executor section"},{"line_number":42,"context_line":"# for now. It seems a waste to have a whole section dedidcated to a"},{"line_number":43,"context_line":"# single task with a single option."},{"line_number":44,"context_line":"CONF.register_opts(convert_task_opts, group\u003d\u0027taskflow_executor\u0027)"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_036da699","line":42,"updated":"2015-03-11 17:35:42.000000000","message":"I\u0027m not sure. Deprecating the option will be annoying otherwise.","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1baa4bae9520a3fe40a03109e18f174d94383041","unresolved":false,"context_lines":[{"line_number":39,"context_line":"CONF \u003d cfg.CONF"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"# NOTE(flaper87): Registering under the taskflow_executor section"},{"line_number":42,"context_line":"# for now. It seems a waste to have a whole section dedidcated to a"},{"line_number":43,"context_line":"# single task with a single option."},{"line_number":44,"context_line":"CONF.register_opts(convert_task_opts, group\u003d\u0027taskflow_executor\u0027)"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_a43e9627","line":42,"in_reply_to":"9a80dd14_036da699","updated":"2015-03-11 22:17:03.000000000","message":"not sure I understand what you mean. Do you think using the same group is not good?","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"376c1347c7748d5a5d6b0dd0302cbddf850d0ff1","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # on the environment OpenStack is running in."},{"line_number":62,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":63,"context_line":"        if conversion_format is None:"},{"line_number":64,"context_line":"            msg \u003d (_LW(\u0027The conversion format is None, please add a value \u0027"},{"line_number":65,"context_line":"                       \u0027for it in the config file for this task to work: %s\u0027) %"},{"line_number":66,"context_line":"                   self.task_id)"},{"line_number":67,"context_line":"            LOG.warn(msg)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_4315dee3","line":64,"updated":"2015-03-11 17:35:42.000000000","message":"Should we use Erno\u0027s trick here of only logging the warning once? I know that was for deprecation, but does it make sense to only warn someone once of this?","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1baa4bae9520a3fe40a03109e18f174d94383041","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        # on the environment OpenStack is running in."},{"line_number":62,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":63,"context_line":"        if conversion_format is None:"},{"line_number":64,"context_line":"            msg \u003d (_LW(\u0027The conversion format is None, please add a value \u0027"},{"line_number":65,"context_line":"                       \u0027for it in the config file for this task to work: %s\u0027) %"},{"line_number":66,"context_line":"                   self.task_id)"},{"line_number":67,"context_line":"            LOG.warn(msg)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a80dd14_e4c32e36","line":64,"in_reply_to":"9a80dd14_4315dee3","updated":"2015-03-11 22:17:03.000000000","message":"yes, lets use it.","commit_id":"1e09f803c0826d1baab73411665f3014c08b5ab7"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"bf6f1efc4af99f131bec036361d7262241e446a5","unresolved":false,"context_lines":[{"line_number":31,"context_line":"convert_task_opts \u003d ["},{"line_number":32,"context_line":"    cfg.StrOpt(\u0027conversion_format\u0027,"},{"line_number":33,"context_line":"               default\u003dNone,"},{"line_number":34,"context_line":"               choices\u003d(\u0027qcow2\u0027, \u0027raw\u0027),"},{"line_number":35,"context_line":"               help\u003d_(\"The format to which images will be automatically \""},{"line_number":36,"context_line":"                      \"converted. \" \"Can be \u0027qcow2\u0027 or \u0027raw\u0027.\")),"},{"line_number":37,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_2371b344","line":34,"updated":"2015-03-12 03:42:37.000000000","message":"I think we need to expand on choices here. Not sure if we should change later or here. Let\u0027s ask Rosmaita for his input on this.","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"bf6f1efc4af99f131bec036361d7262241e446a5","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        super(_Convert, self).__init__("},{"line_number":56,"context_line":"            name\u003d\u0027%s-Convert-%s\u0027 % (task_type, task_id))"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def execute(self, image_id, file_path):"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        # NOTE(flaper87): A format must be explicitly"},{"line_number":61,"context_line":"        # specified. There\u0027s no \"sane\" default for this"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_a3ae8396","line":58,"updated":"2015-03-12 03:42:37.000000000","message":"I think we should add a way for operators to specify their semaphores here. If the processing for their image conversion is expensive configuring appropriate simultaneous tasks might be helpful. Having config for the parallel workers helps however, that does not provide finer control on which tasks need to be run more simultaneously (less expensive ones) and the ones that should be locked and run less. \n\nHaving said that, this patch does not need to address this concern. I\u0027m just stating this here so that it does not get lost in context and looks like we\u0027re making a calculated review for moving ahead with the current approach.\n\nThanks!","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"bf6f1efc4af99f131bec036361d7262241e446a5","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":65,"context_line":"        if conversion_format is None:"},{"line_number":66,"context_line":"            if not _Convert.conversion_missing_warned:"},{"line_number":67,"context_line":"                msg \u003d (_LW(\u0027The conversion format is None, please add a value \u0027"},{"line_number":68,"context_line":"                           \u0027for it in the config file for this task to \u0027"},{"line_number":69,"context_line":"                           \u0027work: %s\u0027) %"},{"line_number":70,"context_line":"                       self.task_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_23bff3ab","line":67,"updated":"2015-03-12 03:42:37.000000000","message":"I see there are multiple concerns regarding logging here. The seem valid and I\u0027ve some more:\n\n1. We should log the task_id for which this is being run.\n\n2. We should add task_repo to the get_flow method below and suggest the status of the process on the task.result field once this is run (i.e. previous is completed) and once this is success or failed. On failure we set task.message so result seems more appropriate for success.","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"b148bbb3b76e6b4c1f4dbcb66d78ca13f5092e73","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":65,"context_line":"        if conversion_format is None:"},{"line_number":66,"context_line":"            if not _Convert.conversion_missing_warned:"},{"line_number":67,"context_line":"                msg \u003d (_LW(\u0027The conversion format is None, please add a value \u0027"},{"line_number":68,"context_line":"                           \u0027for it in the config file for this task to \u0027"},{"line_number":69,"context_line":"                           \u0027work: %s\u0027) %"},{"line_number":70,"context_line":"                       self.task_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_dab98d16","line":67,"in_reply_to":"9a80dd14_23bff3ab","updated":"2015-03-12 09:34:19.000000000","message":"1. I do not agree. This is global config option regardless the task (so this failure does not change between different conversion tasks but rather every conversion task fails if this fails) and will be logged only once. There is really no point to log the task ID.\n\n2. I do agree, we probably should let the user know somehow :P","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"982901c597bdb777abc6cba06e3f9c2804eda9e7","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        conversion_format \u003d CONF.taskflow_executor.conversion_format"},{"line_number":65,"context_line":"        if conversion_format is None:"},{"line_number":66,"context_line":"            if not _Convert.conversion_missing_warned:"},{"line_number":67,"context_line":"                msg \u003d (_LW(\u0027The conversion format is None, please add a value \u0027"},{"line_number":68,"context_line":"                           \u0027for it in the config file for this task to \u0027"},{"line_number":69,"context_line":"                           \u0027work: %s\u0027) %"},{"line_number":70,"context_line":"                       self.task_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_9a953ee6","line":67,"in_reply_to":"9a80dd14_dab98d16","updated":"2015-03-12 18:49:25.000000000","message":"Sorry, I just used a random line to comment that we need task_id in logs. There is no point in logs if the cmd qemu-img is going to be run on something and we have no clue which process is doing that.\n\nIn general, not having task_id in logs that pertain to the task process does not help much.","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"bf6f1efc4af99f131bec036361d7262241e446a5","unresolved":false,"context_lines":[{"line_number":79,"context_line":"        stdout, stderr \u003d putils.trycmd(\u0027qemu-img\u0027, \u0027convert\u0027, \u0027-O\u0027,"},{"line_number":80,"context_line":"                                       conversion_format, file_path, dest_path,"},{"line_number":81,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        if stderr:"},{"line_number":84,"context_line":"            raise RuntimeError(stderr)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_c3f7f78a","line":82,"updated":"2015-03-12 03:42:37.000000000","message":"Add update to task.result for more details on the processing status.","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"bf6f1efc4af99f131bec036361d7262241e446a5","unresolved":false,"context_lines":[{"line_number":80,"context_line":"                                       conversion_format, file_path, dest_path,"},{"line_number":81,"context_line":"                                       log_errors\u003dputils.LOG_ALL_ERRORS)"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        if stderr:"},{"line_number":84,"context_line":"            raise RuntimeError(stderr)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"9a80dd14_63e00bcd","line":83,"updated":"2015-03-12 03:42:37.000000000","message":"I think we should still have debug logging here. Something that indicates what happened and set the task status to failure.","commit_id":"4835eb494e66bfb26dd4b8a18bca163bf3366176"}],"glance/tests/unit/async/flows/test_convert.py":[{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"6dd4071b7d1333b3b06685b157073ae2592a478f","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                # the second one for the introspection one. The later *must*"},{"line_number":142,"context_line":"                # come after the former. If not, the current builtin flow"},{"line_number":143,"context_line":"                # process will be unsound."},{"line_number":144,"context_line":"                # Follow-up work will fix this by having a better way to hanlde"},{"line_number":145,"context_line":"                # task\u0027s dependencies and activation."},{"line_number":146,"context_line":"                exc_mock.side_effect \u003d [(\"\", None), (result, None)]"},{"line_number":147,"context_line":"                executor.begin_processing(self.task.task_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_bdeec2b2","line":144,"updated":"2015-02-27 19:21:58.000000000","message":"typo: hanlde -\u003e handle","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f20fae283aa92ffaba05c1ee1524cecffb4815a9","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                # the second one for the introspection one. The later *must*"},{"line_number":142,"context_line":"                # come after the former. If not, the current builtin flow"},{"line_number":143,"context_line":"                # process will be unsound."},{"line_number":144,"context_line":"                # Follow-up work will fix this by having a better way to hanlde"},{"line_number":145,"context_line":"                # task\u0027s dependencies and activation."},{"line_number":146,"context_line":"                exc_mock.side_effect \u003d [(\"\", None), (result, None)]"},{"line_number":147,"context_line":"                executor.begin_processing(self.task.task_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba7be1f8_ccefff1c","line":144,"in_reply_to":"ba7be1f8_bdeec2b2","updated":"2015-03-02 21:04:54.000000000","message":"danke","commit_id":"b4641a2f95128840a377bdcae7d71e2267b6b483"},{"author":{"_account_id":6908,"name":"Simon Leinen","email":"simon.leinen@gmail.com","username":"simon-leinen"},"change_message_id":"c9b9b9dc2cea5ed842e25b47e35947588090cb72","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                # the second one for the introspection one. The later *must*"},{"line_number":142,"context_line":"                # come after the former. If not, the current builtin flow"},{"line_number":143,"context_line":"                # process will be unsound."},{"line_number":144,"context_line":"                # Follow-up work will fix this by having a better way to hanlde"},{"line_number":145,"context_line":"                # task\u0027s dependencies and activation."},{"line_number":146,"context_line":"                exc_mock.side_effect \u003d [(\"\", None), (result, None)]"},{"line_number":147,"context_line":"                executor.begin_processing(self.task.task_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a80dd14_f3c25b33","line":144,"updated":"2015-03-10 17:03:34.000000000","message":"hanlde -\u003e handle (as per Joel Coffman\u0027s review)","commit_id":"a7e66d6706acb483bbc7590d3b40c51968ee7ebe"}]}