)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1f421c87643659301dee00eccff8a1297d8cc2e0","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     wanghong \u003cw.wanghong@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-10-23 16:09:27 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"cann\u0027t update catalog objects when using kvs driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The update logic of catalog objects in kvs driver is wrong: currently,"},{"line_number":10,"context_line":"it just overwrites the old one rather than update."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9aa7fdbe_cc0b39d4","line":7,"updated":"2014-10-27 15:59:15.000000000","message":"Can\u0027t*","commit_id":"56807a0a7e67fc6850553bfc754cee4129f805a2"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1f421c87643659301dee00eccff8a1297d8cc2e0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"cann\u0027t update catalog objects when using kvs driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The update logic of catalog objects in kvs driver is wrong: currently,"},{"line_number":10,"context_line":"it just overwrites the old one rather than update."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I9fa582a71e1ad9b6fa06e2ffbff1472aeb975a54"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9aa7fdbe_6c024db5","line":9,"updated":"2014-10-27 15:59:15.000000000","message":"wrong. Currently,*","commit_id":"56807a0a7e67fc6850553bfc754cee4129f805a2"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"aca2c53483a933d18b18cee042c17be50dbbf85c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     wanghong \u003cw.wanghong@huawei.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-12-18 10:30:05 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Can\u0027t update catalog objects when using kvs driver"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The update logic of catalog objects in kvs driver is wrong. Currently,"},{"line_number":10,"context_line":"it just overwrites the old one rather than update."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"3a961159_6fad3788","line":7,"updated":"2014-12-18 16:27:10.000000000","message":"Please, describe here what the commit does, not the state before it","commit_id":"64c2aab21dd3d59293333ab847c7fa34978905ea"}],"keystone/catalog/backends/kvs.py":[{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"1f421c87643659301dee00eccff8a1297d8cc2e0","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return self.db.get(\u0027region-%s\u0027 % region_id)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def update_region(self, region_id, region):"},{"line_number":70,"context_line":"        region.setdefault(\u0027parent_region_id\u0027)"},{"line_number":71,"context_line":"        self._check_parent_region(region)"},{"line_number":72,"context_line":"        self.db.set(\u0027region-%s\u0027 % region_id, region)"},{"line_number":73,"context_line":"        return region"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_cf3deb90","side":"PARENT","line":70,"updated":"2014-10-27 15:59:15.000000000","message":"why remove this?","commit_id":"dcf086fa0960c89e4c42ce224a9d2409406c4221"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"ff7a92b8c85ae86809185c4bc7b6a093f5f8372a","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return self.db.get(\u0027region-%s\u0027 % region_id)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def update_region(self, region_id, region):"},{"line_number":70,"context_line":"        region.setdefault(\u0027parent_region_id\u0027)"},{"line_number":71,"context_line":"        self._check_parent_region(region)"},{"line_number":72,"context_line":"        self.db.set(\u0027region-%s\u0027 % region_id, region)"},{"line_number":73,"context_line":"        return region"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_173e1dda","side":"PARENT","line":70,"in_reply_to":"9aa7fdbe_cf3deb90","updated":"2014-10-28 01:15:14.000000000","message":"This will lead to a bug: if I update a region A which has parent_region_id B without parent_region_id property specified, here will update the parent_region_id of A to None...","commit_id":"dcf086fa0960c89e4c42ce224a9d2409406c4221"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"bc8a12b9ea2f40aa4f0a158302b67d555fac9d4a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        return self.db.get(\u0027endpoint-%s\u0027 % endpoint_id)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def update_endpoint(self, endpoint_id, endpoint):"},{"line_number":132,"context_line":"        if endpoint.get(\u0027region_id\u0027) is not None:"},{"line_number":133,"context_line":"            self.get_region(endpoint[\u0027region_id\u0027])"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        old_endpoint \u003d self.get_endpoint(endpoint_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_dbbc9f09","line":132,"updated":"2014-12-15 19:22:52.000000000","message":"I can update the region_id to be None?\n\nUp at line 118, for create_endpoint it checks that the service_id exists, not the region... why check region exists and not service here, or why check service on create and not on update?","commit_id":"938ec1dfe0bd530357d1050bca3635b764aec0eb"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"e470df062c6201ab92c46c710be5bc92385ed011","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        return self.db.get(\u0027endpoint-%s\u0027 % endpoint_id)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def update_endpoint(self, endpoint_id, endpoint):"},{"line_number":132,"context_line":"        if endpoint.get(\u0027region_id\u0027) is not None:"},{"line_number":133,"context_line":"            self.get_region(endpoint[\u0027region_id\u0027])"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        old_endpoint \u003d self.get_endpoint(endpoint_id)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_b8e50e0c","line":132,"in_reply_to":"3a961159_dbbc9f09","updated":"2014-12-16 02:06:01.000000000","message":"OK... It is a little confused in our current code. Here I just copy from sql driver. Let me answer your questions now:\n1. Yes, we can update the region_id to be None.\n2. For create_endpoint, I think we should check region too, because we have checked service_id in controller layer already:https://github.com/openstack/keystone/blob/master/keystone/catalog/controllers.py#L298 but not check region. And sql driver checks region for create_endpoint too.\n3. The reason that we check region and not service here is we have checked service in controller layer already...https://github.com/openstack/keystone/blob/master/keystone/catalog/controllers.py#L321\n\nI think we should refactor the code... I think we should move the region and service exist checks into core layer. I will do this soon.","commit_id":"938ec1dfe0bd530357d1050bca3635b764aec0eb"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"dcd6a5dfe7e995e8b20cfbf7c175dc7a2105fdeb","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def update_region(self, region_id, region):"},{"line_number":70,"context_line":"        self._check_parent_region(region)"},{"line_number":71,"context_line":"        old_region \u003d self.get_region(region_id)"},{"line_number":72,"context_line":"        old_region.update(region)"},{"line_number":73,"context_line":"        self.db.set(\u0027region-%s\u0027 % region_id, old_region)"},{"line_number":74,"context_line":"        return old_region"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_5c113b5c","line":72,"updated":"2014-12-31 15:07:22.000000000","message":"does this allow the region I\u0027d to be changed, thus conflicting with the URL?","commit_id":"211d008e4939326cc0e38ed1a9253fcffe01cb64"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"bc116449e20bf988ee327fd4d9bae13760422c38","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def update_region(self, region_id, region):"},{"line_number":70,"context_line":"        self._check_parent_region(region)"},{"line_number":71,"context_line":"        old_region \u003d self.get_region(region_id)"},{"line_number":72,"context_line":"        old_region.update(region)"},{"line_number":73,"context_line":"        self.db.set(\u0027region-%s\u0027 % region_id, old_region)"},{"line_number":74,"context_line":"        return old_region"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_7d6c1f37","line":72,"in_reply_to":"3a961159_5c113b5c","updated":"2015-01-04 01:01:11.000000000","message":"I think it is OK, we have checked id matching in controller layer: https://github.com/openstack/keystone/blob/master/keystone/catalog/controllers.py#L199","commit_id":"211d008e4939326cc0e38ed1a9253fcffe01cb64"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"e8520e773b4181711653403f12df169c26a4c4a9","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def update_region(self, region_id, region):"},{"line_number":70,"context_line":"        self._check_parent_region(region)"},{"line_number":71,"context_line":"        old_region \u003d self.get_region(region_id)"},{"line_number":72,"context_line":"        old_region.update(region)"},{"line_number":73,"context_line":"        self.db.set(\u0027region-%s\u0027 % region_id, old_region)"},{"line_number":74,"context_line":"        return old_region"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_e9028e3c","line":72,"in_reply_to":"3a961159_7d6c1f37","updated":"2015-01-08 20:22:01.000000000","message":"It would probably be better if the matching Id was checked in the manager, but that is out of scope of the bug fix. \n\nThis looks good here.","commit_id":"211d008e4939326cc0e38ed1a9253fcffe01cb64"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"dcd6a5dfe7e995e8b20cfbf7c175dc7a2105fdeb","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def update_service(self, service_id, service):"},{"line_number":99,"context_line":"        old_service \u003d self.get_service(service_id)"},{"line_number":100,"context_line":"        old_service.update(service)"},{"line_number":101,"context_line":"        self.db.set(\u0027service-%s\u0027 % service_id, old_service)"},{"line_number":102,"context_line":"        return old_service"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_7c14374d","line":100,"updated":"2014-12-31 15:07:22.000000000","message":"same","commit_id":"211d008e4939326cc0e38ed1a9253fcffe01cb64"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"e8520e773b4181711653403f12df169c26a4c4a9","unresolved":false,"context_lines":[{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def update_service(self, service_id, service):"},{"line_number":99,"context_line":"        old_service \u003d self.get_service(service_id)"},{"line_number":100,"context_line":"        old_service.update(service)"},{"line_number":101,"context_line":"        self.db.set(\u0027service-%s\u0027 % service_id, old_service)"},{"line_number":102,"context_line":"        return old_service"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_49f48254","line":100,"in_reply_to":"3a961159_7c14374d","updated":"2015-01-08 20:22:01.000000000","message":"Same","commit_id":"211d008e4939326cc0e38ed1a9253fcffe01cb64"}],"keystone/tests/test_backend.py":[{"author":{"_account_id":2218,"name":"Adam Young","email":"adam@younglogic.com","username":"ayoung"},"change_message_id":"c6cd334e7ce5f49ec2e64fd6145a04f833e77250","unresolved":false,"context_lines":[{"line_number":3843,"context_line":"        services \u003d self.catalog_api.list_services()"},{"line_number":3844,"context_line":"        self.assertIn(service_id, [x[\u0027id\u0027] for x in services])"},{"line_number":3845,"context_line":""},{"line_number":3846,"context_line":"        # update"},{"line_number":3847,"context_line":"        res \u003d self.catalog_api.update_service(service_id, {\u0027name\u0027: \u0027zz\u0027})"},{"line_number":3848,"context_line":"        expected_service \u003d new_service.copy()"},{"line_number":3849,"context_line":"        expected_service[\u0027name\u0027] \u003d \u0027zz\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_d35c7180","line":3846,"updated":"2014-12-05 18:19:21.000000000","message":"the standard for naming is to use uuids, not magic constants, in tests for ids and names","commit_id":"0e97f7d6ee7547938d9ecba0f62f88f51af54e4b"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"bc8a12b9ea2f40aa4f0a158302b67d555fac9d4a","unresolved":false,"context_lines":[{"line_number":4717,"context_line":"        self.assertRaises(exception.NotFound,"},{"line_number":4718,"context_line":"                          self.catalog_api.update_endpoint,"},{"line_number":4719,"context_line":"                          endpoint_ref[\u0027id\u0027],"},{"line_number":4720,"context_line":"                          {\u0027region_id\u0027: \u0027non_exist_region\u0027})"},{"line_number":4721,"context_line":""},{"line_number":4722,"context_line":"    def _create_endpoints(self):"},{"line_number":4723,"context_line":"        # Creates a service and 2 endpoints for the service in the same region."}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_1bd2a734","line":4720,"updated":"2014-12-15 19:22:52.000000000","message":"use a uuid for the region_id.","commit_id":"938ec1dfe0bd530357d1050bca3635b764aec0eb"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"1a88acd2d0b1cf95f7dce6709c6fb501fd086039","unresolved":false,"context_lines":[{"line_number":4712,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4713,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4714,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4715,"context_line":"            # TODO(wanghong) currently, kvs driver does not set enabled default"},{"line_number":4716,"context_line":"            # to True when creating endpoint. This line should be removed if"},{"line_number":4717,"context_line":"            # this patch https://review.openstack.org/#/c/139958/ is merged."},{"line_number":4718,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_f9cf8e55","line":4715,"updated":"2014-12-16 12:02:20.000000000","message":"nit: KVS","commit_id":"0c1f2b4239dd2628eb32822b572bde7da3e2a7b9"},{"author":{"_account_id":11022,"name":"Rodrigo Duarte Sousa","email":"rodrigodsousa@gmail.com","username":"rodrigods"},"change_message_id":"1a88acd2d0b1cf95f7dce6709c6fb501fd086039","unresolved":false,"context_lines":[{"line_number":4714,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4715,"context_line":"            # TODO(wanghong) currently, kvs driver does not set enabled default"},{"line_number":4716,"context_line":"            # to True when creating endpoint. This line should be removed if"},{"line_number":4717,"context_line":"            # this patch https://review.openstack.org/#/c/139958/ is merged."},{"line_number":4718,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4719,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4720,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_99084a93","line":4717,"updated":"2014-12-16 12:02:20.000000000","message":"Prefer to have the other patch as dependency of this one. Or, in the other way around, the other patch removing this message.","commit_id":"0c1f2b4239dd2628eb32822b572bde7da3e2a7b9"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"a9e778b901804a407001f5d79841505de4810b11","unresolved":false,"context_lines":[{"line_number":4714,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4715,"context_line":"            # TODO(wanghong) currently, kvs driver does not set enabled default"},{"line_number":4716,"context_line":"            # to True when creating endpoint. This line should be removed if"},{"line_number":4717,"context_line":"            # this patch https://review.openstack.org/#/c/139958/ is merged."},{"line_number":4718,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4719,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4720,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_5ff7868c","line":4717,"in_reply_to":"3a961159_811c41c3","updated":"2014-12-17 03:02:52.000000000","message":"I will give an other patch as dependency of this one.","commit_id":"0c1f2b4239dd2628eb32822b572bde7da3e2a7b9"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"036cf62a51edf875a452de67197fb046efd03b80","unresolved":false,"context_lines":[{"line_number":4714,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4715,"context_line":"            # TODO(wanghong) currently, kvs driver does not set enabled default"},{"line_number":4716,"context_line":"            # to True when creating endpoint. This line should be removed if"},{"line_number":4717,"context_line":"            # this patch https://review.openstack.org/#/c/139958/ is merged."},{"line_number":4718,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4719,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4720,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_811c41c3","line":4717,"in_reply_to":"3a961159_99084a93","updated":"2014-12-16 20:28:57.000000000","message":"yes, let\u0027s fix the bug rather than have a test that\u0027s guessing as to what\u0027s going to happen.","commit_id":"0c1f2b4239dd2628eb32822b572bde7da3e2a7b9"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"97192f528c19cc813d7d6167e42552e26bc4697c","unresolved":false,"context_lines":[{"line_number":4718,"context_line":"        expected_endpoint \u003d endpoint_ref.copy()"},{"line_number":4719,"context_line":"        expected_endpoint[\u0027interface\u0027] \u003d \u0027private\u0027"},{"line_number":4720,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4721,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4722,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4723,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4724,"context_line":"        self.assertDictEqual(expected_endpoint, res)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_fd6bbe82","line":4721,"updated":"2014-12-17 21:51:28.000000000","message":"why does this need to be done? add a comment.","commit_id":"c389a4983ce4d27b2f3155d9514ed0653ac094a7"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"97192f528c19cc813d7d6167e42552e26bc4697c","unresolved":false,"context_lines":[{"line_number":4719,"context_line":"        expected_endpoint[\u0027interface\u0027] \u003d \u0027private\u0027"},{"line_number":4720,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4721,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4722,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4723,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4724,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4725,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_7d0fcee1","line":4722,"updated":"2014-12-17 21:51:28.000000000","message":"why does this need to be done? add a comment.\n\nTest code should be asserting what\u0027s going to be in the result, not reacting to it.","commit_id":"c389a4983ce4d27b2f3155d9514ed0653ac094a7"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"1358cd44d2d20294dbc59f7d230afadf44b8e7e1","unresolved":false,"context_lines":[{"line_number":4719,"context_line":"        expected_endpoint[\u0027interface\u0027] \u003d \u0027private\u0027"},{"line_number":4720,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4721,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4722,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4723,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4724,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4725,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_22e99d1a","line":4722,"in_reply_to":"3a961159_7d0fcee1","updated":"2014-12-18 01:47:12.000000000","message":"These two lines would be removed in the following review which depends on this review. Should I add a TODO comment here?","commit_id":"c389a4983ce4d27b2f3155d9514ed0653ac094a7"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"599fdad492fe3aa443b0c1aa13e03690da270a01","unresolved":false,"context_lines":[{"line_number":4725,"context_line":"        expected_endpoint \u003d endpoint_ref.copy()"},{"line_number":4726,"context_line":"        expected_endpoint[\u0027interface\u0027] \u003d \u0027private\u0027"},{"line_number":4727,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4728,"context_line":"            # legacy_endpoint_id is just an internal attribute used for"},{"line_number":4729,"context_line":"            # backward compatibility. It is not exposed to user and irrelevant"},{"line_number":4730,"context_line":"            # to the tested behaviour"},{"line_number":4731,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_b45e92e7","line":4728,"updated":"2014-12-18 16:02:36.000000000","message":"remove \"just\"","commit_id":"64c2aab21dd3d59293333ab847c7fa34978905ea"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"599fdad492fe3aa443b0c1aa13e03690da270a01","unresolved":false,"context_lines":[{"line_number":4727,"context_line":"        if \u0027legacy_endpoint_id\u0027 in res:"},{"line_number":4728,"context_line":"            # legacy_endpoint_id is just an internal attribute used for"},{"line_number":4729,"context_line":"            # backward compatibility. It is not exposed to user and irrelevant"},{"line_number":4730,"context_line":"            # to the tested behaviour"},{"line_number":4731,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4732,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4733,"context_line":"            # TODO(wanghong) let the test pass, this will be removed in the"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_d4bdded8","line":4730,"updated":"2014-12-18 16:02:36.000000000","message":"This test is checking that the expected response when doing an update of an endpoint, otherwise the name of the test is incorrect. So I think that some backends are expected to include a legacy_endpoint_id and some aren\u0027t is relevant.\n\nThis could be fixed by having a field in the test class that legacy_endpoint_id is expected and use that:\n\n if not self._legacy_endpoint_id_in_endpoint:\n   del res[\u0027legacy_endpoint_id\u0027]\n\ndefault _legacy_endpoint_id_in_endpoint to False in this class and set it to True in backends that use it.","commit_id":"64c2aab21dd3d59293333ab847c7fa34978905ea"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"599fdad492fe3aa443b0c1aa13e03690da270a01","unresolved":false,"context_lines":[{"line_number":4731,"context_line":"            res.pop(\u0027legacy_endpoint_id\u0027)"},{"line_number":4732,"context_line":"        if \u0027enabled\u0027 in res:"},{"line_number":4733,"context_line":"            # TODO(wanghong) let the test pass, this will be removed in the"},{"line_number":4734,"context_line":"            # following review which depends on this soon"},{"line_number":4735,"context_line":"            expected_endpoint[\u0027enabled\u0027] \u003d True"},{"line_number":4736,"context_line":"        self.assertDictEqual(expected_endpoint, res)"},{"line_number":4737,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_9472760f","line":4734,"updated":"2014-12-18 16:02:36.000000000","message":"The following review might not be accepted.\n\nUse a similar technique so that the test knows when enabled is expected to be in the result.","commit_id":"64c2aab21dd3d59293333ab847c7fa34978905ea"}],"keystone/tests/test_backend_sql.py":[{"author":{"_account_id":2218,"name":"Adam Young","email":"adam@younglogic.com","username":"ayoung"},"change_message_id":"c6cd334e7ce5f49ec2e64fd6145a04f833e77250","unresolved":false,"context_lines":[{"line_number":613,"context_line":"                          self.catalog_api.delete_region,"},{"line_number":614,"context_line":"                          region[\u0027id\u0027])"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"    def test_update_endpoint(self):"},{"line_number":617,"context_line":"        dummy_service_ref, endpoint_ref, dummy_disabled_endpoint_ref \u003d ("},{"line_number":618,"context_line":"            self._create_endpoints())"},{"line_number":619,"context_line":"        res \u003d self.catalog_api.update_endpoint(endpoint_ref[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_53dfa1d6","line":616,"updated":"2014-12-05 18:19:21.000000000","message":"there doesn\u0027t look to be anything sql specific about this.  Is it possible to push this to the baseclass?","commit_id":"0e97f7d6ee7547938d9ecba0f62f88f51af54e4b"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"7383e544578def350089073a7dec9725ac005709","unresolved":false,"context_lines":[{"line_number":613,"context_line":"                          self.catalog_api.delete_region,"},{"line_number":614,"context_line":"                          region[\u0027id\u0027])"},{"line_number":615,"context_line":""},{"line_number":616,"context_line":"    def test_update_endpoint(self):"},{"line_number":617,"context_line":"        dummy_service_ref, endpoint_ref, dummy_disabled_endpoint_ref \u003d ("},{"line_number":618,"context_line":"            self._create_endpoints())"},{"line_number":619,"context_line":"        res \u003d self.catalog_api.update_endpoint(endpoint_ref[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_a2ecffc8","line":616,"in_reply_to":"5a890539_53dfa1d6","updated":"2014-12-08 02:18:48.000000000","message":"The enabled would be True and legacy_endpoint_id would be None when using sql backend, but kvs backend not.","commit_id":"0e97f7d6ee7547938d9ecba0f62f88f51af54e4b"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"bc8a12b9ea2f40aa4f0a158302b67d555fac9d4a","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                          self.catalog_api.delete_region,"},{"line_number":654,"context_line":"                          region[\u0027id\u0027])"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"    def test_update_endpoint(self):"},{"line_number":657,"context_line":"        dummy_service_ref, endpoint_ref, dummy_disabled_endpoint_ref \u003d ("},{"line_number":658,"context_line":"            self._create_endpoints())"},{"line_number":659,"context_line":"        res \u003d self.catalog_api.update_endpoint(endpoint_ref[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_9bc85758","line":656,"updated":"2014-12-15 19:22:52.000000000","message":"the same test should work on test_backend_sql as it does on test_backend... if it doesn\u0027t at least add a comment why not.","commit_id":"938ec1dfe0bd530357d1050bca3635b764aec0eb"}]}