)]}'
{"mistral/actions/openstack/base.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        self._kwargs_for_run \u003d kwargs"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    @abc.abstractmethod"},{"line_number":49,"context_line":"    def _create_client(self, region_name\u003dNone):"},{"line_number":50,"context_line":"        \"\"\"Creates client required for action operation\"\"\""},{"line_number":51,"context_line":"        pass"},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_fa81984e","line":49,"range":{"start_line":49,"start_character":29,"end_line":49,"end_character":40},"updated":"2016-12-20 05:48:57.000000000","message":"I don\u0027t understand why we need this parameter if region name can always be extracted from os_context which is accessed as self.os_context. I would add a method _get_region_name(self) that could be called where region name is needed. In other words, what is part of the object state shouldn\u0027t be passed as a parameter in internal methods.","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/actions/std_actions.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    def __init__(self):"},{"line_number":92,"context_line":"        pass"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def run(self, context\u003dNone):"},{"line_number":95,"context_line":"        LOG.info(\u0027Running fail action.\u0027)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        raise exc.ActionException(\u0027Fail action expected exception.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_ba4ba058","line":94,"range":{"start_line":94,"start_character":18,"end_line":94,"end_character":25},"updated":"2016-12-20 05:48:57.000000000","message":"How about using already existing optional parameter \"action_context\"? Take a look at the base Action interface docstring. Any action can optionally have it. Before we complete our Custom Actions reform this is our contract with users.\n\nIn my understanding, OpenStack context should be just a part of it in one form or another.","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"2b30367214388a8d464e1af20e67b1f5e2c0852f","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    def __init__(self):"},{"line_number":92,"context_line":"        pass"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def run(self, context\u003dNone):"},{"line_number":95,"context_line":"        LOG.info(\u0027Running fail action.\u0027)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        raise exc.ActionException(\u0027Fail action expected exception.\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_54596659","line":94,"range":{"start_line":94,"start_character":18,"end_line":94,"end_character":25},"in_reply_to":"1a6eadb0_ba4ba058","updated":"2016-12-20 15:49:53.000000000","message":"I agree with Renat. We should avoid adding of new parameters to \u0027run\u0027 method.","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/engine/action_queue.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            action_ex.input,"},{"line_number":53,"context_line":"            target,"},{"line_number":54,"context_line":"            safe_rerun\u003daction_ex.runtime_context.get(\u0027safe_rerun\u0027, False),"},{"line_number":55,"context_line":"            os_context\u003daction_ex.runtime_context.get(\u0027os_context\u0027, {})"},{"line_number":56,"context_line":"        )"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_fa0438c7","line":55,"range":{"start_line":55,"start_character":54,"end_line":55,"end_character":64},"updated":"2016-12-20 05:48:57.000000000","message":"Again, I disagree that this should be \"os_context\" as a separate parameter. We already have optional \"action_context\" parameter documented in the base Action interface.\n\nAdditionally, keep in mind that Mistral can be used w/o OpenStack. For this reason, the less mentions about OpenStack we have in the code (especially in engine) the better.","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/engine/actions.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    @abc.abstractmethod"},{"line_number":72,"context_line":"    def schedule(self, input_dict, target, index\u003d0, desc\u003d\u0027\u0027, safe_rerun\u003dFalse,"},{"line_number":73,"context_line":"                 os_context\u003dNone):"},{"line_number":74,"context_line":"        \"\"\"Schedule action run."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        This method is needed to schedule action run so its result can"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_3a0f90e2","line":73,"range":{"start_line":73,"start_character":17,"end_line":73,"end_character":27},"updated":"2016-12-20 05:48:57.000000000","message":"See my comment in action_queue.py","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/engine/default_executor.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    @profiler.trace(\u0027executor-run-action\u0027)"},{"line_number":35,"context_line":"    def run_action(self, action_ex_id, action_class_str, attributes,"},{"line_number":36,"context_line":"                   action_params, safe_rerun, redelivered\u003dFalse,"},{"line_number":37,"context_line":"                   os_context\u003dNone):"},{"line_number":38,"context_line":"        \"\"\"Runs action."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        :param action_ex_id: Action execution id."}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_9af424d0","line":37,"range":{"start_line":37,"start_character":19,"end_line":37,"end_character":29},"updated":"2016-12-20 05:48:57.000000000","message":"See my comment in action_queue.py","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/engine/executor_server.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            self._rpc_server.stop(graceful)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def run_action(self, rpc_ctx, action_ex_id, action_class_str,"},{"line_number":65,"context_line":"                   attributes, params, safe_rerun, os_context):"},{"line_number":66,"context_line":"        \"\"\"Receives calls over RPC to run action on executor."},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        :param rpc_ctx: RPC request context dictionary."}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_ba03a0b7","line":65,"range":{"start_line":65,"start_character":51,"end_line":65,"end_character":61},"updated":"2016-12-20 05:48:57.000000000","message":"Same as in action_queue.py","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/engine/rpc_backend/rpc.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"eab898ca24f62cc625bbfe9e75fb4e144c4acf2a","unresolved":false,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"    def run_action(self, action_ex_id, action_class_str, attributes,"},{"line_number":340,"context_line":"                   action_params, target\u003dNone, async\u003dTrue, safe_rerun\u003dFalse,"},{"line_number":341,"context_line":"                   os_context\u003dNone):"},{"line_number":342,"context_line":"        \"\"\"Sends a request to run action to executor."},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        :param action_ex_id: Action execution id."}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_dafe9cad","line":341,"range":{"start_line":341,"start_character":19,"end_line":341,"end_character":29},"updated":"2016-12-20 05:48:57.000000000","message":"Same as in action_queue.py","commit_id":"329b2a34c972db7ffc728b43dd323658b5b46ad0"}],"mistral/tests/unit/workbook/v2/test_tasks.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8529f92a84f7bc4dc97233b5ee95b639d0054106","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                expect_error\u003dexpect_error"},{"line_number":390,"context_line":"            )"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    def test_openstack_context(self):"},{"line_number":393,"context_line":"        tests \u003d ["},{"line_number":394,"context_line":"            ({\u0027openstack_context\u0027: \u0027\u0027}, True),"},{"line_number":395,"context_line":"            ({\u0027openstack_context\u0027: {}}, True),"}],"source_content_type":"text/x-python","patch_set":7,"id":"da4df55a_aec5455c","line":392,"range":{"start_line":392,"start_character":7,"end_line":392,"end_character":8},"updated":"2016-12-22 07:55:55.000000000","message":"Just a side note: I believe this style of testing is not really good. I spent 3 minutes by looking at this test and I have now idea what we\u0027re testing. In order to understand what\u0027s going on here I have to keep all the machinery from the base class in my mind.\n\nI think we need to refactor these specification tests moving forward. The main rule for writing tests is \"A test should very easy to read and understand\", even if it has duplications and it is very long.\n\nOf course, this comment is not about your patch..","commit_id":"b7ca5d74d9dd54e60d95e0d97c5be35d499e6d03"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"2de937ce8fc31b31b9e08c3ad26839474391742a","unresolved":false,"context_lines":[{"line_number":389,"context_line":"                expect_error\u003dexpect_error"},{"line_number":390,"context_line":"            )"},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"    def test_openstack_context(self):"},{"line_number":393,"context_line":"        tests \u003d ["},{"line_number":394,"context_line":"            ({\u0027openstack_context\u0027: \u0027\u0027}, True),"},{"line_number":395,"context_line":"            ({\u0027openstack_context\u0027: {}}, True),"}],"source_content_type":"text/x-python","patch_set":7,"id":"da4df55a_dcff6ca8","line":392,"range":{"start_line":392,"start_character":7,"end_line":392,"end_character":8},"in_reply_to":"da4df55a_aec5455c","updated":"2016-12-23 10:17:07.000000000","message":"Renat, I totally agree with you, I also don\u0027t think this kind of code is readable for developers. I just use the consistent way as before to avoid making unrelated change in this patch (I prefer one patch do one thing at the time). But I agree that we should refactor this to make it clear in separate patch.","commit_id":"b7ca5d74d9dd54e60d95e0d97c5be35d499e6d03"}],"mistral/workbook/v2/tasks.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8529f92a84f7bc4dc97233b5ee95b639d0054106","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            \"target\": types.NONEMPTY_STRING,"},{"line_number":66,"context_line":"            \"keep-result\": types.EXPRESSION_OR_BOOLEAN,"},{"line_number":67,"context_line":"            \"safe-rerun\": types.EXPRESSION_OR_BOOLEAN,"},{"line_number":68,"context_line":"            \"openstack_context\": types.CONTEXT_DICT,"},{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"additionalProperties\": False,"},{"line_number":71,"context_line":"        \"anyOf\": ["}],"source_content_type":"text/x-python","patch_set":7,"id":"da4df55a_ce8e0927","line":68,"range":{"start_line":68,"start_character":13,"end_line":68,"end_character":30},"updated":"2016-12-22 07:55:55.000000000","message":"Lingxian, why is it a part of the task spec? I honestly don\u0027t understand this yet.","commit_id":"b7ca5d74d9dd54e60d95e0d97c5be35d499e6d03"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"2de937ce8fc31b31b9e08c3ad26839474391742a","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            \"target\": types.NONEMPTY_STRING,"},{"line_number":66,"context_line":"            \"keep-result\": types.EXPRESSION_OR_BOOLEAN,"},{"line_number":67,"context_line":"            \"safe-rerun\": types.EXPRESSION_OR_BOOLEAN,"},{"line_number":68,"context_line":"            \"openstack_context\": types.CONTEXT_DICT,"},{"line_number":69,"context_line":"        },"},{"line_number":70,"context_line":"        \"additionalProperties\": False,"},{"line_number":71,"context_line":"        \"anyOf\": ["}],"source_content_type":"text/x-python","patch_set":7,"id":"da4df55a_5c2a9c2d","line":68,"range":{"start_line":68,"start_character":13,"end_line":68,"end_character":30},"in_reply_to":"da4df55a_ce8e0927","updated":"2016-12-23 10:17:07.000000000","message":"I believe https://review.openstack.org/#/c/414508/ will explain everything :-)","commit_id":"b7ca5d74d9dd54e60d95e0d97c5be35d499e6d03"}]}