)]}'
{"ironic/common/states.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"2649515715a12f1b341b602420c82505b65628e4","unresolved":false,"context_lines":[{"line_number":192,"context_line":"DELETE_ALLOWED_STATES \u003d (AVAILABLE, MANAGEABLE, ENROLL, ADOPTFAIL)"},{"line_number":193,"context_line":"\"\"\"States in which node deletion is allowed.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"STABLE_STATES \u003d (MANAGEABLE, AVAILABLE, ACTIVE)"},{"line_number":196,"context_line":"\"\"\"States which can be used as \u0027target\u0027.\"\"\""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_e8713b0d","line":195,"updated":"2016-10-16 20:16:36.000000000","message":"Is ENROLL not considered a \"stable state\"?\n\nIn docs here, \"stable states\" are anything where transitions from them must be initiated via an API request:\n\nhttp://docs.openstack.org/developer/ironic/dev/states.html\n\nIt makes sense to me that ENROLL wouldn\u0027t be in this part of the code now, since it\u0027s not a target, but maybe we should either add a note here about it or update the docs to change what we define as a \"stable state.\"\n\nThoughts?","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":7080,"name":"Mathieu Mitchell","email":"mat128@gmail.com","username":"mat128"},"change_message_id":"1c716be974d20d51b5dc1eb29dbaf13d84674326","unresolved":false,"context_lines":[{"line_number":192,"context_line":"DELETE_ALLOWED_STATES \u003d (AVAILABLE, MANAGEABLE, ENROLL, ADOPTFAIL)"},{"line_number":193,"context_line":"\"\"\"States in which node deletion is allowed.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"STABLE_STATES \u003d (MANAGEABLE, AVAILABLE, ACTIVE)"},{"line_number":196,"context_line":"\"\"\"States which can be used as \u0027target\u0027.\"\"\""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_26012d6b","line":195,"in_reply_to":"fa6399be_3555f51c","updated":"2016-10-19 11:52:00.000000000","message":"+1","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"51afc9622575d621be823bf5f345029150a265b9","unresolved":false,"context_lines":[{"line_number":192,"context_line":"DELETE_ALLOWED_STATES \u003d (AVAILABLE, MANAGEABLE, ENROLL, ADOPTFAIL)"},{"line_number":193,"context_line":"\"\"\"States in which node deletion is allowed.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"STABLE_STATES \u003d (MANAGEABLE, AVAILABLE, ACTIVE)"},{"line_number":196,"context_line":"\"\"\"States which can be used as \u0027target\u0027.\"\"\""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_3555f51c","line":195,"in_reply_to":"fa6399be_79dfaed5","updated":"2016-10-18 18:55:38.000000000","message":"+1, let\u0027s update docs here before this lands.\n\nI think reviewers will find this a little less confusing for future patch sets involving stable states if we make the docs match the reality in the code.","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"f205338f1dcf2a945d534a99c53b046337ea194d","unresolved":false,"context_lines":[{"line_number":192,"context_line":"DELETE_ALLOWED_STATES \u003d (AVAILABLE, MANAGEABLE, ENROLL, ADOPTFAIL)"},{"line_number":193,"context_line":"\"\"\"States in which node deletion is allowed.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"STABLE_STATES \u003d (MANAGEABLE, AVAILABLE, ACTIVE)"},{"line_number":196,"context_line":"\"\"\"States which can be used as \u0027target\u0027.\"\"\""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_79dfaed5","line":195,"in_reply_to":"fa6399be_e6cf9926","updated":"2016-10-17 12:30:05.000000000","message":"Only states that may be set as \u0027target\u0027 are considered as \u0027stable\u0027. ENROLL and ERROR are not stable states according to this https://review.openstack.org/#/c/385459/2/ironic/common/fsm.py@87\n\n+1 to update documentation and regenerate states svg in this patch also.","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"1b03fafb73d97f48f25994c40a166a47213e79a1","unresolved":false,"context_lines":[{"line_number":192,"context_line":"DELETE_ALLOWED_STATES \u003d (AVAILABLE, MANAGEABLE, ENROLL, ADOPTFAIL)"},{"line_number":193,"context_line":"\"\"\"States in which node deletion is allowed.\"\"\""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"STABLE_STATES \u003d (MANAGEABLE, AVAILABLE, ACTIVE)"},{"line_number":196,"context_line":"\"\"\"States which can be used as \u0027target\u0027.\"\"\""},{"line_number":197,"context_line":""},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa6399be_e6cf9926","line":195,"in_reply_to":"fa6399be_e8713b0d","updated":"2016-10-17 11:44:05.000000000","message":"\"In docs here, \"stable states\" are anything where transitions from them must be initiated via an API request...\"\n\nThis docs looks outdated. Transitions from DEPLOYFAIL, CLEANFAIL, INSPECTFAIL, ADOPTFAIL must be initiated via an API request, but they are not stable in the code. Maybe we should update doc also, but this patch fixes only FSM validation.","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"}],"ironic/conductor/task_manager.py":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"fabdfb11010eb63793cb64e03510b61d22dff6bf","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # NOTE(yuriyz): FSM clears target_state if stable state is reached,"},{"line_number":401,"context_line":"        # we should also clear for ENROLL and ERROR states."},{"line_number":402,"context_line":"        if self.node.provision_state in (states.ENROLL, states.ERROR):"},{"line_number":403,"context_line":"            self.node.target_provision_state \u003d states.NOSTATE"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            self.node.target_provision_state \u003d self.fsm.target_state"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a77a97e_0958c4f9","line":402,"updated":"2016-11-22 18:10:37.000000000","message":"So you assume that callback should not do anything with the node\u0027s states? As previously it seems that was the intent why the callback presence was checked before clearing target state.","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"f0e9d8546c81e60095f8bd7c42303f7ad3bb506a","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        # NOTE(yuriyz): FSM clears target_state if stable state is reached,"},{"line_number":401,"context_line":"        # we should also clear for ENROLL and ERROR states."},{"line_number":402,"context_line":"        if self.node.provision_state in (states.ENROLL, states.ERROR):"},{"line_number":403,"context_line":"            self.node.target_provision_state \u003d states.NOSTATE"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            self.node.target_provision_state \u003d self.fsm.target_state"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_d2b25305","line":402,"in_reply_to":"7a77a97e_0958c4f9","updated":"2016-11-29 15:27:04.000000000","message":"I agree with vdrok\u0027s comment, good catch. There are actually places like [0] where the target state is explicitly kept. If the FSM doesn\u0027t enforce this clearing of target state of ENROLL and ERROR, which it appears it doesn\u0027t, then we probably shouldn\u0027t here, either.\n\n[0] https://github.com/openstack/ironic/blob/8cc063173e7bb5f81dbb956b44dd81c2162a75b7/ironic/conductor/manager.py#L1344","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"099eff9df73d1f938e56c59aeec7ca396aacfc9e","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        # we should also clear for ENROLL and ERROR states."},{"line_number":402,"context_line":"        if self.node.provision_state in (states.ENROLL, states.ERROR):"},{"line_number":403,"context_line":"            self.node.target_provision_state \u003d states.NOSTATE"},{"line_number":404,"context_line":"        else:"},{"line_number":405,"context_line":"            self.node.target_provision_state \u003d self.fsm.target_state"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        # set up the async worker"},{"line_number":408,"context_line":"        if callback:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_3200e733","line":405,"range":{"start_line":404,"start_character":0,"end_line":405,"end_character":68},"updated":"2016-11-29 15:34:59.000000000","message":"I think this wipes out the difference between manual cleaning and automated cleaning as part of the machine workflow, since the target state is preserved for the resumption of cleaning.","commit_id":"4df2a557c004e5d7f1a7000e15a274f9fabb43d9"}]}
