)]}'
{"trove/extensions/redis/models.py":[{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"69ea452a29d9d41ea7e7030b088e1c3fc3a9c1fb","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from trove.extensions.common.models import Root"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class RedisRoot(Root):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_0e967794","line":25,"updated":"2017-11-27 07:40:42.000000000","message":"These two are both not used?","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"301992d9b4d06ec8ad61598bc80eacad1a26b90a","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from trove.extensions.common.models import Root"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"CONF \u003d cfg.CONF"},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class RedisRoot(Root):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_0e0c373f","line":25,"in_reply_to":"ff82abbf_0e967794","updated":"2017-11-27 08:40:34.000000000","message":"Yeah, good catch. Thanks.","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"}],"trove/extensions/redis/service.py":[{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"69ea452a29d9d41ea7e7030b088e1c3fc3a9c1fb","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        is_slave \u003d self._is_slave(tenant_id, instance_id)"},{"line_number":59,"context_line":"        if is_slave:"},{"line_number":60,"context_line":"            raise exception.SlaveOperationNotSupported("},{"line_number":61,"context_line":"                operation\u003d\u0027disable_root\u0027)"},{"line_number":62,"context_line":"        slave_instances \u003d self._get_slaves(tenant_id, instance_id)"},{"line_number":63,"context_line":"        return self._instance_root_delete(req, instance_id, slave_instances)"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_8e3da790","line":61,"updated":"2017-11-27 07:40:42.000000000","message":"The is_cluster check and is_slave check here are quite similar to the ones in root_create. Can we just move these into a single validation helper? Don\u0027t have quite a strong opinion on this though.","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"301992d9b4d06ec8ad61598bc80eacad1a26b90a","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        is_slave \u003d self._is_slave(tenant_id, instance_id)"},{"line_number":59,"context_line":"        if is_slave:"},{"line_number":60,"context_line":"            raise exception.SlaveOperationNotSupported("},{"line_number":61,"context_line":"                operation\u003d\u0027disable_root\u0027)"},{"line_number":62,"context_line":"        slave_instances \u003d self._get_slaves(tenant_id, instance_id)"},{"line_number":63,"context_line":"        return self._instance_root_delete(req, instance_id, slave_instances)"},{"line_number":64,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_716126a9","line":61,"in_reply_to":"ff82abbf_8e3da790","updated":"2017-11-27 08:40:34.000000000","message":"Thx, moving these two validation helper into one looks much better.","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"},{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"69ea452a29d9d41ea7e7030b088e1c3fc3a9c1fb","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            RedisRoot.delete(context, instance_id)"},{"line_number":115,"context_line":"        except exception.TroveError:"},{"line_number":116,"context_line":"            if not self._rollback_once(req, instance_id,"},{"line_number":117,"context_line":"                                       original_auth_password):"},{"line_number":118,"context_line":"                raise exception.TroveError("},{"line_number":119,"context_line":"                    _LE(\"Failed to do root-disable for instance \""},{"line_number":120,"context_line":"                        \"\u0027%(instance_id)s\u0027.\") % {\u0027instance_id\u0027: instance_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_8ebe07b2","line":117,"updated":"2017-11-27 07:40:42.000000000","message":"There can be corner case here. I am not sure whether it matters.\n\nIf _get_original_auth_password happens to fail, it would return None. If the instance was root-enabled, then this rollback call (if called) is not a rollback but a root-disable anyway.","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"301992d9b4d06ec8ad61598bc80eacad1a26b90a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            RedisRoot.delete(context, instance_id)"},{"line_number":115,"context_line":"        except exception.TroveError:"},{"line_number":116,"context_line":"            if not self._rollback_once(req, instance_id,"},{"line_number":117,"context_line":"                                       original_auth_password):"},{"line_number":118,"context_line":"                raise exception.TroveError("},{"line_number":119,"context_line":"                    _LE(\"Failed to do root-disable for instance \""},{"line_number":120,"context_line":"                        \"\u0027%(instance_id)s\u0027.\") % {\u0027instance_id\u0027: instance_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_f1d0d621","line":117,"in_reply_to":"ff82abbf_8ebe07b2","updated":"2017-11-27 08:40:34.000000000","message":"If there is no rollback, even just once, it may cause that the status of instance becomes SHUTDOWN or unreachable. I admit that these rollback operations only happen in some corner cases, but better than nothing, right?","commit_id":"ff5771c689909e36b9c069499bfe83760f4c5639"},{"author":{"_account_id":22694,"name":"jian.song","email":"jiansong19941206@gmail.com","username":"jiansong"},"change_message_id":"2af229268940bfc24171d66e790bbc94e26e6b9e","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    def _instance_root_delete(self, req, instance_id, slave_instances\u003dNone):"},{"line_number":91,"context_line":"        LOG.debug(_LI(\"Disabling authentication for instance \u0027%s\u0027.\")"},{"line_number":92,"context_line":"                  % instance_id)"},{"line_number":93,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\") % req)"},{"line_number":94,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        original_auth_password \u003d self._get_original_auth_password("}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_a6dccb46","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":46},"updated":"2017-12-04 09:26:34.000000000","message":"Here\u0027s a minor problem that we\u0027ve ever done replacing \"%\" with \",\". The reason at that time was that it looked more like Log\u0027s format. But then for some reason did not start pep8 on this check.\n\nReference: https://review.openstack.org/#/c/433846/","commit_id":"87b95bea87b3e9dca1dbc3c692f277f098c31885"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"7d462160d6afa2c8c2a790205ac9548f6ddcceae","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    def _instance_root_delete(self, req, instance_id, slave_instances\u003dNone):"},{"line_number":91,"context_line":"        LOG.debug(_LI(\"Disabling authentication for instance \u0027%s\u0027.\")"},{"line_number":92,"context_line":"                  % instance_id)"},{"line_number":93,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\") % req)"},{"line_number":94,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        original_auth_password \u003d self._get_original_auth_password("}],"source_content_type":"text/x-python","patch_set":6,"id":"df87a7cf_69ae94e8","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":46},"in_reply_to":"df87a7cf_a6dccb46","updated":"2017-12-04 09:55:25.000000000","message":"Done","commit_id":"87b95bea87b3e9dca1dbc3c692f277f098c31885"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"89d4df4cf1c0a8a379fbce3d2828000ec40ec4f2","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        args \u003d {\u0027id\u0027: instance_id, \u0027tenant_id\u0027: tenant_id}"},{"line_number":151,"context_line":"        instance_info \u003d DBInstance.find_by(**args)"},{"line_number":152,"context_line":"        slave_of_id \u003d instance_info.slave_of_id"},{"line_number":153,"context_line":"        return bool(slave_of_id)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @staticmethod"},{"line_number":156,"context_line":"    def _get_slaves(tenant_id, instance_or_cluster_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"df87a7cf_fc548d23","line":153,"range":{"start_line":153,"start_character":31,"end_line":153,"end_character":32},"updated":"2017-12-05 03:40:58.000000000","message":"I think just return instance_info.slave_of_id is OK","commit_id":"32c26aca74333fe23b081b02354544346a4acc89"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"41bf0285f117316a527b6dbaae7b232d8d3323ce","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        args \u003d {\u0027id\u0027: instance_id, \u0027tenant_id\u0027: tenant_id}"},{"line_number":151,"context_line":"        instance_info \u003d DBInstance.find_by(**args)"},{"line_number":152,"context_line":"        slave_of_id \u003d instance_info.slave_of_id"},{"line_number":153,"context_line":"        return bool(slave_of_id)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    @staticmethod"},{"line_number":156,"context_line":"    def _get_slaves(tenant_id, instance_or_cluster_id):"}],"source_content_type":"text/x-python","patch_set":8,"id":"df87a7cf_addd6d15","line":153,"range":{"start_line":153,"start_character":31,"end_line":153,"end_character":32},"in_reply_to":"df87a7cf_fc548d23","updated":"2017-12-05 06:08:37.000000000","message":"Done","commit_id":"32c26aca74333fe23b081b02354544346a4acc89"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"89d4df4cf1c0a8a379fbce3d2828000ec40ec4f2","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    def _get_slaves(tenant_id, instance_or_cluster_id):"},{"line_number":157,"context_line":"        LOG.debug(_LI(\"Getting non-deleted slaves of instance \u0027%s\u0027, \""},{"line_number":158,"context_line":"                      \"if any.\"), instance_or_cluster_id)"},{"line_number":159,"context_line":"        args \u003d {\u0027slave_of_id\u0027: instance_or_cluster_id, \u0027tenant_id\u0027: tenant_id}"},{"line_number":160,"context_line":"        db_infos \u003d DBInstance.find_all(**args)"},{"line_number":161,"context_line":"        slaves \u003d []"},{"line_number":162,"context_line":"        for db_info in db_infos:"}],"source_content_type":"text/x-python","patch_set":8,"id":"df87a7cf_dc07491c","line":159,"range":{"start_line":159,"start_character":15,"end_line":159,"end_character":17},"updated":"2017-12-05 03:40:58.000000000","message":"Since you want get all non-deleted instance, why not add deleted: False in args","commit_id":"32c26aca74333fe23b081b02354544346a4acc89"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"41bf0285f117316a527b6dbaae7b232d8d3323ce","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    def _get_slaves(tenant_id, instance_or_cluster_id):"},{"line_number":157,"context_line":"        LOG.debug(_LI(\"Getting non-deleted slaves of instance \u0027%s\u0027, \""},{"line_number":158,"context_line":"                      \"if any.\"), instance_or_cluster_id)"},{"line_number":159,"context_line":"        args \u003d {\u0027slave_of_id\u0027: instance_or_cluster_id, \u0027tenant_id\u0027: tenant_id}"},{"line_number":160,"context_line":"        db_infos \u003d DBInstance.find_all(**args)"},{"line_number":161,"context_line":"        slaves \u003d []"},{"line_number":162,"context_line":"        for db_info in db_infos:"}],"source_content_type":"text/x-python","patch_set":8,"id":"df87a7cf_cde0f1cf","line":159,"range":{"start_line":159,"start_character":15,"end_line":159,"end_character":17},"in_reply_to":"df87a7cf_dc07491c","updated":"2017-12-05 06:08:37.000000000","message":"Done","commit_id":"32c26aca74333fe23b081b02354544346a4acc89"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"53287b182729ae1bbeb2efef3f15617fd3b7151a","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        try:"},{"line_number":65,"context_line":"            root \u003d RedisRoot.create(context, instance_id,"},{"line_number":66,"context_line":"                                    user_name, password)"},{"line_number":67,"context_line":"            if password is None:"},{"line_number":68,"context_line":"                password \u003d root.password"},{"line_number":69,"context_line":"        except exception.TroveError:"},{"line_number":70,"context_line":"            if not self._rollback_once(req, instance_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_28d14bfc","line":67,"range":{"start_line":67,"start_character":27,"end_line":67,"end_character":31},"updated":"2017-12-05 07:17:44.000000000","message":"if not password","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"9added10064b41708e6958ec5d7dc3e428e670a1","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        try:"},{"line_number":65,"context_line":"            root \u003d RedisRoot.create(context, instance_id,"},{"line_number":66,"context_line":"                                    user_name, password)"},{"line_number":67,"context_line":"            if password is None:"},{"line_number":68,"context_line":"                password \u003d root.password"},{"line_number":69,"context_line":"        except exception.TroveError:"},{"line_number":70,"context_line":"            if not self._rollback_once(req, instance_id,"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_ae76c3a3","line":67,"range":{"start_line":67,"start_character":27,"end_line":67,"end_character":31},"in_reply_to":"df87a7cf_28d14bfc","updated":"2017-12-05 08:34:41.000000000","message":"Done","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"53287b182729ae1bbeb2efef3f15617fd3b7151a","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            if password is None:"},{"line_number":68,"context_line":"                password \u003d root.password"},{"line_number":69,"context_line":"        except exception.TroveError:"},{"line_number":70,"context_line":"            if not self._rollback_once(req, instance_id,"},{"line_number":71,"context_line":"                                       original_auth_password):"},{"line_number":72,"context_line":"                raise exception.TroveError("},{"line_number":73,"context_line":"                    _LE(\"Failed to do root-enable for instance \""}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_68e753e6","line":70,"range":{"start_line":70,"start_character":38,"end_line":70,"end_character":39},"updated":"2017-12-05 07:17:44.000000000","message":"3. My understanging is no matter self._rollback_once return true or false, the root_enable actually has failed. How about just call self._rooback_once(xx,xx,xx) and raise.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"9added10064b41708e6958ec5d7dc3e428e670a1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            if password is None:"},{"line_number":68,"context_line":"                password \u003d root.password"},{"line_number":69,"context_line":"        except exception.TroveError:"},{"line_number":70,"context_line":"            if not self._rollback_once(req, instance_id,"},{"line_number":71,"context_line":"                                       original_auth_password):"},{"line_number":72,"context_line":"                raise exception.TroveError("},{"line_number":73,"context_line":"                    _LE(\"Failed to do root-enable for instance \""}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_ce9ea7b7","line":70,"range":{"start_line":70,"start_character":38,"end_line":70,"end_character":39},"in_reply_to":"df87a7cf_68e753e6","updated":"2017-12-05 08:34:41.000000000","message":"Done","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"53287b182729ae1bbeb2efef3f15617fd3b7151a","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":133,"context_line":"        user_name \u003d context.user"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            if original_auth_password is None:"},{"line_number":136,"context_line":"                # Instance never did root-enable before."},{"line_number":137,"context_line":"                RedisRoot.delete(context, instance_id)"},{"line_number":138,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_c809bf95","line":135,"range":{"start_line":135,"start_character":45,"end_line":135,"end_character":46},"updated":"2017-12-05 07:17:44.000000000","message":"if not original_auth_password","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"78ef97ee3fedcfee0694c1d55d98d50e61f9072e","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":133,"context_line":"        user_name \u003d context.user"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            if original_auth_password is None:"},{"line_number":136,"context_line":"                # Instance never did root-enable before."},{"line_number":137,"context_line":"                RedisRoot.delete(context, instance_id)"},{"line_number":138,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_538484b1","line":135,"range":{"start_line":135,"start_character":45,"end_line":135,"end_character":46},"in_reply_to":"df87a7cf_68351346","updated":"2017-12-05 08:26:07.000000000","message":"Here it actually should be \"if not original_auth_password\", because if instance never did root-enable before, you could not get original password in config file.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"4dd1496a1a376167db9075ea3761a4ab916180f7","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":133,"context_line":"        user_name \u003d context.user"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            if original_auth_password is None:"},{"line_number":136,"context_line":"                # Instance never did root-enable before."},{"line_number":137,"context_line":"                RedisRoot.delete(context, instance_id)"},{"line_number":138,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_68351346","line":135,"range":{"start_line":135,"start_character":45,"end_line":135,"end_character":46},"in_reply_to":"df87a7cf_c809bf95","updated":"2017-12-05 07:19:38.000000000","message":"\u003e if not original_auth_password\n\nif original_auth_password:","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"9added10064b41708e6958ec5d7dc3e428e670a1","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":133,"context_line":"        user_name \u003d context.user"},{"line_number":134,"context_line":"        try:"},{"line_number":135,"context_line":"            if original_auth_password is None:"},{"line_number":136,"context_line":"                # Instance never did root-enable before."},{"line_number":137,"context_line":"                RedisRoot.delete(context, instance_id)"},{"line_number":138,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_8ea89fdb","line":135,"range":{"start_line":135,"start_character":45,"end_line":135,"end_character":46},"in_reply_to":"df87a7cf_c809bf95","updated":"2017-12-05 08:34:41.000000000","message":"Done","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"0a9982bb93296fd2df425606e1a14bf5e1dbe123","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    def _get_original_auth_password(context, instance_id):"},{"line_number":168,"context_line":"        # Check if instance did root-enable before and get original password."},{"line_number":169,"context_line":"        password \u003d None"},{"line_number":170,"context_line":"        if RedisRoot.load(context, instance_id):"},{"line_number":171,"context_line":"            try:"},{"line_number":172,"context_line":"                password \u003d RedisRoot.get_auth_password(context, instance_id)"},{"line_number":173,"context_line":"            except exception.TroveError:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_f3c91878","line":170,"updated":"2017-12-05 08:03:05.000000000","message":"Should also raise error about unable to get original auth password if this load fails (returns False)? Since getting original auth password shouldn\u0027t be consider successful in such a case.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"78ef97ee3fedcfee0694c1d55d98d50e61f9072e","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    def _get_original_auth_password(context, instance_id):"},{"line_number":168,"context_line":"        # Check if instance did root-enable before and get original password."},{"line_number":169,"context_line":"        password \u003d None"},{"line_number":170,"context_line":"        if RedisRoot.load(context, instance_id):"},{"line_number":171,"context_line":"            try:"},{"line_number":172,"context_line":"                password \u003d RedisRoot.get_auth_password(context, instance_id)"},{"line_number":173,"context_line":"            except exception.TroveError:"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_733bc8ea","line":170,"in_reply_to":"df87a7cf_f3c91878","updated":"2017-12-05 08:26:07.000000000","message":"Returning false means there is no RootHistory of this instance, so the instance never executes root-enable before. That leads to original password with None. It\u0027s fair.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"9222da147905fbde43805ed38497cb596a47d939","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _instance_root_create(self, req, instance_id, password,"},{"line_number":53,"context_line":"                              slave_instances\u003dNone):"},{"line_number":54,"context_line":"        LOG.debug(_LI(\"Enabling authentication for instance \u0027%s\u0027.\"),"},{"line_number":55,"context_line":"                  instance_id)"},{"line_number":56,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\"), req)"},{"line_number":57,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_69188b74","line":54,"updated":"2017-12-07 08:47:51.000000000","message":"Why loglevel is debug and translation is INFO? For debug it\u0027s not needed, is it?","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"eb50d7ae22e0a39db5b5478743758f6917e90c8c","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _instance_root_create(self, req, instance_id, password,"},{"line_number":53,"context_line":"                              slave_instances\u003dNone):"},{"line_number":54,"context_line":"        LOG.debug(_LI(\"Enabling authentication for instance \u0027%s\u0027.\"),"},{"line_number":55,"context_line":"                  instance_id)"},{"line_number":56,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\"), req)"},{"line_number":57,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_90db9d7a","line":54,"in_reply_to":"df87a7cf_5dc44e5f","updated":"2017-12-07 10:17:39.000000000","message":"Done","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"59ee71018fe99fe6c67d2774e8a5225d252c1932","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _instance_root_create(self, req, instance_id, password,"},{"line_number":53,"context_line":"                              slave_instances\u003dNone):"},{"line_number":54,"context_line":"        LOG.debug(_LI(\"Enabling authentication for instance \u0027%s\u0027.\"),"},{"line_number":55,"context_line":"                  instance_id)"},{"line_number":56,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\"), req)"},{"line_number":57,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_5dc44e5f","line":54,"in_reply_to":"df87a7cf_69188b74","updated":"2017-12-07 10:09:39.000000000","message":"Oh yes, these logs are just for info, not for debug. I\u0027m fixing it right away.","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"9222da147905fbde43805ed38497cb596a47d939","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                              slave_instances\u003dNone):"},{"line_number":54,"context_line":"        LOG.debug(_LI(\"Enabling authentication for instance \u0027%s\u0027.\"),"},{"line_number":55,"context_line":"                  instance_id)"},{"line_number":56,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\"), req)"},{"line_number":57,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":58,"context_line":"        user_name \u003d context.user"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_c9b677fb","line":56,"updated":"2017-12-07 08:47:51.000000000","message":"Same here.","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"eb50d7ae22e0a39db5b5478743758f6917e90c8c","unresolved":false,"context_lines":[{"line_number":53,"context_line":"                              slave_instances\u003dNone):"},{"line_number":54,"context_line":"        LOG.debug(_LI(\"Enabling authentication for instance \u0027%s\u0027.\"),"},{"line_number":55,"context_line":"                  instance_id)"},{"line_number":56,"context_line":"        LOG.debug(_LI(\"req : \u0027%s\u0027\\n\\n\"), req)"},{"line_number":57,"context_line":"        context \u003d req.environ[wsgi.CONTEXT_KEY]"},{"line_number":58,"context_line":"        user_name \u003d context.user"},{"line_number":59,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_50e19529","line":56,"in_reply_to":"df87a7cf_c9b677fb","updated":"2017-12-07 10:17:39.000000000","message":"Done. And fixed other log statements either.","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"}],"trove/guestagent/datastore/experimental/redis/service.py":[{"author":{"_account_id":17812,"name":"chengyang","email":"374519141@qq.com","username":"chengyang"},"change_message_id":"37df42f8ed57f162e224a3d0aa357f02686c96a3","unresolved":false,"context_lines":[{"line_number":419,"context_line":"            raise"},{"line_number":420,"context_line":"        return redis_password.serialize()"},{"line_number":421,"context_line":""},{"line_number":422,"context_line":"    def disable_root(self):"},{"line_number":423,"context_line":"        try:"},{"line_number":424,"context_line":"            self.configuration_manager.remove_system_override("},{"line_number":425,"context_line":"                change_id\u003dSYS_OVERRIDES_AUTH)"},{"line_number":426,"context_line":"            self.apply_overrides(self.admin,"},{"line_number":427,"context_line":"                                 {\u0027requirepass\u0027: \u0027\u0027, \u0027masterauth\u0027: \u0027\u0027})"},{"line_number":428,"context_line":"        except exception.TroveError:"},{"line_number":429,"context_line":"            LOG.exception(_(\u0027Error disabling authentication for instance.\u0027))"},{"line_number":430,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_21aa8870","line":427,"range":{"start_line":422,"start_character":0,"end_line":427,"end_character":71},"updated":"2017-11-24 09:33:08.000000000","message":"Hi, I\u0027m confused about the purpose of method disable_root,  enable login with NOAUTH? seems doesn\u0027t match the literal meaning of disable_root","commit_id":"88c715f20fdb916fdd4cfa01b904908f79d10ecc"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"53287b182729ae1bbeb2efef3f15617fd3b7151a","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                change_id\u003dSYS_OVERRIDES_AUTH)"},{"line_number":415,"context_line":"            self.apply_overrides("},{"line_number":416,"context_line":"                self.admin, {\u0027requirepass\u0027: password, \u0027masterauth\u0027: password})"},{"line_number":417,"context_line":"        except exception.TroveError:"},{"line_number":418,"context_line":"            LOG.exception(_(\u0027Error enabling authentication for instance.\u0027))"},{"line_number":419,"context_line":"            raise"},{"line_number":420,"context_line":"        return redis_password.serialize()"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_c8e0dfcf","line":417,"range":{"start_line":417,"start_character":35,"end_line":417,"end_character":36},"updated":"2017-12-05 07:17:44.000000000","message":"I have some doubts about the use of try...except:\n1. Ican\u0027t find where could raise a TroveError in that try block.\n2. Since you catch TroveError in extensions/redis/service.py. Use raise exception.TroveError(some_message) instead raise seems better.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"53f9bb9bf4803015af203f358bebb71c834dfd07","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                change_id\u003dSYS_OVERRIDES_AUTH)"},{"line_number":415,"context_line":"            self.apply_overrides("},{"line_number":416,"context_line":"                self.admin, {\u0027requirepass\u0027: password, \u0027masterauth\u0027: password})"},{"line_number":417,"context_line":"        except exception.TroveError:"},{"line_number":418,"context_line":"            LOG.exception(_(\u0027Error enabling authentication for instance.\u0027))"},{"line_number":419,"context_line":"            raise"},{"line_number":420,"context_line":"        return redis_password.serialize()"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_d392d44b","line":417,"range":{"start_line":417,"start_character":35,"end_line":417,"end_character":36},"in_reply_to":"df87a7cf_c8e0dfcf","updated":"2017-12-05 08:13:10.000000000","message":"self.apply_overrides -\u003e client.config_set() -\u003e self.execute https://github.com/openstack/trove/blob/ccb6752f6973a7d1c8bb08675fde421710942998/trove/guestagent/datastore/experimental/redis/service.py#L497, It may happen to be a TroveError. So I am going to keep this try-except block. \nAs for the raise statement, the code indeed raise with messages  inside extension/redis/service.py, so I guess it won\u0027t be necessary to do that with specified messages here?\nThx :)","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"1d28ab4f7bc33356c10a7b5ab09b2f0e839b2b76","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                change_id\u003dSYS_OVERRIDES_AUTH)"},{"line_number":415,"context_line":"            self.apply_overrides("},{"line_number":416,"context_line":"                self.admin, {\u0027requirepass\u0027: password, \u0027masterauth\u0027: password})"},{"line_number":417,"context_line":"        except exception.TroveError:"},{"line_number":418,"context_line":"            LOG.exception(_(\u0027Error enabling authentication for instance.\u0027))"},{"line_number":419,"context_line":"            raise"},{"line_number":420,"context_line":"        return redis_password.serialize()"}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_4efff733","line":417,"range":{"start_line":417,"start_character":35,"end_line":417,"end_character":36},"in_reply_to":"df87a7cf_d392d44b","updated":"2017-12-05 09:06:59.000000000","message":"OK, I see. :)\nBut, catch a TroveError and then raise a TroveError, in my opinion, a \u0027LOG.exception\u0027 is not deserve to use a try...except.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":22608,"name":"zhanggang","email":"zhanggang@cmss.chinamobile.com","username":"zhanggang"},"change_message_id":"53287b182729ae1bbeb2efef3f15617fd3b7151a","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                change_id\u003dSYS_OVERRIDES_AUTH)"},{"line_number":426,"context_line":"            self.apply_overrides(self.admin,"},{"line_number":427,"context_line":"                                 {\u0027requirepass\u0027: \u0027\u0027, \u0027masterauth\u0027: \u0027\u0027})"},{"line_number":428,"context_line":"        except exception.TroveError:"},{"line_number":429,"context_line":"            LOG.exception(_(\u0027Error disabling authentication for instance.\u0027))"},{"line_number":430,"context_line":"            raise"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"df87a7cf_08d6c7f2","line":428,"range":{"start_line":428,"start_character":35,"end_line":428,"end_character":36},"updated":"2017-12-05 07:17:44.000000000","message":"The same.","commit_id":"154bbc1850791e9e4b4fb1b8a9a19d8e740ab3dd"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"9222da147905fbde43805ed38497cb596a47d939","unresolved":false,"context_lines":[{"line_number":38,"context_line":"TIME_OUT \u003d 1200"},{"line_number":39,"context_line":"CONF \u003d cfg.CONF"},{"line_number":40,"context_line":"CLUSTER_CFG \u003d \u0027clustering\u0027"},{"line_number":41,"context_line":"SYS_OVERRIDES_AUTH \u003d \u0027auth_password\u0027"},{"line_number":42,"context_line":"packager \u003d pkg.Package()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_a979b32f","line":41,"updated":"2017-12-07 08:47:51.000000000","message":"Hmm, Maybe we should consider to make some common place for that kind of constants?","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"3f47974cc58a1271762aaaa9d92ac6f6220ef71b","unresolved":false,"context_lines":[{"line_number":38,"context_line":"TIME_OUT \u003d 1200"},{"line_number":39,"context_line":"CONF \u003d cfg.CONF"},{"line_number":40,"context_line":"CLUSTER_CFG \u003d \u0027clustering\u0027"},{"line_number":41,"context_line":"SYS_OVERRIDES_AUTH \u003d \u0027auth_password\u0027"},{"line_number":42,"context_line":"packager \u003d pkg.Package()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_33465bc2","line":41,"in_reply_to":"df87a7cf_7d57d259","updated":"2017-12-08 08:42:54.000000000","message":"Yes, that could be a future work. Just to notice that  :)","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"},{"author":{"_account_id":25113,"name":"Fan Zhang","email":"zh.f@outlook.com","username":"fanzhang"},"change_message_id":"59ee71018fe99fe6c67d2774e8a5225d252c1932","unresolved":false,"context_lines":[{"line_number":38,"context_line":"TIME_OUT \u003d 1200"},{"line_number":39,"context_line":"CONF \u003d cfg.CONF"},{"line_number":40,"context_line":"CLUSTER_CFG \u003d \u0027clustering\u0027"},{"line_number":41,"context_line":"SYS_OVERRIDES_AUTH \u003d \u0027auth_password\u0027"},{"line_number":42,"context_line":"packager \u003d pkg.Package()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"df87a7cf_7d57d259","line":41,"in_reply_to":"df87a7cf_a979b32f","updated":"2017-12-07 10:09:39.000000000","message":"I don\u0027t see many of that constants used in apply_system_overrides. Maybe we can deal with it later?","commit_id":"abfc62d38079164d67f034e77dad87179fef5bcc"}]}