)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"31b025f7c517940aedf3735062fe5524cf84ff88","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Introduce command-specifying dictionary"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Refactor `vm.utils._run_command_over_ssh\u0027 introducing dictionary"},{"line_number":10,"context_line":"to specify command as either local script file, inline script"},{"line_number":11,"context_line":"or remote script path. Both local and inline scripts require"},{"line_number":12,"context_line":"specifying interpreter.  Any of the above can be a list specifying"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a0c5dc1_b531e2a4","line":9,"updated":"2015-05-12 21:54:26.000000000","message":"making it accept a dict","commit_id":"fd990c5f40c395fa0db13a4572109ab5d46c12a2"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"31b025f7c517940aedf3735062fe5524cf84ff88","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Refactor `vm.utils._run_command_over_ssh\u0027 introducing dictionary"},{"line_number":10,"context_line":"to specify command as either local script file, inline script"},{"line_number":11,"context_line":"or remote script path. Both local and inline scripts require"},{"line_number":12,"context_line":"specifying interpreter.  Any of the above can be a list specifying"},{"line_number":13,"context_line":"e.g. environment variables or script or command args"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I3c495297251c87529b9adf1e769cfe277338f5fc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a0c5dc1_35e5f208","line":12,"updated":"2015-05-12 21:54:26.000000000","message":"require `interpreter\u0027 parameter","commit_id":"fd990c5f40c395fa0db13a4572109ab5d46c12a2"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"31b025f7c517940aedf3735062fe5524cf84ff88","unresolved":false,"context_lines":[{"line_number":10,"context_line":"to specify command as either local script file, inline script"},{"line_number":11,"context_line":"or remote script path. Both local and inline scripts require"},{"line_number":12,"context_line":"specifying interpreter.  Any of the above can be a list specifying"},{"line_number":13,"context_line":"e.g. environment variables or script or command args"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I3c495297251c87529b9adf1e769cfe277338f5fc"},{"line_number":16,"context_line":"Implements: blueprint vm-workloads-framework"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a0c5dc1_f5c51a5b","line":13,"updated":"2015-05-12 21:54:26.000000000","message":"This can be used e.g. to specify environment variagbles or script/command args.","commit_id":"fd990c5f40c395fa0db13a4572109ab5d46c12a2"}],"rally/benchmark/scenarios/vm/utils.py":[{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"e9fbba0dfe3854f0571448391c18b6463ca22732","unresolved":false,"context_lines":[{"line_number":96,"context_line":"                \"An `interpreter\u0027 is required for both script_file and\""},{"line_number":97,"context_line":"                \" script_inline\")"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        if \"script_file\" in command or \"script_inline\" in command:"},{"line_number":100,"context_line":"            cmd \u003d command[\"interpreter\"]"},{"line_number":101,"context_line":"            if \"script_file\" in command:"},{"line_number":102,"context_line":"                stdin \u003d open(command[\"script_file\"], \"rb\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_d15b832d","line":99,"updated":"2015-04-27 17:33:07.000000000","message":"Use \u0027xor\u0027 here.\n\nUse \u0027get\u0027 here and below.","commit_id":"fd990c5f40c395fa0db13a4572109ab5d46c12a2"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"e9fbba0dfe3854f0571448391c18b6463ca22732","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                stdin \u003d six.moves.StringIO(command[\"script_inline\"])"},{"line_number":105,"context_line":"        elif \"remote_path\" in command:"},{"line_number":106,"context_line":"            cmd \u003d command[\"remote_path\"]"},{"line_number":107,"context_line":"            stdin \u003d None"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        return ssh.execute(cmd, stdin\u003dstdin)"},{"line_number":110,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_d17263ad","line":107,"updated":"2015-04-27 17:33:07.000000000","message":"Write the above check (and raise ScriptError) as else clause.","commit_id":"fd990c5f40c395fa0db13a4572109ab5d46c12a2"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"ef7666ae919b20825f2ac34aa9fbcdb15f29b71d","unresolved":false,"context_lines":[{"line_number":74,"context_line":""},{"line_number":75,"context_line":"                # Run a remote command"},{"line_number":76,"context_line":"                command \u003d {"},{"line_number":77,"context_line":"                    \"remote_path\": \"/bin/false\""},{"line_number":78,"context_line":"                }"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"                # Run an inline script sending it to a remote interpreter"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_fb962c93","line":77,"updated":"2015-05-14 23:01:49.000000000","message":"why not just remote_command ?","commit_id":"9fbb198c722f12a49ab79b3c624fb2f9de9afc1e"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"ef7666ae919b20825f2ac34aa9fbcdb15f29b71d","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        if ((command.get(\"script_file\") or command.get(\"script_inline\")) and"},{"line_number":94,"context_line":"           \"interpreter\" not in command):"},{"line_number":95,"context_line":"            raise exceptions.ScriptError("},{"line_number":96,"context_line":"                \"An `interpreter\u0027 is required for both script_file and\""},{"line_number":97,"context_line":"                \" script_inline\")"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        if \"script_file\" in command or \"script_inline\" in command:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_5b6300b2","line":96,"updated":"2015-05-14 23:01:49.000000000","message":"It makes sense to pass here \"command\" dict values otherwise it will be hard to debug.","commit_id":"9fbb198c722f12a49ab79b3c624fb2f9de9afc1e"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"ef7666ae919b20825f2ac34aa9fbcdb15f29b71d","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    @utils.log_deprecated_args(\"Use `command\u0027 argument instead\", \"0.0.5\","},{"line_number":178,"context_line":"                               (\"script\", \"interpreter\"))"},{"line_number":179,"context_line":"    def _run_command(self, server_ip, port, username, password,"},{"line_number":180,"context_line":"                     interpreter\u003dNone, script\u003dNone, command\u003dNone, pkey\u003dNone):"},{"line_number":181,"context_line":"        \"\"\"Run command via SSH on server."},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a016987_9b9cd86a","line":179,"updated":"2015-05-14 23:01:49.000000000","message":"This is internal method of utils we don\u0027t need to keep backward compatibility. Let\u0027s make code cleaner","commit_id":"9fbb198c722f12a49ab79b3c624fb2f9de9afc1e"}],"rally/benchmark/validation.py":[{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"08d8cfd90a2ee13bbd3b03238344244372025cc3","unresolved":false,"context_lines":[{"line_number":168,"context_line":"def _check_command_dict(command):"},{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_b114f79f","line":171,"updated":"2015-05-19 14:05:50.000000000","message":"This looks a bit ugly to me, why not just:\n\n    if len(set(command.keys()) \u0026\n           set((\"remote_path\", \"script_file\", \"script_inline\"))) !\u003d 1:","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"1b4d728faad372208bf19e9369e754bbfab9d3a6","unresolved":false,"context_lines":[{"line_number":168,"context_line":"def _check_command_dict(command):"},{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_688e62d1","line":171,"in_reply_to":"7a016987_0e0cafeb","updated":"2015-05-21 10:55:23.000000000","message":"Okay, then this seems to be the simplest implementation indeed.","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"7ce03efbda96a23955512c625b71c8e99c83a692","unresolved":false,"context_lines":[{"line_number":168,"context_line":"def _check_command_dict(command):"},{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_0e0cafeb","line":171,"in_reply_to":"7a016987_b114f79f","updated":"2015-05-20 13:00:02.000000000","message":"Because reasons. But it is left as an exercise for the reviewer (hint: there is a NOTE in the patch that states exactly *why*)","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"08d8cfd90a2ee13bbd3b03238344244372025cc3","unresolved":false,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"@validator"},{"line_number":184,"context_line":"def valid_command(config, clients, deployment, param_name, required\u003dTrue):"},{"line_number":185,"context_line":"    \"\"\"Validator checks parameter is a proper command-specifying dictionary."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    Ensure that the command dictionary is either specifies remote command path"},{"line_number":188,"context_line":"    via `remote_path\u0027, an inline script via `script_inline\u0027 or a local script"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_7181dfc2","line":185,"updated":"2015-05-19 14:05:50.000000000","message":"checks THAT its parameter is ...","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"7ce03efbda96a23955512c625b71c8e99c83a692","unresolved":false,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"@validator"},{"line_number":184,"context_line":"def valid_command(config, clients, deployment, param_name, required\u003dTrue):"},{"line_number":185,"context_line":"    \"\"\"Validator checks parameter is a proper command-specifying dictionary."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    Ensure that the command dictionary is either specifies remote command path"},{"line_number":188,"context_line":"    via `remote_path\u0027, an inline script via `script_inline\u0027 or a local script"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_6e9d6365","line":185,"in_reply_to":"7a016987_7181dfc2","updated":"2015-05-20 13:00:02.000000000","message":"Done","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"08d8cfd90a2ee13bbd3b03238344244372025cc3","unresolved":false,"context_lines":[{"line_number":184,"context_line":"def valid_command(config, clients, deployment, param_name, required\u003dTrue):"},{"line_number":185,"context_line":"    \"\"\"Validator checks parameter is a proper command-specifying dictionary."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    Ensure that the command dictionary is either specifies remote command path"},{"line_number":188,"context_line":"    via `remote_path\u0027, an inline script via `script_inline\u0027 or a local script"},{"line_number":189,"context_line":"    file path using `script_file\u0027. The latter is checked to be accessible"},{"line_number":190,"context_line":"    like in `file_exists\u0027 validator."}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_b162b73c","line":187,"updated":"2015-05-19 14:05:50.000000000","message":"... dictionary either ... (please remove \"is\")","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"7ce03efbda96a23955512c625b71c8e99c83a692","unresolved":false,"context_lines":[{"line_number":184,"context_line":"def valid_command(config, clients, deployment, param_name, required\u003dTrue):"},{"line_number":185,"context_line":"    \"\"\"Validator checks parameter is a proper command-specifying dictionary."},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    Ensure that the command dictionary is either specifies remote command path"},{"line_number":188,"context_line":"    via `remote_path\u0027, an inline script via `script_inline\u0027 or a local script"},{"line_number":189,"context_line":"    file path using `script_file\u0027. The latter is checked to be accessible"},{"line_number":190,"context_line":"    like in `file_exists\u0027 validator."}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_8e941f43","line":187,"in_reply_to":"7a016987_b162b73c","updated":"2015-05-20 13:00:02.000000000","message":"Done","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"08d8cfd90a2ee13bbd3b03238344244372025cc3","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        command \u003d {"},{"line_number":219,"context_line":"            \"script_inline\": \"echo \\\"Hello, ${NAME:-World}\\\"\","},{"line_number":220,"context_line":"            \"interpreter\": [\"NAME\u003dEarth\", \"/bin/sh\"]"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    :param param_name: Name of parameter to validate"},{"line_number":224,"context_line":"    :param required: Boolean indicating that the command dictionary required"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_7158bf8a","line":221,"updated":"2015-05-19 14:05:50.000000000","message":"Maybe also add an example of an incorrect command dict?","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"7ce03efbda96a23955512c625b71c8e99c83a692","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        command \u003d {"},{"line_number":219,"context_line":"            \"script_inline\": \"echo \\\"Hello, ${NAME:-World}\\\"\","},{"line_number":220,"context_line":"            \"interpreter\": [\"NAME\u003dEarth\", \"/bin/sh\"]"},{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    :param param_name: Name of parameter to validate"},{"line_number":224,"context_line":"    :param required: Boolean indicating that the command dictionary required"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_2e8acb1b","line":221,"in_reply_to":"7a016987_7158bf8a","updated":"2015-05-20 13:00:02.000000000","message":"Nope.","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":8507,"name":"Mikhail Dubov","email":"mdubov@mirantis.com","username":"msdubov"},"change_message_id":"08d8cfd90a2ee13bbd3b03238344244372025cc3","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    :param param_name: Name of parameter to validate"},{"line_number":224,"context_line":"    :param required: Boolean indicating that the command dictionary required"},{"line_number":225,"context_line":"    \"\"\""},{"line_number":226,"context_line":"    command \u003d config.get(\"args\", {}).get(param_name)"},{"line_number":227,"context_line":"    if command is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_1154eb50","line":224,"updated":"2015-05-19 14:05:50.000000000","message":"... IS required","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"7ce03efbda96a23955512c625b71c8e99c83a692","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        }"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    :param param_name: Name of parameter to validate"},{"line_number":224,"context_line":"    :param required: Boolean indicating that the command dictionary required"},{"line_number":225,"context_line":"    \"\"\""},{"line_number":226,"context_line":"    command \u003d config.get(\"args\", {}).get(param_name)"},{"line_number":227,"context_line":"    if command is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7a016987_4e9aa76f","line":224,"in_reply_to":"7a016987_1154eb50","updated":"2015-05-20 13:00:02.000000000","message":"Done","commit_id":"b164db8929f3cdd4bbbef96dd47532828053e8a9"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""},{"line_number":175,"context_line":"            \" is expected: %r\" % command)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_1926bb16","line":172,"updated":"2015-05-26 18:01:03.000000000","message":"This might be more elegantly written as:\n\n    if len(set(command.keys()) \u0026 set([\"remote_path\", \"script_file\", \"script_inline\"])) !\u003d 1:\n\nOr maybe that\u0027s not more elegant.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"ccb7c754689c56d453e2c812e3f8a541429967a1","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""},{"line_number":175,"context_line":"            \" is expected: %r\" % command)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_449d6658","line":172,"in_reply_to":"7a016987_1926bb16","updated":"2015-05-26 18:09:42.000000000","message":"It was asked before, please look through the comments to learn why this is not the case (hint: there is a NOTE comment in this patch).","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"e28d565954345f80ccac17c61ba8c148178db3ac","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""},{"line_number":175,"context_line":"            \" is expected: %r\" % command)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_5f11b35c","line":172,"in_reply_to":"7a016987_3f5b9730","updated":"2015-05-26 18:46:26.000000000","message":"Yes, going to do it here. It is obvious that people are unable to read through the code. Not to say it is impossible for them to look forward for usage of template-created configurations.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"6e3068495c86199035e093ae89c04455eb512efa","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""},{"line_number":175,"context_line":"            \" is expected: %r\" % command)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_3f5b9730","line":172,"in_reply_to":"7a016987_449d6658","updated":"2015-05-26 18:23:29.000000000","message":"Please add a similar note here. Figuring out why inelegant code is inelegant should not be left as an exercise for the reader.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"},{"line_number":172,"context_line":"           if command.get(k)]) !\u003d 1:"},{"line_number":173,"context_line":"        raise ValueError("},{"line_number":174,"context_line":"            \"Only exactly one of script_inline, script_file or remote_path\""},{"line_number":175,"context_line":"            \" is expected: %r\" % command)"},{"line_number":176,"context_line":"    if ((command.get(\"script_file\") or command.get(\"script_inline\")) and"},{"line_number":177,"context_line":"       \"interpreter\" not in command):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_2496ea94","line":174,"updated":"2015-05-26 18:01:03.000000000","message":"The \"Only\" here is extraneous: \"Exactly one of ...\"","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"20c48364030749010816aa34fcd785e917d7e29a","unresolved":false,"context_lines":[{"line_number":168,"context_line":"def check_command_dict(command):"},{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    # NOTE(pboldin): Here we check for the values not for presence of the keys"},{"line_number":172,"context_line":"    # due to template-driven configuration generation that can leave keys"},{"line_number":173,"context_line":"    # defined but values empty."},{"line_number":174,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_ba05e011","line":171,"updated":"2015-05-29 10:08:13.000000000","message":"I would strongly recommend to use here jsonschema. \n\nit will be much simpler to read and you won\u0027t need such large comment in valid_command() because it will be obvious from JSONCHEMA","commit_id":"6121cfc0b7058df6117dff733e9441f3fa0ddfd2"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"d6d5d15d19229f496f484fcc37fc5d46ea38143f","unresolved":false,"context_lines":[{"line_number":168,"context_line":"def check_command_dict(command):"},{"line_number":169,"context_line":"    \"\"\"Check command-specifying dict `command\u0027, raise ValueError on error.\"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    # NOTE(pboldin): Here we check for the values not for presence of the keys"},{"line_number":172,"context_line":"    # due to template-driven configuration generation that can leave keys"},{"line_number":173,"context_line":"    # defined but values empty."},{"line_number":174,"context_line":"    if len([1 for k in (\"remote_path\", \"script_file\", \"script_inline\")"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_ba84bdd2","line":171,"in_reply_to":"7a016987_ba05e011","updated":"2015-05-29 10:39:05.000000000","message":"No, it won\u0027t be obvious, because the schema is complicated as such.\n\nPlease take a look at the JSONSchema here:\nhttps://review.openstack.org/#/c/183227/8/rally/plugins/openstack/context/vm/command_customizer.py,cm\n\nFurthermore, the scheme is unable to check against empty values with the keys defined that are product of the template-driver configuration generation.","commit_id":"6121cfc0b7058df6117dff733e9441f3fa0ddfd2"}],"rally/plugins/openstack/scenarios/vm/utils.py":[{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        :returns: tuple (exit_status, stdout, stderr)"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":"        validation._check_command_dict(command)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # NOTE(pboldin): Here we use \u0027get\u0027 since the command-dict can be build"},{"line_number":54,"context_line":"        # using template mechanism leaving keys specified but values empty"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_b9f8c74a","line":51,"updated":"2015-05-26 18:01:03.000000000","message":"1) Why is this called here instead of using the @valid_command decorator on the scenario?\n\n2) If this is the (or an) intended use, why is _check_command_dict() private?","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"ccb7c754689c56d453e2c812e3f8a541429967a1","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        :returns: tuple (exit_status, stdout, stderr)"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":"        validation._check_command_dict(command)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # NOTE(pboldin): Here we use \u0027get\u0027 since the command-dict can be build"},{"line_number":54,"context_line":"        # using template mechanism leaving keys specified but values empty"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_c4d716ec","line":51,"in_reply_to":"7a016987_b9f8c74a","updated":"2015-05-26 18:09:42.000000000","message":"1) Because it is not only scenarios that are able to call _run_command_over_ssh.\n\n2) There is no such a thing as \u0027private\u0027 in Python.\n As you can see most of these methods are declared with \u0027_\u0027 but most of them are called outside their class.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"6e3068495c86199035e093ae89c04455eb512efa","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        :returns: tuple (exit_status, stdout, stderr)"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":"        validation._check_command_dict(command)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # NOTE(pboldin): Here we use \u0027get\u0027 since the command-dict can be build"},{"line_number":54,"context_line":"        # using template mechanism leaving keys specified but values empty"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_df8fe373","line":51,"in_reply_to":"7a016987_c4d716ec","updated":"2015-05-26 18:23:29.000000000","message":"There is private by convention. And, since all but one method in this class is an instance method, I can safely state that they are only ever called by instances of this class or of child classes. Please don\u0027t violate Python conventions without a good reason.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"e28d565954345f80ccac17c61ba8c148178db3ac","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        :returns: tuple (exit_status, stdout, stderr)"},{"line_number":50,"context_line":"        \"\"\""},{"line_number":51,"context_line":"        validation._check_command_dict(command)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # NOTE(pboldin): Here we use \u0027get\u0027 since the command-dict can be build"},{"line_number":54,"context_line":"        # using template mechanism leaving keys specified but values empty"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_316d0bcd","line":51,"in_reply_to":"7a016987_df8fe373","updated":"2015-05-26 18:46:26.000000000","message":"Your assumption is wrong and your abstractions are leaky. Sorry for your believe in the conventions.\n\n $ git grep -P \u0027(?\u003c!self)\\._[a-zA-Z].*?\\(\u0027 rally | wc -l\n49\n\nBut I think I need to fix this in my code at least.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"20c48364030749010816aa34fcd785e917d7e29a","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from rally.benchmark.scenarios import base"},{"line_number":23,"context_line":"from rally.benchmark import utils as bench_utils"},{"line_number":24,"context_line":"from rally.benchmark import validation"},{"line_number":25,"context_line":"from rally.benchmark.wrappers import network as network_wrapper"},{"line_number":26,"context_line":"from rally.common.i18n import _"},{"line_number":27,"context_line":"from rally.common import log as logging"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_3af7b08c","line":24,"updated":"2015-05-29 10:08:13.000000000","message":"make please TODO for me, \n\nSo I won\u0027t forget to refactor this place after finishing new validation mechanism that will just allow to use validator directly","commit_id":"6121cfc0b7058df6117dff733e9441f3fa0ddfd2"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"20c48364030749010816aa34fcd785e917d7e29a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        validation.check_command_dict(command)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # NOTE(pboldin): Here we use \u0027get\u0027 since the command-dict can be build"},{"line_number":54,"context_line":"        # using template mechanism leaving keys specified but values empty"},{"line_number":55,"context_line":"        if command.get(\"script_file\") or command.get(\"script_inline\"):"},{"line_number":56,"context_line":"            cmd \u003d command[\"interpreter\"]"},{"line_number":57,"context_line":"            if command.get(\"script_file\"):"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a016987_706f87fc","line":54,"updated":"2015-05-29 10:08:13.000000000","message":"template mechanism? note is unclear for me...","commit_id":"6121cfc0b7058df6117dff733e9441f3fa0ddfd2"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"c82529aaf345c0c6e6a37ac1303c8da553ce6b17","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        :returns: tuple (exit_status, stdout, stderr)"},{"line_number":53,"context_line":"        \"\"\""},{"line_number":54,"context_line":"        validation.check_command_dict(command)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        # NOTE(pboldin): Here we `get\u0027 the values and not check for the keys"},{"line_number":57,"context_line":"        # due to template-driven configuration generation that can leave keys"}],"source_content_type":"text/x-python","patch_set":16,"id":"3afb71cf_6d9a4665","line":54,"updated":"2015-06-08 18:29:50.000000000","message":"Maybe it\u0027s better to override this method? \n\nhttps://github.com/openstack/rally/blob/master/rally/benchmark/context.py#L86-L90\n\nso this will be checked, before actually running task (not in the middle of process?)","commit_id":"0c4de04da0b0c743444f6a093b4cf77811890494"}],"rally/plugins/openstack/scenarios/vm/vmtasks.py":[{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                               floating_network\u003dNone,"},{"line_number":51,"context_line":"                               port\u003d22,"},{"line_number":52,"context_line":"                               use_floating_ip\u003dTrue,"},{"line_number":53,"context_line":"                               force_delete\u003dFalse,"},{"line_number":54,"context_line":"                               **kwargs):"},{"line_number":55,"context_line":"        \"\"\"Boot a server, run a script that outputs JSON, delete the server."},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_24af0ae5","line":53,"updated":"2015-05-26 18:01:03.000000000","message":"Are there plans to make this scenario accept a new command dict argument?","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"ccb7c754689c56d453e2c812e3f8a541429967a1","unresolved":false,"context_lines":[{"line_number":50,"context_line":"                               floating_network\u003dNone,"},{"line_number":51,"context_line":"                               port\u003d22,"},{"line_number":52,"context_line":"                               use_floating_ip\u003dTrue,"},{"line_number":53,"context_line":"                               force_delete\u003dFalse,"},{"line_number":54,"context_line":"                               **kwargs):"},{"line_number":55,"context_line":"        \"\"\"Boot a server, run a script that outputs JSON, delete the server."},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_64fd026e","line":53,"in_reply_to":"7a016987_24af0ae5","updated":"2015-05-26 18:09:42.000000000","message":"Of course, look forward for that patch in the series.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"}],"tests/unit/benchmark/test_validation.py":[{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":130,"context_line":"            \"test_file\", os.R_OK, \"p\", False)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def test__check_command_valid(self):"},{"line_number":133,"context_line":"        check_command_dict \u003d validation._check_command_dict"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        e \u003d self.assertRaises("},{"line_number":136,"context_line":"            ValueError, check_command_dict,"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_e4e69243","line":133,"updated":"2015-05-26 18:01:03.000000000","message":"Unnecessary indirection that reduces code readability.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        self.assertFalse(result.is_valid, result.msg)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    @mock.patch(\"rally.benchmark.validation._file_access_ok\")"},{"line_number":172,"context_line":"    def test_valid_command_fail(self, mock__file_access_ok):"},{"line_number":173,"context_line":"        mock__file_access_ok.return_value \u003d validation.ValidationResult(False)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"        validator \u003d self._unwrap_validator(validation.valid_command,"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_c45156da","line":172,"updated":"2015-05-26 18:01:03.000000000","message":"It\u0027d be nice if the name of this test was a little more descriptive, e.g.:\n\n    def test_valid_command_unreadable_script_file(...):","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"ec00c83535c2c77094fd4cee7e8da76ace5f03cb","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        command \u003d {\"script_inline\": \"bar\", \"interpreter\": \"/bin/sh\"}"},{"line_number":199,"context_line":"        result \u003d validator({\"args\": {\"p\": command}}, None, None)"},{"line_number":200,"context_line":"        self.assertTrue(result.is_valid, result.msg)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test__get_validated_image_no_value_in_config(self):"},{"line_number":203,"context_line":"        result \u003d validation._get_validated_image({}, None, \"non_existing\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_24726a7d","line":200,"updated":"2015-05-26 18:01:03.000000000","message":"This test doesn\u0027t mock out _check_command_dict(), so depends on _check_command_dict() functioning correctly. Mock out _check_command_dict() here to make this a properly-confined unit test.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":11748,"name":"Chris St. Pierre","email":"cstpierr@cisco.com","username":"stpierre"},"change_message_id":"6e3068495c86199035e093ae89c04455eb512efa","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        command \u003d {\"script_inline\": \"bar\", \"interpreter\": \"/bin/sh\"}"},{"line_number":199,"context_line":"        result \u003d validator({\"args\": {\"p\": command}}, None, None)"},{"line_number":200,"context_line":"        self.assertTrue(result.is_valid, result.msg)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test__get_validated_image_no_value_in_config(self):"},{"line_number":203,"context_line":"        result \u003d validation._get_validated_image({}, None, \"non_existing\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_7f30ef9e","line":200,"in_reply_to":"7a016987_0423ee06","updated":"2015-05-26 18:23:29.000000000","message":"Mocking _check_command_dict() is necessary here, otherwise this ceases to be a unit test. Unless you\u0027re defining _check_command_dict() as an implementation detail that is part of the same \"unit\" as valid_command(), in which case  test__check_command_valid() can be removed and replaced with tests that call valid_command().","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":13609,"name":"Pavel Boldin","email":"pboldin@mirantis.com","username":"pboldin"},"change_message_id":"ccb7c754689c56d453e2c812e3f8a541429967a1","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        command \u003d {\"script_inline\": \"bar\", \"interpreter\": \"/bin/sh\"}"},{"line_number":199,"context_line":"        result \u003d validator({\"args\": {\"p\": command}}, None, None)"},{"line_number":200,"context_line":"        self.assertTrue(result.is_valid, result.msg)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test__get_validated_image_no_value_in_config(self):"},{"line_number":203,"context_line":"        result \u003d validation._get_validated_image({}, None, \"non_existing\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_0423ee06","line":200,"in_reply_to":"7a016987_24726a7d","updated":"2015-05-26 18:09:42.000000000","message":"We do really try not to mock everything out. Only the necessary things are mocked.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"},{"author":{"_account_id":6172,"name":"Boris Pavlovic","email":"boris@pavlovic.me","username":"boris-42"},"change_message_id":"c31b6f16d29cb84c969d79e42dcdc66e268cbd94","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"        command \u003d {\"script_inline\": \"bar\", \"interpreter\": \"/bin/sh\"}"},{"line_number":199,"context_line":"        result \u003d validator({\"args\": {\"p\": command}}, None, None)"},{"line_number":200,"context_line":"        self.assertTrue(result.is_valid, result.msg)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def test__get_validated_image_no_value_in_config(self):"},{"line_number":203,"context_line":"        result \u003d validation._get_validated_image({}, None, \"non_existing\")"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a016987_f14ad326","line":200,"in_reply_to":"7a016987_7f30ef9e","updated":"2015-05-26 18:49:57.000000000","message":"Chris, \n\nIn case of Rally we don\u0027t use true \"unittest\" way where everything is mocked for 2 reasons: \n\n  - it\u0027s more complicated \n\n  - Mocking everything usually test less (much less) with the same coverage report. Like if A calls B and we are mocking B then if we change fully B including signature A test will still pass (even if we are calling old B signature). That\u0027s why the only way to do safe mocking is to mock C in case when A is calling B that calls C.","commit_id":"27ad0db8591ee1edaf8464de157995436ad6b434"}]}