)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"1b1c632cf99fb9cdb29ea0bdc49b30abd67e5d88","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When create redis instance with a configuration which has requirepass"},{"line_number":10,"context_line":"value(not \u0027foobared\u0027). Period task still use old password to poll instance\u0027s"},{"line_number":11,"context_line":"status, so instance create failed because authentication error. Rewrite restart"},{"line_number":12,"context_line":" method in service.py to rebuild amdin client during db restart."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I27598a9031efcb74821f77c36710d224b3a9a0e6"},{"line_number":15,"context_line":"Closes-bug: #1733815"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"ff82abbf_7b87e3b6","line":12,"range":{"start_line":11,"start_character":64,"end_line":12,"end_character":64},"updated":"2017-11-24 08:31:15.000000000","message":"This should also be updated to follow how this patch actually works.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"}],"trove/guestagent/datastore/experimental/redis/manager.py":[{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"1b1c632cf99fb9cdb29ea0bdc49b30abd67e5d88","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    def apply_overrides_on_prepare(self, context, overrides):"},{"line_number":128,"context_line":"        \"\"\"Override logic in parent class\"\"\""},{"line_number":129,"context_line":"        self._app.update_overrides(context, overrides)"},{"line_number":130,"context_line":"        self._app.apply_overrides(self._app.admin, overrides)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":133,"context_line":"        LOG.debug(\"Updating overrides.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_bba1eb48","line":130,"updated":"2017-11-24 08:31:15.000000000","message":"We should also call restart here.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"cec5a1c9dca21f772802015b6b58c66e8fc66bea","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    def apply_overrides_on_prepare(self, context, overrides):"},{"line_number":128,"context_line":"        \"\"\"Override logic in parent class\"\"\""},{"line_number":129,"context_line":"        self._app.update_overrides(context, overrides)"},{"line_number":130,"context_line":"        self._app.apply_overrides(self._app.admin, overrides)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":133,"context_line":"        LOG.debug(\"Updating overrides.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_ccf513ef","line":130,"in_reply_to":"ff82abbf_0c069b1e","updated":"2017-11-24 10:13:30.000000000","message":"I was thinking do the trick in RedisApp.update_overrides like this:\n...\n        if overrides:\n            self.configuration_manager.apply_user_override(overrides)\n            if \u0027requirepass\u0027 in overrides:\n                self.apply_overrides(self.admin, {                                                                                                \n                    \u0027requirepass\u0027: overrides[\u0027requirepass\u0027]                                                                                       \n                    }) \n...\n\nBut \u0027requirepass\u0027 will be applied twice (this shouldn\u0027t fail anything, but still not good).\n\nFor the approach of modifying Redis Manager restart(), there are 2 things to improve:\n1. unnecessary admin client refreshing should be avoided;\n2. most of the code is the same of BaseDbStatus.restart_db_service(), more work should be done to reduce code duplication.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"bbfa54aec0e0363231f80d05fdfe8516c703c986","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    def apply_overrides_on_prepare(self, context, overrides):"},{"line_number":128,"context_line":"        \"\"\"Override logic in parent class\"\"\""},{"line_number":129,"context_line":"        self._app.update_overrides(context, overrides)"},{"line_number":130,"context_line":"        self._app.apply_overrides(self._app.admin, overrides)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":133,"context_line":"        LOG.debug(\"Updating overrides.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_c6b412d4","line":130,"in_reply_to":"ff82abbf_bba1eb48","updated":"2017-11-24 09:02:25.000000000","message":"\u003e We should also call restart here.\n\nHi Zhao. I think there is no need to call restart here. If original code, because update_overrides only modify config in file, so restart db to apply configuration at runtime. Here apply_override also play role of apply configuration at runtime, and avoid to overrides restart() to support rebuild admin client","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"e81faa45ecb3828bc5cf7f135d99a22cf1bed62f","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    def apply_overrides_on_prepare(self, context, overrides):"},{"line_number":128,"context_line":"        \"\"\"Override logic in parent class\"\"\""},{"line_number":129,"context_line":"        self._app.update_overrides(context, overrides)"},{"line_number":130,"context_line":"        self._app.apply_overrides(self._app.admin, overrides)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":133,"context_line":"        LOG.debug(\"Updating overrides.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_e1902046","line":130,"in_reply_to":"ff82abbf_c6b412d4","updated":"2017-11-24 09:38:29.000000000","message":"So there must be some other problems here we were not taking into account.\n\nRedis should be restarted here because we cannot determine whether there are some configuration parameters cannot be applied at runtime (have a look at Redis configuration template, you will find some of them).\n\nThen this problem is not only about create a Redis instance with a configuration group containing \u0027requirepass\u0027. Assigning a configuration group containing \u0027requirepass\u0027 and other need_restart parameters will fail too (this kind of configuration group won\u0027t be applied at runtime, so a restart is needed, when restarting, admin client will not be refreshed neither).\n\nFor the above reason, I will suggest modifying RedisApp.update_overrides() (this approach will be simple). Modifying Redis Manager restart() in the patch set 11 is quite straightforward, but should be changed to avoid unnecessary admin client refreshing.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"7940a2a5bb9099d406ee2ea5c2fee0f0c978581b","unresolved":false,"context_lines":[{"line_number":127,"context_line":"    def apply_overrides_on_prepare(self, context, overrides):"},{"line_number":128,"context_line":"        \"\"\"Override logic in parent class\"\"\""},{"line_number":129,"context_line":"        self._app.update_overrides(context, overrides)"},{"line_number":130,"context_line":"        self._app.apply_overrides(self._app.admin, overrides)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":133,"context_line":"        LOG.debug(\"Updating overrides.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_0c069b1e","line":130,"in_reply_to":"ff82abbf_e1902046","updated":"2017-11-24 09:51:59.000000000","message":"\u003e So there must be some other problems here we were not taking into\n \u003e account.\n \u003e \n \u003e Redis should be restarted here because we cannot determine whether\n \u003e there are some configuration parameters cannot be applied at\n \u003e runtime (have a look at Redis configuration template, you will find\n \u003e some of them).\n \u003e \n \u003e Then this problem is not only about create a Redis instance with a\n \u003e configuration group containing \u0027requirepass\u0027. Assigning a\n \u003e configuration group containing \u0027requirepass\u0027 and other need_restart\n \u003e parameters will fail too (this kind of configuration group won\u0027t be\n \u003e applied at runtime, so a restart is needed, when restarting, admin\n \u003e client will not be refreshed neither).\n \u003e \n \u003e For the above reason, I will suggest modifying RedisApp.update_overrides()\n \u003e (this approach will be simple). Modifying Redis Manager restart()\n \u003e in the patch set 11 is quite straightforward, but should be changed\n \u003e to avoid unnecessary admin client refreshing.\n\nYes, you are right, some config can\u0027t be applied at runtime, I forgot this, thanks for remind me.\nAbout which method should be override, I think override restart is better.If call _refresh_admin_client in update_overrides, it will affect configuration_attach(apply_overrides after update_overrides, apply_overrides will failed beacuse update_overrides rebuild admin client with new pass  which doesn\u0027t applied at runtime yet), besides, can avoid a extreme case:poll status task just happen after rebuild admin client and not restart db yet.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"}],"trove/guestagent/datastore/experimental/redis/service.py":[{"author":{"_account_id":26895,"name":"Luke Browning","email":"lukebrowning@us.ibm.com","username":"lukebrowning"},"change_message_id":"0b550f59c7f2a1f2ada35fb4bce60b7cd5abca8a","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":147,"context_line":"        if overrides:"},{"line_number":148,"context_line":"            self.configuration_manager.apply_user_override(overrides)"},{"line_number":149,"context_line":"        self.admin \u003d self._build_admin_client()"},{"line_number":150,"context_line":"        self.status \u003d RedisAppStatus(self.admin)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def apply_overrides(self, client, overrides):"},{"line_number":153,"context_line":"        \"\"\"Use the \u0027CONFIG SET\u0027 command to apply configuration at runtime."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_36299489","line":150,"range":{"start_line":149,"start_character":8,"end_line":150,"end_character":48},"updated":"2017-11-22 23:23:24.000000000","message":"This should be under the if statement.","commit_id":"fe66953f6a6ce86d18e5fc9cd69958e60eaa80ac"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"fe4f18e1f98845f588ed043c333f10e69c136323","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":147,"context_line":"        if overrides:"},{"line_number":148,"context_line":"            self.configuration_manager.apply_user_override(overrides)"},{"line_number":149,"context_line":"        self.admin \u003d self._build_admin_client()"},{"line_number":150,"context_line":"        self.status \u003d RedisAppStatus(self.admin)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def apply_overrides(self, client, overrides):"},{"line_number":153,"context_line":"        \"\"\"Use the \u0027CONFIG SET\u0027 command to apply configuration at runtime."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_7f1daf64","line":150,"range":{"start_line":149,"start_character":8,"end_line":150,"end_character":48},"in_reply_to":"ff82abbf_36299489","updated":"2017-11-23 02:30:12.000000000","message":"Nice catch, thanks. Besides, seems has other problem. If rebuild admin client here,  it may affect replace instance\u0027s configuration:apply_overrides will failed.New password hasn\u0027t worked yet but only modifed in config file. I need to think about how to change it","commit_id":"fe66953f6a6ce86d18e5fc9cd69958e60eaa80ac"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"eff9911f46f33f349e90c46633b0938201690499","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            disable_on_boot\u003ddo_not_start_on_reboot, update_db\u003dupdate_db)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def restart(self):"},{"line_number":143,"context_line":"        try:"},{"line_number":144,"context_line":"            self.status.begin_restart()"},{"line_number":145,"context_line":"            self.status.stop_db_service(system.SERVICE_CANDIDATES,"},{"line_number":146,"context_line":"                                        self.state_change_wait_time,"},{"line_number":147,"context_line":"                                        disable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":148,"context_line":"            self.admin \u003d self._build_admin_client()"},{"line_number":149,"context_line":"            self.status \u003d RedisAppStatus(self.admin)"},{"line_number":150,"context_line":"            self.status.start_db_service(system.SERVICE_CANDIDATES,"},{"line_number":151,"context_line":"                                         self.state_change_wait_time,"},{"line_number":152,"context_line":"                                         enable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":153,"context_line":"        except Exception as e:"},{"line_number":154,"context_line":"            LOG.exception(e)"},{"line_number":155,"context_line":"            raise RuntimeError(_(\"Database restart failed.\"))"},{"line_number":156,"context_line":"        finally:"},{"line_number":157,"context_line":"            self.status.end_restart()"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":160,"context_line":"        if overrides:"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_5d36b0e8","line":157,"range":{"start_line":143,"start_character":0,"end_line":157,"end_character":37},"updated":"2017-11-23 06:59:41.000000000","message":"Only create instance with configuration_id, guestagent will restart db after update_overrides.So rebuild admin client during restart to avoid affect configuration_attach(update_overrides and then call\n apply_overrides).Besides, rebuild here rather than rebuild after update_overrides can avoid a extreme case:poll status task just happen after rebuild admin client and not restart db yet.","commit_id":"7452b34f08ac06c94cba7544ad5b03ef137ee8f2"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"00facaee5a886ab9c3eb8bdc79462a479f2f1dc2","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            self.status.stop_db_service(system.SERVICE_CANDIDATES,"},{"line_number":146,"context_line":"                                        self.state_change_wait_time,"},{"line_number":147,"context_line":"                                        disable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":148,"context_line":"            self.admin \u003d self._build_admin_client()"},{"line_number":149,"context_line":"            self.status \u003d RedisAppStatus(self.admin)"},{"line_number":150,"context_line":"            self.status.start_db_service(system.SERVICE_CANDIDATES,"},{"line_number":151,"context_line":"                                         self.state_change_wait_time,"},{"line_number":152,"context_line":"                                         enable_on_boot\u003dFalse, update_db\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff82abbf_912adf8d","line":149,"range":{"start_line":148,"start_character":0,"end_line":149,"end_character":52},"updated":"2017-11-23 10:28:40.000000000","message":"We have RedisAppStatus.set_client(), so there is no need to reinitialize RedisAppStatus ? \n\nThis same precedure has been called by RedisApp.apply_overrides(), so we can create a new wrapper function like _rebuild_admin_client() to reduce code duplication.\n\nWe also shouldn avoid unnecessary admin client rebuilding. Admin client rebuilding is only needed when the \u0027requirepass\u0027 parameter changed by now. So RedisApp.apply_overrides() should also be fixed for this (I\u0027ll bring up this to https://review.openstack.org/#/c/514955/ too, though it\u0027s already merged). \n\nFor the RedisApp.restart() scenario, maybe we can create a new method for RedisApp like \u0027restart_and_rebuild_admin_client\u0027 to do all the work in this patch. Then when a instance created with \u0027requirepass\u0027 included in config_contents, this new method will be called, otherwize the original \u0027restart\u0027 will be called just like before.","commit_id":"97dd3b3f559d5842215ca164824d0a428558f605"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"e3b8157b14948c56c56d5ea5b55c126f7b9e4f36","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            self.status.stop_db_service(system.SERVICE_CANDIDATES,"},{"line_number":146,"context_line":"                                        self.state_change_wait_time,"},{"line_number":147,"context_line":"                                        disable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":148,"context_line":"            self.admin \u003d self._build_admin_client()"},{"line_number":149,"context_line":"            self.status \u003d RedisAppStatus(self.admin)"},{"line_number":150,"context_line":"            self.status.start_db_service(system.SERVICE_CANDIDATES,"},{"line_number":151,"context_line":"                                         self.state_change_wait_time,"},{"line_number":152,"context_line":"                                         enable_on_boot\u003dFalse, update_db\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff82abbf_ec5cb07f","line":149,"range":{"start_line":148,"start_character":0,"end_line":149,"end_character":52},"in_reply_to":"ff82abbf_912adf8d","updated":"2017-11-23 10:55:15.000000000","message":"\u003e We have RedisAppStatus.set_client(), so there is no need to\n \u003e reinitialize RedisAppStatus ?\n \u003e \n \u003e This same precedure has been called by RedisApp.apply_overrides(),\n \u003e so we can create a new wrapper function like _rebuild_admin_client()\n \u003e to reduce code duplication.\n \u003e \n \u003e We also shouldn avoid unnecessary admin client rebuilding. Admin\n \u003e client rebuilding is only needed when the \u0027requirepass\u0027 parameter\n \u003e changed by now. So RedisApp.apply_overrides() should also be fixed\n \u003e for this (I\u0027ll bring up this to https://review.openstack.org/#/c/514955/\n \u003e too, though it\u0027s already merged).\n \u003e \n \u003e For the RedisApp.restart() scenario, maybe we can create a new\n \u003e method for RedisApp like \u0027restart_and_rebuild_admin_client\u0027 to do\n \u003e all the work in this patch. Then when a instance created with\n \u003e \u0027requirepass\u0027 included in config_contents, this new method will be\n \u003e called, otherwize the original \u0027restart\u0027 will be called just like\n \u003e before.\n\nThank you very much for the good advice. \n1. use set_client to replace reinitialize RedisAppStatus;Extract the common code to _rebuild_admin_client.\n   good catch, I\u0027ll fix it \n2. I\u0027ve thought about determine whether rebuild_admin_client is needed  by requirepass was passed in overrides or not before.But update_overrides and restart method wasn\u0027t called in redis/manager.py but in parent class(guestgent/datastore/manager.py).If add some judge logic, I think maybe affect other datastores and seems not elegant enough and readable enough.","commit_id":"97dd3b3f559d5842215ca164824d0a428558f605"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"6afc9e6da33340b1b1ac2932d1d79200db88b1da","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            self.status.stop_db_service(system.SERVICE_CANDIDATES,"},{"line_number":146,"context_line":"                                        self.state_change_wait_time,"},{"line_number":147,"context_line":"                                        disable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":148,"context_line":"            self.admin \u003d self._build_admin_client()"},{"line_number":149,"context_line":"            self.status \u003d RedisAppStatus(self.admin)"},{"line_number":150,"context_line":"            self.status.start_db_service(system.SERVICE_CANDIDATES,"},{"line_number":151,"context_line":"                                         self.state_change_wait_time,"},{"line_number":152,"context_line":"                                         enable_on_boot\u003dFalse, update_db\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ff82abbf_22961ff2","line":149,"range":{"start_line":148,"start_character":0,"end_line":149,"end_character":52},"in_reply_to":"ff82abbf_ec5cb07f","updated":"2017-11-23 12:43:04.000000000","message":"Sorry, I also misunderstood this problem. I was thinking this problem is about _do_prepare and \u0027requirepass\u0027 was set by saving config_contents.\n\nAs \u0027requirepass\u0027 is set by overrides, the solution should be quite simple, we can override apply_overrides_on_prepare in Redis Manager (adding apply_overrides after update_overrides) or modify RedisApp.update_overrides() (for \u0027requirepass\u0027 in overrides, should call apply_overrides immediately for all overrides or \u0027requirepass\u0027 only).\n\nThe other opinion of the \u0027_rebuild_admin_client\u0027 wapper function keeps the same.","commit_id":"97dd3b3f559d5842215ca164824d0a428558f605"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"3dc354cc866de327fc44a4b3d27ab930a0e3f5d7","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            disable_on_boot\u003ddo_not_start_on_reboot, update_db\u003dupdate_db)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def restart(self):"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            self.status.begin_restart()"},{"line_number":150,"context_line":"            self.status.stop_db_service(system.SERVICE_CANDIDATES,"},{"line_number":151,"context_line":"                                        self.state_change_wait_time,"},{"line_number":152,"context_line":"                                        disable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":153,"context_line":"            self._refresh_admin_client()"},{"line_number":154,"context_line":"            self.status.start_db_service(system.SERVICE_CANDIDATES,"},{"line_number":155,"context_line":"                                         self.state_change_wait_time,"},{"line_number":156,"context_line":"                                         enable_on_boot\u003dFalse, update_db\u003dFalse)"},{"line_number":157,"context_line":"        except Exception as e:"},{"line_number":158,"context_line":"            LOG.exception(e)"},{"line_number":159,"context_line":"            raise RuntimeError(_(\"Database restart failed.\"))"},{"line_number":160,"context_line":"        finally:"},{"line_number":161,"context_line":"            self.status.end_restart()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"    def update_overrides(self, context, overrides, remove\u003dFalse):"},{"line_number":164,"context_line":"        if overrides:"}],"source_content_type":"text/x-python","patch_set":11,"id":"ff82abbf_647c34b5","line":161,"range":{"start_line":148,"start_character":0,"end_line":161,"end_character":37},"updated":"2017-11-23 13:24:28.000000000","message":"See previous comments. \n\nAs we do not need to _refresh_admin_client every time when restart Redis, so I think it\u0027s better to do this after update_overrides (there will be an extra call of apply_overrides).","commit_id":"dd309c8d684b97d776e27aa7cb220fb9df42dd26"}],"trove/tests/unittests/guestagent/test_redis_manager.py":[{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"1b1c632cf99fb9cdb29ea0bdc49b30abd67e5d88","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        RedisManager().create_backup(self.context, \u0027backup_id_123\u0027)"},{"line_number":186,"context_line":"        backup.backup.assert_any_call(self.context, \u0027backup_id_123\u0027)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @patch.object(redis_service.RedisApp, \u0027apply_overrides\u0027)"},{"line_number":189,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":190,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":191,"context_line":"    def test_apply_overrides_on_prepare(self, remove_config_mock,"},{"line_number":192,"context_line":"                                        update_config_mock,"},{"line_number":193,"context_line":"                                        apply_config_mock):"},{"line_number":194,"context_line":"        self.manager.apply_overrides_on_prepare(self.context, \u0027overrides\u0027)"},{"line_number":195,"context_line":"        remove_config_mock.assert_not_called()"},{"line_number":196,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":197,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":200,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_9be747ad","line":197,"range":{"start_line":188,"start_character":0,"end_line":197,"end_character":79},"updated":"2017-11-24 08:31:15.000000000","message":"To test what this patch has changed, I think testing when Redis Manager apply_overrides_on_prepare runs, update_overrides and apply_overrides (both are methods of Redis Manager) are called once should be sufficient.","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"bbfa54aec0e0363231f80d05fdfe8516c703c986","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        RedisManager().create_backup(self.context, \u0027backup_id_123\u0027)"},{"line_number":186,"context_line":"        backup.backup.assert_any_call(self.context, \u0027backup_id_123\u0027)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @patch.object(redis_service.RedisApp, \u0027apply_overrides\u0027)"},{"line_number":189,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":190,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":191,"context_line":"    def test_apply_overrides_on_prepare(self, remove_config_mock,"},{"line_number":192,"context_line":"                                        update_config_mock,"},{"line_number":193,"context_line":"                                        apply_config_mock):"},{"line_number":194,"context_line":"        self.manager.apply_overrides_on_prepare(self.context, \u0027overrides\u0027)"},{"line_number":195,"context_line":"        remove_config_mock.assert_not_called()"},{"line_number":196,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":197,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":200,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_464322b0","line":197,"range":{"start_line":188,"start_character":0,"end_line":197,"end_character":79},"in_reply_to":"ff82abbf_9be747ad","updated":"2017-11-24 09:02:25.000000000","message":"\u003e To test what this patch has changed, I think testing when Redis\n \u003e Manager apply_overrides_on_prepare runs, update_overrides and\n \u003e apply_overrides (both are methods of Redis Manager) are called once\n \u003e should be sufficient.\n\nThanks, I was careless, will fix in next ps","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"1b1c632cf99fb9cdb29ea0bdc49b30abd67e5d88","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":197,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":200,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":201,"context_line":"    def test_update_overrides(self, remove_config_mock, update_config_mock):"},{"line_number":202,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":203,"context_line":"        remove_config_mock.assert_not_called()"},{"line_number":204,"context_line":"        update_config_mock.assert_any_call(self.context, \u0027overrides\u0027, False)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":207,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":208,"context_line":"    def test_update_overrides_with_remove(self, remove_config_mock,"},{"line_number":209,"context_line":"                                          update_config_mock):"},{"line_number":210,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027, True)"},{"line_number":211,"context_line":"        remove_config_mock.assert_any_call()"},{"line_number":212,"context_line":"        update_config_mock.assert_not_called()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @patch.object(redis_service.RedisApp, \u0027apply_overrides\u0027)"},{"line_number":215,"context_line":"    def test_apply_overrides(self, apply_config_mock):"},{"line_number":216,"context_line":"        self.manager.apply_overrides(self.context, \u0027overrides\u0027)"},{"line_number":217,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def test_backup_required_for_replication(self):"},{"line_number":220,"context_line":"        mock_replication \u003d MagicMock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_5b40dfa5","line":217,"range":{"start_line":199,"start_character":0,"end_line":217,"end_character":79},"updated":"2017-11-24 08:31:15.000000000","message":"Seems like these cases are unnecessary ?","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":13157,"name":"Zhao Chao","email":"zhaochao1984@gmail.com","username":"zhaochao"},"change_message_id":"e81faa45ecb3828bc5cf7f135d99a22cf1bed62f","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":197,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":200,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":201,"context_line":"    def test_update_overrides(self, remove_config_mock, update_config_mock):"},{"line_number":202,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":203,"context_line":"        remove_config_mock.assert_not_called()"},{"line_number":204,"context_line":"        update_config_mock.assert_any_call(self.context, \u0027overrides\u0027, False)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":207,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":208,"context_line":"    def test_update_overrides_with_remove(self, remove_config_mock,"},{"line_number":209,"context_line":"                                          update_config_mock):"},{"line_number":210,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027, True)"},{"line_number":211,"context_line":"        remove_config_mock.assert_any_call()"},{"line_number":212,"context_line":"        update_config_mock.assert_not_called()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @patch.object(redis_service.RedisApp, \u0027apply_overrides\u0027)"},{"line_number":215,"context_line":"    def test_apply_overrides(self, apply_config_mock):"},{"line_number":216,"context_line":"        self.manager.apply_overrides(self.context, \u0027overrides\u0027)"},{"line_number":217,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def test_backup_required_for_replication(self):"},{"line_number":220,"context_line":"        mock_replication \u003d MagicMock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_21f12896","line":217,"range":{"start_line":199,"start_character":0,"end_line":217,"end_character":79},"in_reply_to":"ff82abbf_26461ec2","updated":"2017-11-24 09:38:29.000000000","message":"Then another patch for these cases may be better, :)","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"},{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"bbfa54aec0e0363231f80d05fdfe8516c703c986","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":197,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":200,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":201,"context_line":"    def test_update_overrides(self, remove_config_mock, update_config_mock):"},{"line_number":202,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027)"},{"line_number":203,"context_line":"        remove_config_mock.assert_not_called()"},{"line_number":204,"context_line":"        update_config_mock.assert_any_call(self.context, \u0027overrides\u0027, False)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @patch.object(redis_service.RedisApp, \u0027update_overrides\u0027)"},{"line_number":207,"context_line":"    @patch.object(redis_service.RedisApp, \u0027remove_overrides\u0027)"},{"line_number":208,"context_line":"    def test_update_overrides_with_remove(self, remove_config_mock,"},{"line_number":209,"context_line":"                                          update_config_mock):"},{"line_number":210,"context_line":"        self.manager.update_overrides(self.context, \u0027overrides\u0027, True)"},{"line_number":211,"context_line":"        remove_config_mock.assert_any_call()"},{"line_number":212,"context_line":"        update_config_mock.assert_not_called()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    @patch.object(redis_service.RedisApp, \u0027apply_overrides\u0027)"},{"line_number":215,"context_line":"    def test_apply_overrides(self, apply_config_mock):"},{"line_number":216,"context_line":"        self.manager.apply_overrides(self.context, \u0027overrides\u0027)"},{"line_number":217,"context_line":"        apply_config_mock.assert_any_call(self.manager._app.admin, \u0027overrides\u0027)"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def test_backup_required_for_replication(self):"},{"line_number":220,"context_line":"        mock_replication \u003d MagicMock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"ff82abbf_26461ec2","line":217,"range":{"start_line":199,"start_character":0,"end_line":217,"end_character":79},"in_reply_to":"ff82abbf_5b40dfa5","updated":"2017-11-24 09:02:25.000000000","message":"\u003e Seems like these cases are unnecessary ?\n\nWhen I add test case for apply_overrides_on_prepare, I found these cases haven\u0027t been added yet so I added them together","commit_id":"63d6b49a3d5abdab316882e5400391930937989f"}]}