)]}'
{"tripleo_common/actions/parameters.py":[{"author":{"_account_id":18575,"name":"Saravanan KR","email":"krsacme@gmail.com","username":"saravanankr"},"change_message_id":"05445df500ef387048bab67b6371e85ab78ffada","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        orc \u003d self.get_orchestration_client(context)"},{"line_number":566,"context_line":"        preview_data \u003d orc.stacks.preview(**fields)"},{"line_number":567,"context_line":"        try:"},{"line_number":568,"context_line":"            result \u003d self.get_network_config(preview_data, self.container,"},{"line_number":569,"context_line":"                                             self.role_name)"},{"line_number":570,"context_line":"            return result"},{"line_number":571,"context_line":"        except exception.DeriveParamsError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff346bd7_6b1ff307","line":568,"range":{"start_line":568,"start_character":12,"end_line":568,"end_character":18},"updated":"2017-07-28 10:34:22.000000000","message":"Do we need exception? Why cant we check for valid \u0027result\u0027 to return error?","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"c75e88ac734c78033376ec60a1d334139feb826b","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        orc \u003d self.get_orchestration_client(context)"},{"line_number":566,"context_line":"        preview_data \u003d orc.stacks.preview(**fields)"},{"line_number":567,"context_line":"        try:"},{"line_number":568,"context_line":"            result \u003d self.get_network_config(preview_data, self.container,"},{"line_number":569,"context_line":"                                             self.role_name)"},{"line_number":570,"context_line":"            return result"},{"line_number":571,"context_line":"        except exception.DeriveParamsError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_36e69d46","line":568,"range":{"start_line":568,"start_character":12,"end_line":568,"end_character":18},"in_reply_to":"df3967d1_fc1da2bd","updated":"2017-07-31 12:02:29.000000000","message":"OK, thanks!","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":22865,"name":"Jaganathan Palanisamy","email":"jpalanis@redhat.com","username":"jaganathancse"},"change_message_id":"c755a593aa5ed759e7b151b4ffc08c262cc63d81","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        orc \u003d self.get_orchestration_client(context)"},{"line_number":566,"context_line":"        preview_data \u003d orc.stacks.preview(**fields)"},{"line_number":567,"context_line":"        try:"},{"line_number":568,"context_line":"            result \u003d self.get_network_config(preview_data, self.container,"},{"line_number":569,"context_line":"                                             self.role_name)"},{"line_number":570,"context_line":"            return result"},{"line_number":571,"context_line":"        except exception.DeriveParamsError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff346bd7_abacbb9e","line":568,"range":{"start_line":568,"start_character":12,"end_line":568,"end_character":18},"in_reply_to":"ff346bd7_6b1ff307","updated":"2017-07-28 10:40:05.000000000","message":"None is not valid return data in get_network_config method, so get_network_config method raises exception when unable to determine network config for any role.\nso i think, raising exception will be better instead of none check.","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8d564610e9a9f47aa4e83470b5cda1f9fafc3a85","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        orc \u003d self.get_orchestration_client(context)"},{"line_number":566,"context_line":"        preview_data \u003d orc.stacks.preview(**fields)"},{"line_number":567,"context_line":"        try:"},{"line_number":568,"context_line":"            result \u003d self.get_network_config(preview_data, self.container,"},{"line_number":569,"context_line":"                                             self.role_name)"},{"line_number":570,"context_line":"            return result"},{"line_number":571,"context_line":"        except exception.DeriveParamsError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff346bd7_dc510738","line":568,"range":{"start_line":568,"start_character":12,"end_line":568,"end_character":18},"in_reply_to":"ff346bd7_abacbb9e","updated":"2017-07-28 13:07:51.000000000","message":"I can see why it makes sense for get_network_config to raise an exception, but I also take Saravanan\u0027s point. Is it necessary for this code to catch the exception and return an error, or can we just allow the exception to propagate? I thought it\u0027s OK for mistral actions to raise exceptions, and when this happens the action fails (which is what we want it to do).","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":22865,"name":"Jaganathan Palanisamy","email":"jpalanis@redhat.com","username":"jaganathancse"},"change_message_id":"49a7da6c90aaf117b323bc7d73629605c85d0aad","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        orc \u003d self.get_orchestration_client(context)"},{"line_number":566,"context_line":"        preview_data \u003d orc.stacks.preview(**fields)"},{"line_number":567,"context_line":"        try:"},{"line_number":568,"context_line":"            result \u003d self.get_network_config(preview_data, self.container,"},{"line_number":569,"context_line":"                                             self.role_name)"},{"line_number":570,"context_line":"            return result"},{"line_number":571,"context_line":"        except exception.DeriveParamsError as err:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_fc1da2bd","line":568,"range":{"start_line":568,"start_character":12,"end_line":568,"end_character":18},"in_reply_to":"ff346bd7_dc510738","updated":"2017-07-31 04:34:54.000000000","message":"I think, We cannot just propagate exception in action file. mostly they are using actions.Result to fail the action which will invoke on-error in calling task.\nand here not just return an error, using actions.Result to fail the action.\n\nIn case later, need to add any additional validations, then only require to raise exception is validation place. please look into line  448 in GetProfileOfFlavorAction.","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8d564610e9a9f47aa4e83470b5cda1f9fafc3a85","unresolved":false,"context_lines":[{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def get_network_config(self, preview_data, stack_name, role_name):"},{"line_number":576,"context_line":"        result \u003d None"},{"line_number":577,"context_line":"        if preview_data:"},{"line_number":578,"context_line":"            for res in preview_data.resources:"},{"line_number":579,"context_line":"                net_script \u003d self.process_preview_list(res,"},{"line_number":580,"context_line":"                                                       stack_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff346bd7_3c2db3a7","line":577,"updated":"2017-07-28 13:07:51.000000000","message":"Possibly a nit, but this conditional could be done by the caller. It looks awkward here, where it essentially \"wraps\" most of the code.","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"c75e88ac734c78033376ec60a1d334139feb826b","unresolved":false,"context_lines":[{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def get_network_config(self, preview_data, stack_name, role_name):"},{"line_number":576,"context_line":"        result \u003d None"},{"line_number":577,"context_line":"        if preview_data:"},{"line_number":578,"context_line":"            for res in preview_data.resources:"},{"line_number":579,"context_line":"                net_script \u003d self.process_preview_list(res,"},{"line_number":580,"context_line":"                                                       stack_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_168ef977","line":577,"in_reply_to":"df3967d1_5c5f0e41","updated":"2017-07-31 12:02:29.000000000","message":"OK, thanks for explanation","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":22865,"name":"Jaganathan Palanisamy","email":"jpalanis@redhat.com","username":"jaganathancse"},"change_message_id":"49a7da6c90aaf117b323bc7d73629605c85d0aad","unresolved":false,"context_lines":[{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def get_network_config(self, preview_data, stack_name, role_name):"},{"line_number":576,"context_line":"        result \u003d None"},{"line_number":577,"context_line":"        if preview_data:"},{"line_number":578,"context_line":"            for res in preview_data.resources:"},{"line_number":579,"context_line":"                net_script \u003d self.process_preview_list(res,"},{"line_number":580,"context_line":"                                                       stack_name,"}],"source_content_type":"text/x-python","patch_set":1,"id":"df3967d1_5c5f0e41","line":577,"in_reply_to":"ff346bd7_3c2db3a7","updated":"2017-07-31 04:34:54.000000000","message":"i think, if we move condition to caller method, need else condition there to fail the action which will also avoid propagating exception and cacthing part but if require additional validation in later, may need to update the code in caller method like this.\ncurrenly we used try block in caller for entire logic, so which will also take care any other exceptions(currently no possible exception like that).","commit_id":"a89b5a4dab1abdbb51f39f237f2a37eca5dea210"},{"author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"change_message_id":"eaf3445a2ac136b96b08db3b60ddb63ec19867f7","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                             self.role_name)"},{"line_number":573,"context_line":"            return result"},{"line_number":574,"context_line":"        except exception.DeriveParamsError as err:"},{"line_number":575,"context_line":"            LOG.error(\u0027Derive Params Error: %s\u0027, err)"},{"line_number":576,"context_line":"            return actions.Result(error\u003dstr(err))"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def get_network_config(self, preview_data, stack_name, role_name):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_993b2837","line":575,"range":{"start_line":575,"start_character":16,"end_line":575,"end_character":21},"updated":"2017-12-12 08:12:17.000000000","message":"LOG.exception will add the full traceback to the log - would that be better here?","commit_id":"05e0f96a2f24416bf39469060f44b5974cc4340a"},{"author":{"_account_id":22865,"name":"Jaganathan Palanisamy","email":"jpalanis@redhat.com","username":"jaganathancse"},"change_message_id":"18815f3049468a5989966ee34033f919d9b14dac","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                                             self.role_name)"},{"line_number":573,"context_line":"            return result"},{"line_number":574,"context_line":"        except exception.DeriveParamsError as err:"},{"line_number":575,"context_line":"            LOG.error(\u0027Derive Params Error: %s\u0027, err)"},{"line_number":576,"context_line":"            return actions.Result(error\u003dstr(err))"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def get_network_config(self, preview_data, stack_name, role_name):"}],"source_content_type":"text/x-python","patch_set":5,"id":"df87a7cf_1625e008","line":575,"range":{"start_line":575,"start_character":16,"end_line":575,"end_character":21},"in_reply_to":"ff82abbf_993b2837","updated":"2017-12-12 08:52:36.000000000","message":"Done","commit_id":"05e0f96a2f24416bf39469060f44b5974cc4340a"}]}
