)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":40,"context_line":"   task_handler.continue_task()"},{"line_number":41,"context_line":" * More tests for error handling"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"P.S. This patch is very big but it was nearly impossible to split"},{"line_number":44,"context_line":"it into multiple smaller patches just because how entangled everything"},{"line_number":45,"context_line":"was in Mistral Engine."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Partially implements: blueprint mistral-engine-error-handling"},{"line_number":48,"context_line":"Implements: blueprint mistral-action-result-processing-pipeline"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":41,"id":"9abb7d3a_74b3dee9","line":45,"range":{"start_line":43,"start_character":0,"end_line":45,"end_character":22},"updated":"2016-05-31 11:27:31.000000000","message":"I guess that introduction of new tasks and actions (mistral/engine/actions.py and tasks.py) could be done in separate patch. Than this patch would be only using of already existing code, which would make it smaller","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/actions/std_actions.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def run(self):"},{"line_number":49,"context_line":"        LOG.info(\u0027Running echo action [output\u003d%s]\u0027 % self.output)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        print(\"output \u003d %s\" % self.output)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        return self.output"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_d47e69ad","line":51,"updated":"2016-05-27 23:40:59.000000000","message":"Soft: what\u0027s this print for? Please fix this in a separate patch if you want to merge this patchset quickly.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    def run(self):"},{"line_number":49,"context_line":"        LOG.info(\u0027Running echo action [output\u003d%s]\u0027 % self.output)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        print(\"output \u003d %s\" % self.output)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        return self.output"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_ac7271e3","line":51,"in_reply_to":"9abb7d3a_d47e69ad","updated":"2016-05-28 17:03:48.000000000","message":"Left it by mistake.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"}],"mistral/db/v2/sqlalchemy/models.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        size_limit_kb \u003d cfg.CONF.engine.execution_field_size_limit_kb"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        # If the size is unlimited."},{"line_number":202,"context_line":"        if size_limit_kb \u003c 0:"},{"line_number":203,"context_line":"            return"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        size_kb \u003d int(sys.getsizeof(str(value)) / 1024)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d4fa8a74","line":202,"range":{"start_line":202,"start_character":8,"end_line":202,"end_character":29},"updated":"2016-05-31 11:27:31.000000000","message":"This change is needed, but it makes big patch even bigger. It shouldn\u0027t be done as part of engine refactoring IMO.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        size_limit_kb \u003d cfg.CONF.engine.execution_field_size_limit_kb"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        # If the size is unlimited."},{"line_number":202,"context_line":"        if size_limit_kb \u003c 0:"},{"line_number":203,"context_line":"            return"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        size_kb \u003d int(sys.getsizeof(str(value)) / 1024)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_cdcdd886","line":202,"range":{"start_line":202,"start_character":8,"end_line":202,"end_character":29},"in_reply_to":"9abb7d3a_d4fa8a74","updated":"2016-05-31 13:32:19.000000000","message":"Dawid, I have a strong opinion about making cosmetic changes: if I see that something can be within 2 seconds then go ahead to do this no matter what patch this is. Otherwise, we won\u0027t be progressing quickly.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":394,"context_line":"# Register all hooks related to secure models."},{"line_number":395,"context_line":"mb.register_secure_model_hooks()"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"# TODO(rakhmerov): This is a bad solution. It\u0027s hard to find in the code,"},{"line_number":398,"context_line":"# configure flexibly etc. Fix it."},{"line_number":399,"context_line":"# Register an event listener to verify that the size of all the long columns"},{"line_number":400,"context_line":"# affected by the user do not exceed the limit configuration."},{"line_number":401,"context_line":"for attr_name in [\u0027input\u0027, \u0027output\u0027, \u0027params\u0027, \u0027published\u0027]:"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_b4f7068a","line":398,"range":{"start_line":397,"start_character":0,"end_line":398,"end_character":33},"updated":"2016-05-31 11:27:31.000000000","message":"Same as above - this should be done in separate patch IMO","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":394,"context_line":"# Register all hooks related to secure models."},{"line_number":395,"context_line":"mb.register_secure_model_hooks()"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"# TODO(rakhmerov): This is a bad solution. It\u0027s hard to find in the code,"},{"line_number":398,"context_line":"# configure flexibly etc. Fix it."},{"line_number":399,"context_line":"# Register an event listener to verify that the size of all the long columns"},{"line_number":400,"context_line":"# affected by the user do not exceed the limit configuration."},{"line_number":401,"context_line":"for attr_name in [\u0027input\u0027, \u0027output\u0027, \u0027params\u0027, \u0027published\u0027]:"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_edfb7cbf","line":398,"range":{"start_line":397,"start_character":0,"end_line":398,"end_character":33},"in_reply_to":"9abb7d3a_b4f7068a","updated":"2016-05-31 13:32:19.000000000","message":"What exactly?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/action_handler.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        action.fail(msg)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        if task_ex:"},{"line_number":44,"context_line":"            task_handler.fail_task(task_ex, msg)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        return"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_ad543854","line":44,"updated":"2016-05-27 23:40:59.000000000","message":"Why not do something like task.fail() when you have action.fail() above? This seems inconsistent.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        action.fail(msg)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"        if task_ex:"},{"line_number":44,"context_line":"            task_handler.fail_task(task_ex, msg)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        return"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_8c632d1a","line":44,"in_reply_to":"9abb7d3a_ad543854","updated":"2016-05-28 17:03:48.000000000","message":"Well, this is still slightly moving but the idea is that Task should not be responsible for error handling, that way Task implementation remain clear of any try-excepts and hence are more isolated and maintainable. For now, I decided to put all things related to error handling into handlers (action, task, workflow). It seem a little bit more fine-grained to me. However, what you\u0027re saying may make sense and we need to see moving forward if can get rid of these handlers at all. I felt like we can leave them for now with responsibilities to serve as factories that know how to create certain types of actions and tasks and to handler errors.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"}],"mistral/engine/actions.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from mistral.db.v2 import api as db_api"},{"line_number":21,"context_line":"from mistral.engine import rpc"},{"line_number":22,"context_line":"from mistral.engine import utils as e_utils"},{"line_number":23,"context_line":"from mistral import exceptions as exc"},{"line_number":24,"context_line":"from mistral import expressions as expr"},{"line_number":25,"context_line":"from mistral.services import action_manager as a_m"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_b43ee625","line":22,"range":{"start_line":22,"start_character":36,"end_line":22,"end_character":43},"updated":"2016-05-31 11:27:31.000000000","message":"e_utils is not very clear - could you use engine_utils instead?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from mistral.db.v2 import api as db_api"},{"line_number":21,"context_line":"from mistral.engine import rpc"},{"line_number":22,"context_line":"from mistral.engine import utils as e_utils"},{"line_number":23,"context_line":"from mistral import exceptions as exc"},{"line_number":24,"context_line":"from mistral import expressions as expr"},{"line_number":25,"context_line":"from mistral.services import action_manager as a_m"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_4de448e0","line":22,"range":{"start_line":22,"start_character":36,"end_line":22,"end_character":43},"in_reply_to":"9abb7d3a_b43ee625","updated":"2016-05-31 13:32:19.000000000","message":"It\u0027s a matter of taste I think. Yeah, we could do this but I thought in this context (in engine package) it was pretty clear what it is. And that used to be the same before refactoring.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from mistral.engine import rpc"},{"line_number":22,"context_line":"from mistral.engine import utils as e_utils"},{"line_number":23,"context_line":"from mistral import exceptions as exc"},{"line_number":24,"context_line":"from mistral import expressions as expr"},{"line_number":25,"context_line":"from mistral.services import action_manager as a_m"},{"line_number":26,"context_line":"from mistral.services import executions as wf_ex_service"},{"line_number":27,"context_line":"from mistral.services import scheduler"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_341956d8","line":24,"range":{"start_line":24,"start_character":35,"end_line":24,"end_character":39},"updated":"2016-05-31 11:27:31.000000000","message":"Why not just leave \u0027expressions\u0027? \u0027expr\u0027 can be misleading.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from mistral.engine import rpc"},{"line_number":22,"context_line":"from mistral.engine import utils as e_utils"},{"line_number":23,"context_line":"from mistral import exceptions as exc"},{"line_number":24,"context_line":"from mistral import expressions as expr"},{"line_number":25,"context_line":"from mistral.services import action_manager as a_m"},{"line_number":26,"context_line":"from mistral.services import executions as wf_ex_service"},{"line_number":27,"context_line":"from mistral.services import scheduler"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_0deec0b3","line":24,"range":{"start_line":24,"start_character":35,"end_line":24,"end_character":39},"in_reply_to":"9abb7d3a_341956d8","updated":"2016-05-31 13:32:19.000000000","message":"Generally we try to be more concise. I would say: as concise as possible in a given context. This is what we\u0027ve been using for a long time.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from mistral.engine import utils as e_utils"},{"line_number":23,"context_line":"from mistral import exceptions as exc"},{"line_number":24,"context_line":"from mistral import expressions as expr"},{"line_number":25,"context_line":"from mistral.services import action_manager as a_m"},{"line_number":26,"context_line":"from mistral.services import executions as wf_ex_service"},{"line_number":27,"context_line":"from mistral.services import scheduler"},{"line_number":28,"context_line":"from mistral.services import security"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_b425460a","line":25,"range":{"start_line":25,"start_character":47,"end_line":25,"end_character":50},"updated":"2016-05-31 11:27:31.000000000","message":"a_m is not clear - could you just use action_manager?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        self.action_ex.output \u003d {\u0027result\u0027: msg}"},{"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):"},{"line_number":73,"context_line":"        \"\"\"Schedule action run."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        This method is needed to schedule action run so its result can"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_547b3af5","line":72,"range":{"start_line":72,"start_character":8,"end_line":72,"end_character":16},"updated":"2016-05-31 11:27:31.000000000","message":"Maybe it should be renamed to \"run_asynchronous\" to give better perspective what outcome would be from using it instead of \u0027run\u0027 action","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        self.action_ex.output \u003d {\u0027result\u0027: msg}"},{"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):"},{"line_number":73,"context_line":"        \"\"\"Schedule action run."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        This method is needed to schedule action run so its result can"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_8da2b049","line":72,"range":{"start_line":72,"start_character":8,"end_line":72,"end_character":16},"in_reply_to":"9abb7d3a_547b3af5","updated":"2016-05-31 13:32:19.000000000","message":"IMO, \"asynchronous\" here may slightly confuse because there synchronous and asynchronous actions. In verbal conversations we often use word \"schedule\" so I decided to have it in code too. But generally, we can rename it yes.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        raise NotImplementedError"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    @abc.abstractmethod"},{"line_number":89,"context_line":"    def run(self, input_dict, target, index\u003d0, desc\u003d\u0027\u0027, save\u003dTrue):"},{"line_number":90,"context_line":"        \"\"\"Immediately run action."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"        This method runs method w/o scheduling its run for a later time."}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d46f2a29","line":89,"range":{"start_line":89,"start_character":8,"end_line":89,"end_character":11},"updated":"2016-05-31 11:27:31.000000000","message":"Maybe it should be renamed to \"run_synchronous\" to give better perspective what outcome would be from using it instead of \u0027schedule\u0027 action and to make it clear that using it would block engine","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        # to all actions. It\u0027s related to"},{"line_number":128,"context_line":"        # https://blueprints.launchpad.net/mistral/+spec/mistral-custom-actions-api"},{"line_number":129,"context_line":"        if a_m.has_action_context("},{"line_number":130,"context_line":"                self.action_def.action_class,"},{"line_number":131,"context_line":"                self.action_def.attributes or {}) and self.task_ex:"},{"line_number":132,"context_line":"            input_dict.update("},{"line_number":133,"context_line":"                a_m.get_action_context(self.task_ex, action_ex_id)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_f4498eb8","line":130,"range":{"start_line":130,"start_character":21,"end_line":130,"end_character":44},"updated":"2016-05-31 11:27:31.000000000","message":"What if self.action_def is None? Should this part be inside a try-except statement?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        # to all actions. It\u0027s related to"},{"line_number":128,"context_line":"        # https://blueprints.launchpad.net/mistral/+spec/mistral-custom-actions-api"},{"line_number":129,"context_line":"        if a_m.has_action_context("},{"line_number":130,"context_line":"                self.action_def.action_class,"},{"line_number":131,"context_line":"                self.action_def.attributes or {}) and self.task_ex:"},{"line_number":132,"context_line":"            input_dict.update("},{"line_number":133,"context_line":"                a_m.get_action_context(self.task_ex, action_ex_id)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_4d51e835","line":130,"range":{"start_line":130,"start_character":21,"end_line":130,"end_character":44},"in_reply_to":"9abb7d3a_f4498eb8","updated":"2016-05-31 13:32:19.000000000","message":"No it\u0027s just not a part of this patch yet but I\u0027m planning to add a number of assertions in the code to show what preconditions we expect in certain places. In this case it must never be None.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        scheduler.schedule_call("},{"line_number":210,"context_line":"            None,"},{"line_number":211,"context_line":"            \u0027mistral.engine.actions._run_existing_action\u0027,"},{"line_number":212,"context_line":"            0,"},{"line_number":213,"context_line":"            action_ex_id\u003dself.action_ex.id,"},{"line_number":214,"context_line":"            target\u003dtarget"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_949da20c","line":211,"range":{"start_line":211,"start_character":12,"end_line":211,"end_character":58},"updated":"2016-05-31 11:27:31.000000000","message":"This string should be store somewhere and only called here by it\u0027s name e.g. mistral_cons.RUN_EXISTING_ACTION IMO","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        scheduler.schedule_call("},{"line_number":210,"context_line":"            None,"},{"line_number":211,"context_line":"            \u0027mistral.engine.actions._run_existing_action\u0027,"},{"line_number":212,"context_line":"            0,"},{"line_number":213,"context_line":"            action_ex_id\u003dself.action_ex.id,"},{"line_number":214,"context_line":"            target\u003dtarget"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_2d574427","line":211,"range":{"start_line":211,"start_character":12,"end_line":211,"end_character":58},"in_reply_to":"9abb7d3a_949da20c","updated":"2016-05-31 13:32:19.000000000","message":"Agree with that. Will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":346,"context_line":"    \"\"\"Workflow action.\"\"\""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def complete(self, result):"},{"line_number":349,"context_line":"        # No-op because in case of workflow result is already processed."},{"line_number":350,"context_line":"        pass"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def schedule(self, input_dict, target, index\u003d0, desc\u003d\u0027\u0027):"},{"line_number":353,"context_line":"        parent_wf_ex \u003d self.task_ex.workflow_execution"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_94b48282","line":350,"range":{"start_line":349,"start_character":0,"end_line":350,"end_character":12},"updated":"2016-05-31 11:27:31.000000000","message":"This comment may be replace with docstring, so that \u0027pass\u0027 statement may be deleted, since it should be used to mark place in a code that needs future work - it may be misleading in this place.\nOr you can just replace pass with return.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":346,"context_line":"    \"\"\"Workflow action.\"\"\""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def complete(self, result):"},{"line_number":349,"context_line":"        # No-op because in case of workflow result is already processed."},{"line_number":350,"context_line":"        pass"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def schedule(self, input_dict, target, index\u003d0, desc\u003d\u0027\u0027):"},{"line_number":353,"context_line":"        parent_wf_ex \u003d self.task_ex.workflow_execution"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_31303ee8","line":350,"range":{"start_line":349,"start_character":0,"end_line":350,"end_character":12},"in_reply_to":"9abb7d3a_94b48282","updated":"2016-05-31 13:32:19.000000000","message":"I don\u0027t think \"pass\" exactly means that something else must be done. Agree on docstring though.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        wf_ex, _ \u003d wf_ex_service.create_workflow_execution("},{"line_number":382,"context_line":"            wf_def.name,"},{"line_number":383,"context_line":"            input_dict,"},{"line_number":384,"context_line":"            \"sub-workflow execution\","},{"line_number":385,"context_line":"            wf_params,"},{"line_number":386,"context_line":"            wf_spec"},{"line_number":387,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_14a9b224","line":384,"range":{"start_line":384,"start_character":12,"end_line":384,"end_character":36},"updated":"2016-05-31 11:27:31.000000000","message":"This should be stored as a variable and in this place only that variable shall be called","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":381,"context_line":"        wf_ex, _ \u003d wf_ex_service.create_workflow_execution("},{"line_number":382,"context_line":"            wf_def.name,"},{"line_number":383,"context_line":"            input_dict,"},{"line_number":384,"context_line":"            \"sub-workflow execution\","},{"line_number":385,"context_line":"            wf_params,"},{"line_number":386,"context_line":"            wf_spec"},{"line_number":387,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_71892675","line":384,"range":{"start_line":384,"start_character":12,"end_line":384,"end_character":36},"in_reply_to":"9abb7d3a_14a9b224","updated":"2016-05-31 13:32:19.000000000","message":"Well, I don\u0027t quite agree on that. Because this is just an arbitrary string that doesn\u0027t matter for correctness of program logic.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        scheduler.schedule_call("},{"line_number":390,"context_line":"            None,"},{"line_number":391,"context_line":"            \u0027mistral.engine.actions.resume_workflow\u0027,"},{"line_number":392,"context_line":"            0,"},{"line_number":393,"context_line":"            wf_ex_id\u003dwf_ex.id,"},{"line_number":394,"context_line":"            env\u003dNone"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_f47ceeb6","line":391,"range":{"start_line":391,"start_character":12,"end_line":391,"end_character":52},"updated":"2016-05-31 11:27:31.000000000","message":"same as above","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        scheduler.schedule_call("},{"line_number":390,"context_line":"            None,"},{"line_number":391,"context_line":"            \u0027mistral.engine.actions.resume_workflow\u0027,"},{"line_number":392,"context_line":"            0,"},{"line_number":393,"context_line":"            wf_ex_id\u003dwf_ex.id,"},{"line_number":394,"context_line":"            env\u003dNone"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_31479e4c","line":391,"range":{"start_line":391,"start_character":12,"end_line":391,"end_character":52},"in_reply_to":"9abb7d3a_f47ceeb6","updated":"2016-05-31 13:32:19.000000000","message":"Ok","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":447,"context_line":"    :param action_spec_name: Action name according to a spec."},{"line_number":448,"context_line":"    :param wf_name: Workflow name."},{"line_number":449,"context_line":"    :param wf_spec_name: Workflow name according to a spec."},{"line_number":450,"context_line":"    :return:"},{"line_number":451,"context_line":"    \"\"\""},{"line_number":452,"context_line":"    action_db \u003d None"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_54333a14","line":450,"range":{"start_line":450,"start_character":4,"end_line":450,"end_character":12},"updated":"2016-05-31 11:27:31.000000000","message":"Return is not described","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":447,"context_line":"    :param action_spec_name: Action name according to a spec."},{"line_number":448,"context_line":"    :param wf_name: Workflow name."},{"line_number":449,"context_line":"    :param wf_spec_name: Workflow name according to a spec."},{"line_number":450,"context_line":"    :return:"},{"line_number":451,"context_line":"    \"\"\""},{"line_number":452,"context_line":"    action_db \u003d None"},{"line_number":453,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_b1f6eef6","line":450,"range":{"start_line":450,"start_character":4,"end_line":450,"end_character":12},"in_reply_to":"9abb7d3a_54333a14","updated":"2016-05-31 13:32:19.000000000","message":"Ok, thanks. Will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/default_engine.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                )"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"                return wf_ex.get_clone()"},{"line_number":85,"context_line":"        except Exception as e:"},{"line_number":86,"context_line":"            LOG.error("},{"line_number":87,"context_line":"                \"Failed to start workflow \u0027%s\u0027 id\u003d%s: %s\\n%s\","},{"line_number":88,"context_line":"                wf_identifier, wf_ex_id, e, traceback.format_exc()"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_ad3a78cb","side":"PARENT","line":85,"updated":"2016-05-27 23:40:59.000000000","message":"Since this try block is removed, if this start workflow sequence failed, will there be any records in the database showing this failed execution?","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"2e93081e017baf99c71ea47d30fc75ce078256f5","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                )"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"                return wf_ex.get_clone()"},{"line_number":85,"context_line":"        except Exception as e:"},{"line_number":86,"context_line":"            LOG.error("},{"line_number":87,"context_line":"                \"Failed to start workflow \u0027%s\u0027 id\u003d%s: %s\\n%s\","},{"line_number":88,"context_line":"                wf_identifier, wf_ex_id, e, traceback.format_exc()"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_8101ca08","side":"PARENT","line":85,"in_reply_to":"9abb7d3a_9774d69c","updated":"2016-05-31 17:41:36.000000000","message":"I only care whether this will lead to orphaned executions. If you\u0027re confident this won\u0027t happen, then it\u0027s all good, please highlight the unit tests.","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                )"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"                return wf_ex.get_clone()"},{"line_number":85,"context_line":"        except Exception as e:"},{"line_number":86,"context_line":"            LOG.error("},{"line_number":87,"context_line":"                \"Failed to start workflow \u0027%s\u0027 id\u003d%s: %s\\n%s\","},{"line_number":88,"context_line":"                wf_identifier, wf_ex_id, e, traceback.format_exc()"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_9774d69c","side":"PARENT","line":85,"in_reply_to":"9abb7d3a_ad3a78cb","updated":"2016-05-28 17:03:48.000000000","message":"This particular method is not fully refactored yet (see my comment in the commit message about need to refactor some of the things with Workflow abstraction). It will also touch resume and rerun methods. There will be error handling but not in the way it was before.\n\nOn error handling in general. Sorry for my harsh words, but what we had here in engine regarding error handling was just ridiculous. Here are the reasons:\n1) This blind error handling masked a number of real problems that we had in the code, I stumbled on it many times. For example, some time ago I fixed an error with redundant network hops between executor and engine just for how error handling worked, and it was very difficult to understand what was going on. If you\u0027re interested I can try to recall details.\n2) Some exceptions must not be handled that way. E.g. if there\u0027s a problem with environment (like DB is not accessible). Same about logical errors in the program itself. For this reason I separated MistralError and MistralException where MistralException is something that we can handle and should handle (like YAQL errors, bad user input, validation etc.). MistralError is something that tells about Force Majeure situations that we should actually see that they happen. And we must not try to handle them.\n3) Probably most important one: we shouldn\u0027t be handling errors blindly by just having a global try-except block because it doesn\u0027t give enough level of granularity for error information. Like we discussed previously, if action result is processed successfully but then something bad happened with its task then we must not roll back the entire transaction, we should fail the task and the workflow so that we see where exactly the error is when we analyse states of DB objects. With this old approach we just always saved it in state_info of workflow execution trying to collect as much info as possible (it often looked more like guessing rather than knowing). In other words, we should handler errors wisely at places where they happen. That\u0027s why I left default_engine free of any handling and moved it to handlers (again, not yet 100% finished, especially with workflow_handler, this patch is already too big). Handler are modules that are supposed to know exactly where those try-except blocks should be applied so that we keep enough granularity for error handling.\n\nFrom the very beginning, this primitive error handling was considered a temporary solution (btw, Dmitri agreed with me on that when we discussed it long ago), we just kind of got used to it and now it seems scary to move away from it. But we have to, I\u0027m 100% confident, otherwise we\u0027ll not be able to move forward productively.\n\nI hope that makes sense.","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                self._on_task_state_change(task_ex, wf_ex, wf_spec)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"                return action_ex.get_clone()"},{"line_number":264,"context_line":"        except Exception as e:"},{"line_number":265,"context_line":"            # TODO(rakhmerov): Need to refactor logging in a more elegant way."},{"line_number":266,"context_line":"            LOG.error("},{"line_number":267,"context_line":"                \u0027Failed to handle action execution result [id\u003d%s]: %s\\n%s\u0027,"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_0dee2422","side":"PARENT","line":264,"updated":"2016-05-27 23:40:59.000000000","message":"With this try block gone, what happens to the workflow execution state processing of the action execution fails here?","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                    return wf_ex.get_clone()"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"                return self._continue_workflow(wf_ex, task_ex, reset, env\u003denv)"},{"line_number":348,"context_line":"        except Exception as e:"},{"line_number":349,"context_line":"            LOG.error("},{"line_number":350,"context_line":"                \"Failed to rerun execution id\u003d%s at task\u003d%s: %s\\n%s\","},{"line_number":351,"context_line":"                wf_ex_id, task_ex_id, e, traceback.format_exc()"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_adc43892","side":"PARENT","line":348,"updated":"2016-05-27 23:40:59.000000000","message":"Same question as above about remove of the try block here.","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":364,"context_line":"                    return wf_ex.get_clone()"},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"                return self._continue_workflow(wf_ex, env\u003denv)"},{"line_number":367,"context_line":"        except Exception as e:"},{"line_number":368,"context_line":"            LOG.error("},{"line_number":369,"context_line":"                \"Failed to resume execution id\u003d%s: %s\\n%s\","},{"line_number":370,"context_line":"                wf_ex_id, e, traceback.format_exc()"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_edbe40fe","side":"PARENT","line":367,"updated":"2016-05-27 23:40:59.000000000","message":"Same question as above about remove of the try block here.","commit_id":"2cf2fc5c745aaddcc85bdf230d23b246761ec112"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            action_handler.on_action_complete(action_ex, result)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"            if not task_ex:"},{"line_number":112,"context_line":"                return action_ex.get_clone()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"            return action_ex.get_clone()"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_addd5872","line":112,"updated":"2016-05-27 23:40:59.000000000","message":"What\u0027s the difference with the get_clone in the line below?","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            action_handler.on_action_complete(action_ex, result)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"            if not task_ex:"},{"line_number":112,"context_line":"                return action_ex.get_clone()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"            return action_ex.get_clone()"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_cc441576","line":112,"in_reply_to":"9abb7d3a_addd5872","updated":"2016-05-28 17:03:48.000000000","message":"No difference, it\u0027s a mistake. Thanks!","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            if not task_ex:"},{"line_number":112,"context_line":"                return action_ex.get_clone()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"            return action_ex.get_clone()"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    @u.log_exec(LOG)"},{"line_number":117,"context_line":"    def pause_workflow(self, wf_ex_id):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_6df3700b","line":114,"updated":"2016-05-27 23:40:59.000000000","message":"What\u0027s the difference with the get_clone in the line above?","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            if not task_ex:"},{"line_number":112,"context_line":"                return action_ex.get_clone()"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"            return action_ex.get_clone()"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    @u.log_exec(LOG)"},{"line_number":117,"context_line":"    def pause_workflow(self, wf_ex_id):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_0c5f3d42","line":114,"in_reply_to":"9abb7d3a_6df3700b","updated":"2016-05-28 17:03:48.000000000","message":"Done","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        return wf_ex"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    @staticmethod"},{"line_number":123,"context_line":"    def _continue_workflow(wf_ex, task_ex\u003dNone, reset\u003dTrue, env\u003dNone):"},{"line_number":124,"context_line":"        wf_ex \u003d wf_service.update_workflow_execution_env(wf_ex, env)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        wf_handler.set_workflow_state("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d4272ac8","line":123,"range":{"start_line":122,"start_character":0,"end_line":123,"end_character":70},"updated":"2016-05-31 11:27:31.000000000","message":"Static methods are meant to be called from outside the class, so private method should\u0027t be static","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"f38ac1097eeb09b160c6713bec22d0fc5d81ca60","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        return wf_ex"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    @staticmethod"},{"line_number":123,"context_line":"    def _continue_workflow(wf_ex, task_ex\u003dNone, reset\u003dTrue, env\u003dNone):"},{"line_number":124,"context_line":"        wf_ex \u003d wf_service.update_workflow_execution_env(wf_ex, env)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        wf_handler.set_workflow_state("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_86a3aab1","line":123,"in_reply_to":"9abb7d3a_7133e6ab","updated":"2016-05-31 14:42:33.000000000","message":"I agree with Dawid, I\u0027ve seen people use this before to make it clear that self isn\u0027t used, but I think it is confusing. If you really want to indicate that, moving it outside the class would be the best way.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"9cc5a064672062c8907bdff6bacec8117a7f085e","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        return wf_ex"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    @staticmethod"},{"line_number":123,"context_line":"    def _continue_workflow(wf_ex, task_ex\u003dNone, reset\u003dTrue, env\u003dNone):"},{"line_number":124,"context_line":"        wf_ex \u003d wf_service.update_workflow_execution_env(wf_ex, env)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        wf_handler.set_workflow_state("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_4676e206","line":123,"in_reply_to":"9abb7d3a_86a3aab1","updated":"2016-05-31 14:44:34.000000000","message":"Yes, it will change in one of the subsequent patches anyway since I haven\u0027t finished with this part (resume and rerun methods).","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":119,"context_line":""},{"line_number":120,"context_line":"        return wf_ex"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"    @staticmethod"},{"line_number":123,"context_line":"    def _continue_workflow(wf_ex, task_ex\u003dNone, reset\u003dTrue, env\u003dNone):"},{"line_number":124,"context_line":"        wf_ex \u003d wf_service.update_workflow_execution_env(wf_ex, env)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        wf_handler.set_workflow_state("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_7133e6ab","line":123,"range":{"start_line":122,"start_character":0,"end_line":123,"end_character":70},"in_reply_to":"9abb7d3a_d4272ac8","updated":"2016-05-31 13:32:19.000000000","message":"Hm.. really? Why not? Anyway, this method is not fully refactored yet (see commit message)","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/dispatcher.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    from mistral.engine import workflow_handler as wf_handler"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    if not wf_cmds:"},{"line_number":28,"context_line":"        return"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    for cmd in wf_cmds:"},{"line_number":31,"context_line":"        if isinstance(cmd, (commands.RunTask, commands.RunExistingTask)):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_6d26d069","line":28,"updated":"2016-05-27 23:40:59.000000000","message":"Soft: This is not necessary. If there\u0027s no wf_cmds, then the for loop will return anyway.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    from mistral.engine import workflow_handler as wf_handler"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    if not wf_cmds:"},{"line_number":28,"context_line":"        return"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    for cmd in wf_cmds:"},{"line_number":31,"context_line":"        if isinstance(cmd, (commands.RunTask, commands.RunExistingTask)):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_cc9ff501","line":28,"in_reply_to":"9abb7d3a_6d26d069","updated":"2016-05-28 17:03:48.000000000","message":"for loop will fail if wf_cmds is None.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def dispatch_workflow_commands(wf_ex, wf_cmds):"},{"line_number":22,"context_line":"    # TODO(rakhmerov): I don\u0027t like these imports but otherwise we have"},{"line_number":23,"context_line":"    # import cycles."},{"line_number":24,"context_line":"    from mistral.engine import task_handler"},{"line_number":25,"context_line":"    from mistral.engine import workflow_handler as wf_handler"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    if not wf_cmds:"},{"line_number":28,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_74159e47","line":25,"range":{"start_line":22,"start_character":0,"end_line":25,"end_character":61},"updated":"2016-05-31 11:27:31.000000000","message":"Some believes that such imports are OK to break import loop","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def dispatch_workflow_commands(wf_ex, wf_cmds):"},{"line_number":22,"context_line":"    # TODO(rakhmerov): I don\u0027t like these imports but otherwise we have"},{"line_number":23,"context_line":"    # import cycles."},{"line_number":24,"context_line":"    from mistral.engine import task_handler"},{"line_number":25,"context_line":"    from mistral.engine import workflow_handler as wf_handler"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    if not wf_cmds:"},{"line_number":28,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d1a9f291","line":25,"range":{"start_line":22,"start_character":0,"end_line":25,"end_character":61},"in_reply_to":"9abb7d3a_74159e47","updated":"2016-05-31 13:32:19.000000000","message":"Yeah, but usually such loops signal about some issues in design. From my experience, it\u0027s always possible to get rid of them if packages and modules are designed well.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/policies.py":[{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"073bbd12725c297f122f309270c8e5c5510e2488","unresolved":false,"context_lines":[{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _continue_task(task_ex_id):"},{"line_number":439,"context_line":"    from mistral.engine import task_handler"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"    # TODO(rakhmerov): It must be done in TX after Scheduler is fixed."},{"line_number":442,"context_line":"    task_handler.continue_task(db_api.get_task_execution(task_ex_id))"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_97693128","line":439,"range":{"start_line":439,"start_character":4,"end_line":439,"end_character":43},"updated":"2016-05-31 13:34:46.000000000","message":"Importing inside the function is usually bad idea as per python guide line. It makes function execution slower. Do we have strong reason for doing this ?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _continue_task(task_ex_id):"},{"line_number":439,"context_line":"    from mistral.engine import task_handler"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"    # TODO(rakhmerov): It must be done in TX after Scheduler is fixed."},{"line_number":442,"context_line":"    task_handler.continue_task(db_api.get_task_execution(task_ex_id))"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_281c5a69","line":439,"range":{"start_line":439,"start_character":4,"end_line":439,"end_character":43},"updated":"2016-05-31 11:27:31.000000000","message":"Is there a reason for this import to be inside the function?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":436,"context_line":""},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"def _continue_task(task_ex_id):"},{"line_number":439,"context_line":"    from mistral.engine import task_handler"},{"line_number":440,"context_line":""},{"line_number":441,"context_line":"    # TODO(rakhmerov): It must be done in TX after Scheduler is fixed."},{"line_number":442,"context_line":"    task_handler.continue_task(db_api.get_task_execution(task_ex_id))"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_f1c9965a","line":439,"range":{"start_line":439,"start_character":4,"end_line":439,"end_character":43},"in_reply_to":"9abb7d3a_281c5a69","updated":"2016-05-31 13:32:19.000000000","message":"Same reason: to break the loop.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"073bbd12725c297f122f309270c8e5c5510e2488","unresolved":false,"context_lines":[{"line_number":443,"context_line":""},{"line_number":444,"context_line":""},{"line_number":445,"context_line":"def _complete_task(task_ex_id, state, state_info):"},{"line_number":446,"context_line":"    from mistral.engine import task_handler"},{"line_number":447,"context_line":""},{"line_number":448,"context_line":"    # TODO(rakhmerov): It must be done in TX after Scheduler is fixed."},{"line_number":449,"context_line":"    task_handler.complete_task("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_f7787df4","line":446,"range":{"start_line":446,"start_character":4,"end_line":446,"end_character":43},"updated":"2016-05-31 13:34:46.000000000","message":"Ditto","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"073bbd12725c297f122f309270c8e5c5510e2488","unresolved":false,"context_lines":[{"line_number":454,"context_line":""},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"def _fail_task_if_incomplete(task_ex_id, timeout):"},{"line_number":457,"context_line":"    from mistral.engine import task_handler"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    # TODO(rakhmerov): It must be done in TX after Scheduler is fixed."},{"line_number":460,"context_line":"    task_ex \u003d db_api.get_task_execution(task_ex_id)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_775bed87","line":457,"range":{"start_line":457,"start_character":5,"end_line":457,"end_character":43},"updated":"2016-05-31 13:34:46.000000000","message":"Ditto","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/rpc.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":549,"context_line":"        # type."},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"        return (wf_utils.Result(data\u003dres[\u0027data\u0027], error\u003dres[\u0027error\u0027]) if res"},{"line_number":552,"context_line":"                else None)"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_edf120cf","line":552,"updated":"2016-05-27 23:40:59.000000000","message":"Soft: Follow guideline...\n\nreturn (\n    wf_utils.Result(data\u003dres[\u0027data\u0027], error\u003dres[\u0027error\u0027])\n    if res else None\n)","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":549,"context_line":"        # type."},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"        return (wf_utils.Result(data\u003dres[\u0027data\u0027], error\u003dres[\u0027error\u0027]) if res"},{"line_number":552,"context_line":"                else None)"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_ec7479d0","line":552,"in_reply_to":"9abb7d3a_edf120cf","updated":"2016-05-28 17:03:48.000000000","message":"Done","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"change_message_id":"073bbd12725c297f122f309270c8e5c5510e2488","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        # deserialization to concrete serializers depending on object"},{"line_number":549,"context_line":"        # type."},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"        return ("},{"line_number":552,"context_line":"            wf_utils.Result(data\u003dres[\u0027data\u0027], error\u003dres[\u0027error\u0027])"},{"line_number":553,"context_line":"            if res else None"},{"line_number":554,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_1216efa6","line":554,"range":{"start_line":551,"start_character":7,"end_line":554,"end_character":9},"updated":"2016-05-31 13:34:46.000000000","message":"Isn\u0027t it same like we did in executor?\nhttps://github.com/openstack/mistral/blob/master/mistral/engine/default_executor.py#L82","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"2ed8e6c29b5a104638cc4032fc3c279e6d114515","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        # deserialization to concrete serializers depending on object"},{"line_number":549,"context_line":"        # type."},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"        return ("},{"line_number":552,"context_line":"            wf_utils.Result(data\u003dres[\u0027data\u0027], error\u003dres[\u0027error\u0027])"},{"line_number":553,"context_line":"            if res else None"},{"line_number":554,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_17fc0ab7","line":554,"range":{"start_line":551,"start_character":7,"end_line":554,"end_character":9},"in_reply_to":"9abb7d3a_1216efa6","updated":"2016-05-31 13:38:07.000000000","message":"No, it\u0027s deserialization from a primitive type (dict).","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/task_handler.py":[{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        msg \u003d (\"Failed to run task [wf\u003d%s, task\u003d%s]: %s\\n%s\" %"},{"line_number":48,"context_line":"               (wf_ex, task_spec.get_name(), e, tb.format_exc()))"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        LOG.error(msg)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        task.set_state(states.ERROR, msg)"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_0d946435","line":49,"updated":"2016-05-27 23:40:59.000000000","message":"Soft: Follow guideline...\n\nmsg \u003d (\n    \"Failed to run task [wf\u003d%s, task\u003d%s]: %s\\n%s\" %\n    (wf_ex, task_spec.get_name(), e, tb.format_exc())\n)","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":"        msg \u003d (\"Failed to run task [wf\u003d%s, task\u003d%s]: %s\\n%s\" %"},{"line_number":48,"context_line":"               (wf_ex, task_spec.get_name(), e, tb.format_exc()))"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"        LOG.error(msg)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        task.set_state(states.ERROR, msg)"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_8c4a0d82","line":49,"in_reply_to":"9abb7d3a_0d946435","updated":"2016-05-28 17:03:48.000000000","message":"Done","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    if task.is_completed():"},{"line_number":59,"context_line":"        wf_handler.check_workflow_completion(wf_cmd.wf_ex)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def on_action_complete(action_ex):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_2d04c87c","line":59,"updated":"2016-05-27 23:40:59.000000000","message":"I don\u0027t quite agree with the architecture here. I would keep any workflow handling out of task handler and let the engine be responsible for it.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    if task.is_completed():"},{"line_number":59,"context_line":"        wf_handler.check_workflow_completion(wf_cmd.wf_ex)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def on_action_complete(action_ex):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_cce8957f","line":59,"in_reply_to":"9abb7d3a_2d04c87c","updated":"2016-05-28 17:03:48.000000000","message":"After a lot of thinking I came to conclusion that one central place (engine) for doing this kind of things is a bad idea. Because there\u0027s a lot of variations of situations that may happen with workflow execution.\n\nI realized that the better idea is to make the whole processing chained from action to task to workflow (workflow part is not yet fully refactored in this patch). So whenever something unexpected happens with an action we need to chain this event to the next level, task. Same with task and workflow. This model has a lot of advantages that I can describe probably in some document or whitepaper (thinking now to write it once workload allows it) where the most important one is flexibility. Having a reference to any object in this action-\u003etask-\u003eworkflow chain we should be able to do processing related to this level of object. It\u0027s especially useful for implementing policies.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"2e93081e017baf99c71ea47d30fc75ce078256f5","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    if task.is_completed():"},{"line_number":59,"context_line":"        wf_handler.check_workflow_completion(wf_cmd.wf_ex)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"def on_action_complete(action_ex):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_6104a649","line":59,"in_reply_to":"9abb7d3a_cce8957f","updated":"2016-05-31 17:41:36.000000000","message":"Yes, this chaining makes more sense to me. Since you\u0027re still refactoring the workflow part, I\u0027m looking forward to the change there.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        return"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    if task.is_completed():"},{"line_number":103,"context_line":"        wf_handler.check_workflow_completion(wf_ex)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def fail_task(task_ex, msg):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_0d090496","line":103,"updated":"2016-05-27 23:40:59.000000000","message":"See comment above on responsibility of workflow handling.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        return"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    if task.is_completed():"},{"line_number":103,"context_line":"        wf_handler.check_workflow_completion(wf_ex)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def fail_task(task_ex, msg):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_4cfdc539","line":103,"in_reply_to":"9abb7d3a_0d090496","updated":"2016-05-28 17:03:48.000000000","message":"See my answer above.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    task.set_state(states.ERROR, msg)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    wf_handler.fail_workflow(task_ex.workflow_execution, msg)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"def continue_task(task_ex):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_6dfed06c","line":111,"updated":"2016-05-27 23:40:59.000000000","message":"See comment above on responsibility of workflow handling.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    task.set_state(states.ERROR, msg)"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    wf_handler.fail_workflow(task_ex.workflow_execution, msg)"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"def continue_task(task_ex):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_0c03bd43","line":111,"in_reply_to":"9abb7d3a_6dfed06c","updated":"2016-05-28 17:03:48.000000000","message":"See my answer above.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    task.run()"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    if task.is_completed():"},{"line_number":121,"context_line":"        wf_handler.check_workflow_completion(task_ex.workflow_execution)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"def complete_task(task_ex, state, state_info):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_edf2c099","line":121,"updated":"2016-05-27 23:40:59.000000000","message":"See comment above on responsibility of workflow handling.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    task.run()"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    if task.is_completed():"},{"line_number":121,"context_line":"        wf_handler.check_workflow_completion(task_ex.workflow_execution)"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"def complete_task(task_ex, state, state_info):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_2cfa0143","line":121,"in_reply_to":"9abb7d3a_edf2c099","updated":"2016-05-28 17:03:48.000000000","message":"See my answer above.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"change_message_id":"14bae83338b243c353c80547e886eb7881ec7205","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    task.complete(state, state_info)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    if task.is_completed():"},{"line_number":131,"context_line":"        wf_handler.check_workflow_completion(task_ex.workflow_execution)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def _build_task_from_execution(task_ex, task_spec\u003dNone):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_cdeffcae","line":131,"updated":"2016-05-27 23:40:59.000000000","message":"See comment above on responsibility of workflow handling.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"8bede69ece5d0a6569d5f9817ca4a91f87abdbc8","unresolved":false,"context_lines":[{"line_number":128,"context_line":"    task.complete(state, state_info)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    if task.is_completed():"},{"line_number":131,"context_line":"        wf_handler.check_workflow_completion(task_ex.workflow_execution)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"def _build_task_from_execution(task_ex, task_spec\u003dNone):"}],"source_content_type":"text/x-python","patch_set":39,"id":"9abb7d3a_8c95ede5","line":131,"in_reply_to":"9abb7d3a_cdeffcae","updated":"2016-05-28 17:03:48.000000000","message":"See my answer above.","commit_id":"a84e26d78866aa35776c208b4e8e66f80bc23bd5"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    limitations under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log as logging"},{"line_number":18,"context_line":"import traceback as tb"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from mistral.engine import tasks"},{"line_number":21,"context_line":"from mistral.engine import workflow_handler as wf_handler"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_08a9967b","line":18,"range":{"start_line":18,"start_character":20,"end_line":18,"end_character":22},"updated":"2016-05-31 11:27:31.000000000","message":"\u0027tb\u0027 can be misleading - why not use traceback?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    limitations under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log as logging"},{"line_number":18,"context_line":"import traceback as tb"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from mistral.engine import tasks"},{"line_number":21,"context_line":"from mistral.engine import workflow_handler as wf_handler"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_7132c660","line":18,"range":{"start_line":18,"start_character":20,"end_line":18,"end_character":22},"in_reply_to":"9abb7d3a_08a9967b","updated":"2016-05-31 13:32:19.000000000","message":"Again, a matter of taste, we always try to be concise by our tradition.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/tasks.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from mistral.engine import dispatcher"},{"line_number":24,"context_line":"from mistral.engine import policies"},{"line_number":25,"context_line":"from mistral import exceptions as exc"},{"line_number":26,"context_line":"from mistral import expressions as expr"},{"line_number":27,"context_line":"from mistral import utils"},{"line_number":28,"context_line":"from mistral.utils import wf_trace"},{"line_number":29,"context_line":"from mistral.workbook import parser as spec_parser"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_48e43ee0","line":26,"range":{"start_line":26,"start_character":35,"end_line":26,"end_character":39},"updated":"2016-05-31 11:27:31.000000000","message":"Why not just leave \u0027expressions\u0027? \u0027expr\u0027 can be misleading.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from mistral.engine import dispatcher"},{"line_number":24,"context_line":"from mistral.engine import policies"},{"line_number":25,"context_line":"from mistral import exceptions as exc"},{"line_number":26,"context_line":"from mistral import expressions as expr"},{"line_number":27,"context_line":"from mistral import utils"},{"line_number":28,"context_line":"from mistral.utils import wf_trace"},{"line_number":29,"context_line":"from mistral.workbook import parser as spec_parser"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_91c92a29","line":26,"range":{"start_line":26,"start_character":35,"end_line":26,"end_character":39},"in_reply_to":"9abb7d3a_48e43ee0","updated":"2016-05-31 13:32:19.000000000","message":"Same explanation as before: tradition )","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        # Explicitly change task state to RUNNING."},{"line_number":260,"context_line":"        # Throw exception if the existing task already succeeded."},{"line_number":261,"context_line":"        if self.task_ex.state \u003d\u003d states.SUCCESS:"},{"line_number":262,"context_line":"            raise exc.MistralError("},{"line_number":263,"context_line":"                \u0027Rerunning succeeded tasks is not supported.\u0027"},{"line_number":264,"context_line":"            )"},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_e8d37202","line":262,"range":{"start_line":262,"start_character":12,"end_line":262,"end_character":35},"updated":"2016-05-31 11:27:31.000000000","message":"More specific exception could be used here - MistralError is not giving any information about the error itself.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        # Explicitly change task state to RUNNING."},{"line_number":260,"context_line":"        # Throw exception if the existing task already succeeded."},{"line_number":261,"context_line":"        if self.task_ex.state \u003d\u003d states.SUCCESS:"},{"line_number":262,"context_line":"            raise exc.MistralError("},{"line_number":263,"context_line":"                \u0027Rerunning succeeded tasks is not supported.\u0027"},{"line_number":264,"context_line":"            )"},{"line_number":265,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_9162ca3c","line":262,"range":{"start_line":262,"start_character":12,"end_line":262,"end_character":35},"in_reply_to":"9abb7d3a_e8d37202","updated":"2016-05-31 13:32:19.000000000","message":"Yes, agree. We will add more errors inheriting MistralError moving forward. The whole big shift is just not done yet. This patch is already big.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            return actions.WorkflowAction(wf_name, task_ex\u003dself.task_ex)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        if not action_name:"},{"line_number":331,"context_line":"            action_name \u003d \u0027std.noop\u0027"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        action_def \u003d actions.resolve_action_definition("},{"line_number":334,"context_line":"            action_name,"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_08d7d6ed","line":331,"range":{"start_line":331,"start_character":26,"end_line":331,"end_character":36},"updated":"2016-05-31 11:27:31.000000000","message":"This string should be stored in a value and refereed here as a value name.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":328,"context_line":"            return actions.WorkflowAction(wf_name, task_ex\u003dself.task_ex)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"        if not action_name:"},{"line_number":331,"context_line":"            action_name \u003d \u0027std.noop\u0027"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        action_def \u003d actions.resolve_action_definition("},{"line_number":334,"context_line":"            action_name,"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_11053a45","line":331,"range":{"start_line":331,"start_character":26,"end_line":331,"end_character":36},"in_reply_to":"9abb7d3a_08d7d6ed","updated":"2016-05-31 13:32:19.000000000","message":"Dunno, it\u0027s a well known thing because it\u0027s a part of standard Mistral installation so it\u0027s not going to change.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        state_info \u003d (None if state \u003d\u003d states.SUCCESS"},{"line_number":357,"context_line":"                      else action_ex.output.get(\u0027result\u0027))"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"        with_items.increase_capacity(self.task_ex)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        if with_items.is_completed(self.task_ex):"},{"line_number":362,"context_line":"            self.complete("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_8801066d","line":359,"range":{"start_line":359,"start_character":0,"end_line":359,"end_character":50},"updated":"2016-05-31 11:27:31.000000000","message":"Could you describe why it is needed to increase capacity on action complete?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        state_info \u003d (None if state \u003d\u003d states.SUCCESS"},{"line_number":357,"context_line":"                      else action_ex.output.get(\u0027result\u0027))"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"        with_items.increase_capacity(self.task_ex)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        if with_items.is_completed(self.task_ex):"},{"line_number":362,"context_line":"            self.complete("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d16bb2ef","line":359,"range":{"start_line":359,"start_character":0,"end_line":359,"end_character":50},"in_reply_to":"9abb7d3a_8801066d","updated":"2016-05-31 13:32:19.000000000","message":"Yes. Basically there\u0027s a notion of capacity which indicates how many actions we\u0027re allowed to run simultaneously. On action completion we have more capacity since this action is running anymore. It will allow us to schedule more actions.\n\nBtw, I mentioned in the commit message that I\u0027d like to get rid of this module with_items completely and rewrite its logic actually. Just not in this patch.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/engine/workflow_handler.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # Workflow is not completed if there are any incomplete task"},{"line_number":46,"context_line":"    # executions that are not in WAITING state. If all incomplete"},{"line_number":47,"context_line":"    # tasks are waiting and there are unhandled errors, then these"},{"line_number":48,"context_line":"    # tasks will not reach completion. In this case, mark the"},{"line_number":49,"context_line":"    # workflow complete."},{"line_number":50,"context_line":"    incomplete_tasks \u003d wf_utils.find_incomplete_task_executions(wf_ex)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_c83e6e27","line":47,"range":{"start_line":47,"start_character":28,"end_line":47,"end_character":54},"updated":"2016-05-31 11:27:31.000000000","message":"Should it be \u0027there are no unhandled errors\u0027?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    # supported."},{"line_number":194,"context_line":"    # TODO(rakhmerov): I don\u0027t like this hardcoded special case. It\u0027s"},{"line_number":195,"context_line":"    # used only to continue the workflow (rerun) but at the first glance"},{"line_number":196,"context_line":"    # seems like a generic behavior. Need to handler it differently."},{"line_number":197,"context_line":"    if set_upstream and state \u003d\u003d states.RUNNING and wf_ex.task_execution_id:"},{"line_number":198,"context_line":"        task_ex \u003d db_api.get_task_execution(wf_ex.task_execution_id)"},{"line_number":199,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_084516b2","line":196,"range":{"start_line":196,"start_character":45,"end_line":196,"end_character":52},"updated":"2016-05-31 11:27:31.000000000","message":"handle","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    # supported."},{"line_number":194,"context_line":"    # TODO(rakhmerov): I don\u0027t like this hardcoded special case. It\u0027s"},{"line_number":195,"context_line":"    # used only to continue the workflow (rerun) but at the first glance"},{"line_number":196,"context_line":"    # seems like a generic behavior. Need to handler it differently."},{"line_number":197,"context_line":"    if set_upstream and state \u003d\u003d states.RUNNING and wf_ex.task_execution_id:"},{"line_number":198,"context_line":"        task_ex \u003d db_api.get_task_execution(wf_ex.task_execution_id)"},{"line_number":199,"context_line":""}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_b4237ca6","line":196,"range":{"start_line":196,"start_character":45,"end_line":196,"end_character":52},"in_reply_to":"9abb7d3a_084516b2","updated":"2016-05-31 13:32:19.000000000","message":"Ok, will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/services/scheduler.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                    # Call the method."},{"line_number":177,"context_line":"                    target_method(**method_args)"},{"line_number":178,"context_line":"                except Exception as e:"},{"line_number":179,"context_line":"                    LOG.error("},{"line_number":180,"context_line":"                        \"Delayed call failed [exception\u003d%s]: %s\","},{"line_number":181,"context_line":"                        e,"},{"line_number":182,"context_line":"                        traceback.format_exc(e)"},{"line_number":183,"context_line":"                    )"},{"line_number":184,"context_line":"                finally:"},{"line_number":185,"context_line":"                    # Remove context."},{"line_number":186,"context_line":"                    context.set_ctx(None)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_888626d9","line":183,"range":{"start_line":179,"start_character":20,"end_line":183,"end_character":21},"updated":"2016-05-31 11:27:31.000000000","message":"You can use LOG.exception instead of LOG.error. Then oslo.logging will automatically display traceback. You can check it out here: http://docs.openstack.org/developer/oslo.log/examples.html#usage-py","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                    # Call the method."},{"line_number":177,"context_line":"                    target_method(**method_args)"},{"line_number":178,"context_line":"                except Exception as e:"},{"line_number":179,"context_line":"                    LOG.error("},{"line_number":180,"context_line":"                        \"Delayed call failed [exception\u003d%s]: %s\","},{"line_number":181,"context_line":"                        e,"},{"line_number":182,"context_line":"                        traceback.format_exc(e)"},{"line_number":183,"context_line":"                    )"},{"line_number":184,"context_line":"                finally:"},{"line_number":185,"context_line":"                    # Remove context."},{"line_number":186,"context_line":"                    context.set_ctx(None)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_747bd48c","line":183,"range":{"start_line":179,"start_character":20,"end_line":183,"end_character":21},"in_reply_to":"9abb7d3a_888626d9","updated":"2016-05-31 13:32:19.000000000","message":"Ok, will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/tests/unit/engine/base.py":[{"author":{"_account_id":8592,"name":"Anastasia Kuznetsova","email":"akuznetsova@mirantis.com","username":"nastya-kuz"},"change_message_id":"812d7f2280824b7235df0753c496ef05fca65801","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @staticmethod"},{"line_number":131,"context_line":"    def print_workflow_executions(exc_info):"},{"line_number":132,"context_line":"        print(\"\\nEngine test case exception occurred: %s\" % exc_info[1])"},{"line_number":133,"context_line":"        print(\"Exception type: %s\" % exc_info[0])"},{"line_number":134,"context_line":"        print(\"\\nPrinting workflow executions...\")"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_538fbdcb","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":13},"updated":"2016-05-31 15:47:40.000000000","message":"Why do we use print here?","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"84d68dadd7bb5089dfec2d879e7290782e865d76","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    @staticmethod"},{"line_number":131,"context_line":"    def print_workflow_executions(exc_info):"},{"line_number":132,"context_line":"        print(\"\\nEngine test case exception occurred: %s\" % exc_info[1])"},{"line_number":133,"context_line":"        print(\"Exception type: %s\" % exc_info[0])"},{"line_number":134,"context_line":"        print(\"\\nPrinting workflow executions...\")"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_69e65245","line":132,"range":{"start_line":132,"start_character":8,"end_line":132,"end_character":13},"in_reply_to":"9abb7d3a_538fbdcb","updated":"2016-05-31 16:04:37.000000000","message":"It has a special use: it runs only if some test fails. If it happens we can easily see it in the console and hence use it as a debug info when fixing a problem. I found it very convenient. Other than that, it doesn\u0027t appear anywhere else.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8592,"name":"Anastasia Kuznetsova","email":"akuznetsova@mirantis.com","username":"nastya-kuz"},"change_message_id":"812d7f2280824b7235df0753c496ef05fca65801","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        wf_execs \u003d db_api.get_workflow_executions()"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        for w in wf_execs:"},{"line_number":139,"context_line":"            print("},{"line_number":140,"context_line":"                \"\\n%s [state\u003d%s, state_info\u003d%s, output\u003d%s]\" %"},{"line_number":141,"context_line":"                (w.name, w.state, w.state_info, w.output)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_53721d4a","line":138,"range":{"start_line":138,"start_character":12,"end_line":138,"end_character":13},"updated":"2016-05-31 15:47:40.000000000","message":"I am not a fan of such kind of variables naming. It decreases readability.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"84d68dadd7bb5089dfec2d879e7290782e865d76","unresolved":false,"context_lines":[{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        wf_execs \u003d db_api.get_workflow_executions()"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        for w in wf_execs:"},{"line_number":139,"context_line":"            print("},{"line_number":140,"context_line":"                \"\\n%s [state\u003d%s, state_info\u003d%s, output\u003d%s]\" %"},{"line_number":141,"context_line":"                (w.name, w.state, w.state_info, w.output)"}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_4968ae87","line":138,"range":{"start_line":138,"start_character":12,"end_line":138,"end_character":13},"in_reply_to":"9abb7d3a_53721d4a","updated":"2016-05-31 16:04:37.000000000","message":"It\u0027s a local variable within a very limited context, small method. I agree that we need to use longer names in case of field names, global names etc.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"}],"mistral/tests/unit/engine/test_direct_workflow.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_689782ec","line":489,"range":{"start_line":489,"start_character":32,"end_line":489,"end_character":37},"updated":"2016-05-31 11:27:31.000000000","message":"If you change this from SUCCESS to ERROR, you should also change it in comment in line 481","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_14b2484c","line":489,"range":{"start_line":489,"start_character":32,"end_line":489,"end_character":37},"in_reply_to":"9abb7d3a_689782ec","updated":"2016-05-31 13:32:19.000000000","message":"Yes, will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8592,"name":"Anastasia Kuznetsova","email":"akuznetsova@mirantis.com","username":"nastya-kuz"},"change_message_id":"812d7f2280824b7235df0753c496ef05fca65801","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_b360a9a6","line":489,"updated":"2016-05-31 15:47:40.000000000","message":"Do I undestand correct that the behaviour was changed: now if task is executed, but it has invalid transitions (like in the test), then it goes to \"ERROR\"?","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"change_message_id":"cbc7b499f5e3eb724df6152778c733b8badd192c","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_29cd6a66","line":489,"in_reply_to":"9abb7d3a_09418638","updated":"2016-05-31 16:07:09.000000000","message":"I guess the wrong definition is the reference to unexisting yaql function but Mistral doesn\u0027t have such detail validation mechanism for yaql, isn\u0027t it?","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"5e9ded86c8e6d2ee2a44fa8f2ce214a7c65e7ca3","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_49454eb1","line":489,"in_reply_to":"9abb7d3a_09418638","updated":"2016-05-31 16:09:35.000000000","message":"Nikolay, no, please think about it carefully. When you say task succeeded what exactly do you mean? You mean that its action succeeded. And that will be reflected in SUCCESS state of an action. But task should be considered successful only if its actions succeeded AND all other stuff that happens upon actions completion runs well, such as publishing and calculating transitions. If we had a different behavior before that doesn\u0027t mean it was semantically right.\n\nThat way we can also see more exactly where the problem happened, task1 because in its definition YAQL expression is corrupted.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"1281c0c03e8e2f70140bf73cb56bd3007964c8b0","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_64c31939","line":489,"in_reply_to":"9abb7d3a_44ba550b","updated":"2016-05-31 16:21:45.000000000","message":"More on that. We didn\u0027t pay a lot of attention before on how errors should be handled. Now I have clear understanding that it should be splitted into multiple levels: action, task and workflow. If, for example, action is successful but something is wrong at task level then we shouldn\u0027t rollback the transaction, we just need to adjust state of task and workflow correspondingly. Same with task-\u003eworkflow relationship. If task failed we should drop its result, we just need to modify its state and workflow state.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"change_message_id":"4d9b44a4c19a305236ed7ceba473f19389256cdc","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_44fff5a9","line":489,"in_reply_to":"9abb7d3a_44ba550b","updated":"2016-05-31 16:19:12.000000000","message":"Ok, makes sense, thanks for clarification.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8592,"name":"Anastasia Kuznetsova","email":"akuznetsova@mirantis.com","username":"nastya-kuz"},"change_message_id":"f426e6e0bd4afe64e59e11e5dcbe5ed49e829d66","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_44ba550b","line":489,"in_reply_to":"9abb7d3a_49454eb1","updated":"2016-05-31 16:17:38.000000000","message":"Thanks for an explanation","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"change_message_id":"7c04667909611ad1f16291a6e70651627b766c1d","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_a4018107","line":489,"in_reply_to":"9abb7d3a_49454eb1","updated":"2016-05-31 16:13:28.000000000","message":"hmmm. What error message in task.state_info do we get in that case? (about its failure)","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"b8252b612ef556b616016241bc24a186dcb340c7","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_84327d5d","line":489,"in_reply_to":"9abb7d3a_64c31939","updated":"2016-05-31 16:22:55.000000000","message":"... we should not drop ...\n\nThanks","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"adfddf4a2d740229e5151a59aeef8500f43b78de","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_24447148","line":489,"in_reply_to":"9abb7d3a_a4018107","updated":"2016-05-31 16:15:20.000000000","message":"\"Can not evaluate YAQL expression...\", may be also wrapped into something else but it will contain that.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"84d68dadd7bb5089dfec2d879e7290782e865d76","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_09418638","line":489,"in_reply_to":"9abb7d3a_b360a9a6","updated":"2016-05-31 16:04:37.000000000","message":"Yes, because the problem is in its definition.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"},{"author":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"change_message_id":"51f12a65a8462e715e4ce80ca3c831d63a846a27","unresolved":false,"context_lines":[{"line_number":486,"context_line":"            name\u003d\u0027task1\u0027"},{"line_number":487,"context_line":"        )"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        self.assertEqual(states.ERROR, task_1_ex.state)"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        # Assert that there is only one action execution and it\u0027s SUCCESS."},{"line_number":492,"context_line":"        task_1_action_exs \u003d db_api.get_action_executions("}],"source_content_type":"text/x-python","patch_set":43,"id":"9abb7d3a_896576b6","line":489,"in_reply_to":"9abb7d3a_b360a9a6","updated":"2016-05-31 16:04:02.000000000","message":"ooh. Agree with Nastya. Logically task1 is succeed, why it should be in ERROR? I like the previous behavior much more. Probably, this change breaks that logic.","commit_id":"816bfd9dcc63dad735e6429c6f952147c88c159b"}],"mistral/tests/unit/engine/test_error_handling.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        self._assert_single_item(task_execs, name\u003d\u0027task1\u0027, state\u003dstates.ERROR)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # TODO(rakhmerov): Finish. Need to check more complicated cases."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_task_error(self):"},{"line_number":56,"context_line":"        # TODO(rakhmerov): Implement."},{"line_number":57,"context_line":"        pass"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_workflow_error(self):"},{"line_number":60,"context_line":"        # TODO(rakhmerov): Implement."},{"line_number":61,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_48c39eed","line":61,"range":{"start_line":53,"start_character":0,"end_line":61,"end_character":12},"updated":"2016-05-31 11:27:31.000000000","message":"Would it be better to finish this tests before merging to make sure that engine refactoring didn\u0027t brake anything?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        self._assert_single_item(task_execs, name\u003d\u0027task1\u0027, state\u003dstates.ERROR)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        # TODO(rakhmerov): Finish. Need to check more complicated cases."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_task_error(self):"},{"line_number":56,"context_line":"        # TODO(rakhmerov): Implement."},{"line_number":57,"context_line":"        pass"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_workflow_error(self):"},{"line_number":60,"context_line":"        # TODO(rakhmerov): Implement."},{"line_number":61,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_f4cd0486","line":61,"range":{"start_line":53,"start_character":0,"end_line":61,"end_character":12},"in_reply_to":"9abb7d3a_48c39eed","updated":"2016-05-31 13:32:19.000000000","message":"Dawid, seriously, no :) The primary goal of this patch was to make a fundamental refactoring related to engine and make all tests pass. As a mentioned above, \"error handling\" BP is not yet fully completed (that\u0027s why commit message has \"Partially implements: blueprint...\". So I would prefer to fill these test separately since it\u0027s too much for this patch.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}],"mistral/utils/__init__.py":[{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"def cut_by_kb(data, kilobytes):"},{"line_number":174,"context_line":"    if kilobytes \u003e 0:"},{"line_number":175,"context_line":"        return cut(data)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    bytes_per_char \u003d sys.getsizeof(\u0027s\u0027) - sys.getsizeof(\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_c8af8e28","line":174,"range":{"start_line":174,"start_character":7,"end_line":174,"end_character":20},"updated":"2016-05-31 11:27:31.000000000","message":"It should be kilobytes \u003c\u003d 0, shouldn\u0027t it?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"def cut_by_kb(data, kilobytes):"},{"line_number":174,"context_line":"    if kilobytes \u003e 0:"},{"line_number":175,"context_line":"        return cut(data)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    bytes_per_char \u003d sys.getsizeof(\u0027s\u0027) - sys.getsizeof(\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_34c02c2a","line":174,"range":{"start_line":174,"start_character":7,"end_line":174,"end_character":20},"in_reply_to":"9abb7d3a_c8af8e28","updated":"2016-05-31 13:32:19.000000000","message":"Ok, will fix it.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"change_message_id":"eb9921ac5c587fdadeffbe318ba71041266b6c4d","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    if kilobytes \u003e 0:"},{"line_number":175,"context_line":"        return cut(data)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    bytes_per_char \u003d sys.getsizeof(\u0027s\u0027) - sys.getsizeof(\u0027\u0027)"},{"line_number":178,"context_line":"    length \u003d int(kilobytes * 1024 / bytes_per_char)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    return cut(data, length)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_08a6b652","line":177,"range":{"start_line":177,"start_character":21,"end_line":177,"end_character":59},"updated":"2016-05-31 11:27:31.000000000","message":"Shouldn\u0027t this be consider a constant? Or it can differ between OSes?","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"a60fe19ea9dc86016fbaed5dd3934c3c83e6b375","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    if kilobytes \u003e 0:"},{"line_number":175,"context_line":"        return cut(data)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    bytes_per_char \u003d sys.getsizeof(\u0027s\u0027) - sys.getsizeof(\u0027\u0027)"},{"line_number":178,"context_line":"    length \u003d int(kilobytes * 1024 / bytes_per_char)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    return cut(data, length)"}],"source_content_type":"text/x-python","patch_set":41,"id":"9abb7d3a_d4e560dc","line":177,"range":{"start_line":177,"start_character":21,"end_line":177,"end_character":59},"in_reply_to":"9abb7d3a_08a6b652","updated":"2016-05-31 13:32:19.000000000","message":"Unlikely but I assumed it can be different.","commit_id":"93b92bc83e337bb6e9c42abb02c1b194e81e03f3"}]}
