)]}'
{"rally/plugins/openstack/scenarios/vm/utils.py":[{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"236f54c381780ded67d22055f0003a1407bb0206","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        \"\"\""},{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_200dce37","line":115,"updated":"2016-06-06 17:58:25.000000000","message":"Why .get() or [] instead of .get(\"interpreter\", [])? Is there some case where someone deliberately passes in interpreter\u003dNone that we have to work around?","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"c083ff4b4ba373a112736088b85d2d6669ff99aa","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        \"\"\""},{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_1fb1f347","line":115,"in_reply_to":"7aa08908_200dce37","updated":"2016-06-07 09:22:25.000000000","message":"`.get(\"interpreter\") or []\u0027 is better because someone can put `\"interpreter\": null\u0027 or `\"interpreter\": {}\u0027 or `\"interpreter\": 0\u0027 and this will cause error\n\nWe do not have jsonschema validation for command. Also validation.py:L180 uses same logic - it chacks boolean value of get(\"interpreter\")\n\nSo option `.get(\"interpreter\") or []\u0027 wins","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"236f54c381780ded67d22055f0003a1407bb0206","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"},{"line_number":119,"context_line":"            cmd.extend(interpreter)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_952b2694","line":116,"updated":"2016-06-06 17:58:25.000000000","message":"We should at least check for tuple here too, similar to the original code. I know all of our current invocations of this are taking their data from JSON, but there\u0027s nothing to stop end users from writing their own scenarios that might pass interpreter as a tuple and that would lead to a very unintuitive failure mode.","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"58c0b34a3956b288f6d2a0044ae3a0a3983cc6ad","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"},{"line_number":119,"context_line":"            cmd.extend(interpreter)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_501f8cb6","line":116,"in_reply_to":"7aa08908_05d7c1ed","updated":"2016-06-14 09:18:55.000000000","message":"Since users can pass unexpected types (like tuple), it\u0027s even better to explicitly check if interpreter is string type. I will do this","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"c3f1d94f8b026977180768db43e63c15e4788de1","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"},{"line_number":119,"context_line":"            cmd.extend(interpreter)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_05d7c1ed","line":116,"in_reply_to":"7aa08908_7fbe37d5","updated":"2016-06-07 16:46:04.000000000","message":"I agree entirely, I just don\u0027t want to break existing custom plugins that may already be out there in the wild.","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"c083ff4b4ba373a112736088b85d2d6669ff99aa","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        cmd, stdin \u003d [], None"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        interpreter \u003d command.get(\"interpreter\") or []"},{"line_number":116,"context_line":"        if type(interpreter) !\u003d list:"},{"line_number":117,"context_line":"            interpreter \u003d [interpreter]"},{"line_number":118,"context_line":"        if interpreter:"},{"line_number":119,"context_line":"            cmd.extend(interpreter)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_7fbe37d5","line":116,"in_reply_to":"7aa08908_952b2694","updated":"2016-06-07 09:22:25.000000000","message":"All this command-related stuff is buggy, I would suggest to rework it completely and replace validation part (from task.validation) and this piece of code by ResourceType plugin which would process (and verify) `command\u0027 argument structure, in single place, single time before iterations actually start.\n\nBut here we have at least a bug which should be fixed, without great re-working.\n\nThat is why I do not make significant changes here, just bugfix + improvement to make actual code a bit better","commit_id":"7eff8b7612ba232e559e76293ca055c6044fd976"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"f3e2dd88185fe43e8617778c6ff34844e5908a78","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                interpreter \u003d [interpreter]"},{"line_number":119,"context_line":"            elif type(interpreter) !\u003d list:"},{"line_number":120,"context_line":"                raise ValueError(\"command \u0027interpreter\u0027 value must be str \""},{"line_number":121,"context_line":"                                 \"or list type\")"},{"line_number":122,"context_line":"            cmd.extend(interpreter)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        remote_path \u003d command.get(\"remote_path\") or []"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_e18da57c","line":121,"range":{"start_line":121,"start_character":42,"end_line":121,"end_character":46},"updated":"2016-06-22 09:55:06.000000000","message":"I would be more exact here \u0027list of strings\u0027.\nI\u0027m not sure that we need to check whether the list only contains strings though.","commit_id":"c1e4d626787005399d9bc89f56cd10eb29f0bfcb"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"f3e2dd88185fe43e8617778c6ff34844e5908a78","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                 \"or list type\")"},{"line_number":122,"context_line":"            cmd.extend(interpreter)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        remote_path \u003d command.get(\"remote_path\") or []"},{"line_number":125,"context_line":"        if remote_path:"},{"line_number":126,"context_line":"            if isinstance(remote_path, six.string_types):"},{"line_number":127,"context_line":"                remote_path \u003d [remote_path]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_21d5ed42","line":124,"range":{"start_line":124,"start_character":35,"end_line":124,"end_character":46},"updated":"2016-06-22 09:55:06.000000000","message":"as for \u0027remote_path\u0027 I\u0027m pretty sure this should only be a string, not a list","commit_id":"c1e4d626787005399d9bc89f56cd10eb29f0bfcb"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"b76f72594b53f611e2aea772e71c788cae83edac","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                 \"or list type\")"},{"line_number":122,"context_line":"            cmd.extend(interpreter)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        remote_path \u003d command.get(\"remote_path\") or []"},{"line_number":125,"context_line":"        if remote_path:"},{"line_number":126,"context_line":"            if isinstance(remote_path, six.string_types):"},{"line_number":127,"context_line":"                remote_path \u003d [remote_path]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_fc1d4a67","line":124,"range":{"start_line":124,"start_character":35,"end_line":124,"end_character":46},"in_reply_to":"3aaa91ec_21d5ed42","updated":"2016-06-22 10:04:48.000000000","message":"We support both variants and list is even better because of L131. It is better to use only list in opposite to only str","commit_id":"c1e4d626787005399d9bc89f56cd10eb29f0bfcb"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"9d04ab0d67d6d8b060853ae241429611b1e0c5f7","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                 \"or list type\")"},{"line_number":122,"context_line":"            cmd.extend(interpreter)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        remote_path \u003d command.get(\"remote_path\") or []"},{"line_number":125,"context_line":"        if remote_path:"},{"line_number":126,"context_line":"            if isinstance(remote_path, six.string_types):"},{"line_number":127,"context_line":"                remote_path \u003d [remote_path]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_60616c39","line":124,"range":{"start_line":124,"start_character":35,"end_line":124,"end_character":46},"in_reply_to":"3aaa91ec_21d5ed42","updated":"2016-06-23 11:28:46.000000000","message":"Your comment sounds reasonable, however this change (do not use list for remote_path) will break backward compatibility\n\nhttps://github.com/openstack/rally/blob/master/tests/unit/plugins/openstack/scenarios/vm/test_utils.py#L73\n\nhttps://github.com/openstack/rally/blob/master/tests/unit/plugins/openstack/scenarios/vm/test_utils.py#L87\n\nIdeas?","commit_id":"c1e4d626787005399d9bc89f56cd10eb29f0bfcb"},{"author":{"_account_id":8491,"name":"Illia Khudoshyn","email":"ikhudoshyn@mirantis.com","username":"ikhudoshyn"},"change_message_id":"83482c5ea5205498238da0941649385c6f6af1cf","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                 \"or list type\")"},{"line_number":122,"context_line":"            cmd.extend(interpreter)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        remote_path \u003d command.get(\"remote_path\") or []"},{"line_number":125,"context_line":"        if remote_path:"},{"line_number":126,"context_line":"            if isinstance(remote_path, six.string_types):"},{"line_number":127,"context_line":"                remote_path \u003d [remote_path]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_20ac8433","line":124,"range":{"start_line":124,"start_character":35,"end_line":124,"end_character":46},"in_reply_to":"3aaa91ec_60616c39","updated":"2016-06-23 11:32:02.000000000","message":"Yep, I missed that it\u0027s that old. So lets keep for a while.","commit_id":"c1e4d626787005399d9bc89f56cd10eb29f0bfcb"}],"rally/task/validation.py":[{"author":{"_account_id":12712,"name":"Gleb Stepanov","email":"gstepanov@mirantis.com","username":"stgleb"},"change_message_id":"8b52a7b292b1c348e8303b1b043660e49c1157b3","unresolved":false,"context_lines":[{"line_number":171,"context_line":"def check_command_dict(command):"},{"line_number":172,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    if type(command) !\u003d dict:"},{"line_number":175,"context_line":"        raise ValueError(\"Command must be a dictionary\")"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    # NOTE(pboldin): Here we check for the values not for presence of the keys"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_fd49e18f","line":174,"updated":"2016-07-18 09:26:26.000000000","message":"Why Value Error is better then Attribute Error?","commit_id":"0b51158f732f43648da7ee605ef7e54781ceeec4"},{"author":{"_account_id":10475,"name":"Alexander Maretskiy","email":"amaretskiy@mirantis.com","username":"maretskiy"},"change_message_id":"c5b0080afbf394e042262ed1ca605a595b33ecb9","unresolved":false,"context_lines":[{"line_number":171,"context_line":"def check_command_dict(command):"},{"line_number":172,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    if type(command) !\u003d dict:"},{"line_number":175,"context_line":"        raise ValueError(\"Command must be a dictionary\")"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    # NOTE(pboldin): Here we check for the values not for presence of the keys"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_9ba05145","line":174,"in_reply_to":"dada55a8_fd49e18f","updated":"2016-07-19 19:34:07.000000000","message":"There is no attribute-related actions here, so AttributeError is wrong choice","commit_id":"0b51158f732f43648da7ee605ef7e54781ceeec4"}]}