)]}'
{"dragonflow/neutron/common/constants.py":[{"author":{"_account_id":7805,"name":"Li Ma","email":"skywalker.nick@gmail.com","username":"Nick"},"change_message_id":"900ed9c88dab950e7beaf2db9140ec8efe5bdc60","unresolved":false,"context_lines":[{"line_number":15,"context_line":"DF_NETWORK_NAME_EXT_ID_KEY \u003d \u0027neutron:network_name\u0027"},{"line_number":16,"context_line":"DF_PORT_NAME_EXT_ID_KEY \u003d \u0027neutron:port_name\u0027"},{"line_number":17,"context_line":"DF_ROUTER_NAME_EXT_ID_KEY \u003d \u0027neutron:router_name\u0027"},{"line_number":18,"context_line":"DF_SG_NAME_EXT_ID_KEY \u003d \u0027neutron:security_group_name\u0027"},{"line_number":19,"context_line":"DF_PORT_BINDING_PROFILE \u003d portbindings.PROFILE"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_d6958840","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":53},"updated":"2016-05-14 07:02:49.000000000","message":"I noticed this patch has conflicts with https://review.openstack.org/#/c/315433/.","commit_id":"29d35d05ca73e27819a859babe1010d77b22be9d"}],"dragonflow/neutron/plugin.py":[{"author":{"_account_id":7805,"name":"Li Ma","email":"skywalker.nick@gmail.com","username":"Nick"},"change_message_id":"900ed9c88dab950e7beaf2db9140ec8efe5bdc60","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        tenant_id \u003d sg_db[\u0027tenant_id\u0027]"},{"line_number":209,"context_line":"        rules \u003d sg_db.get(\u0027security_group_rules\u0027)"},{"line_number":210,"context_line":"        external_ids \u003d {df_const.DF_SG_NAME_EXT_ID_KEY:"},{"line_number":211,"context_line":"                        sg_db.get(\u0027name\u0027, \u0027no_sg_name\u0027)}"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"        self.nb_api.create_security_group(name\u003dsg_name, topic\u003dtenant_id,"},{"line_number":214,"context_line":"                                          rules\u003drules,"}],"source_content_type":"text/x-python","patch_set":3,"id":"dab17558_76b77cf5","line":211,"range":{"start_line":211,"start_character":43,"end_line":211,"end_character":53},"updated":"2016-05-14 07:02:49.000000000","message":"constants?","commit_id":"29d35d05ca73e27819a859babe1010d77b22be9d"}],"dragonflow/tests/fullstack/test_neutron_api.py":[{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"061a8da8409347c8baa686994018ba1cf0ebe46a","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        router.update()"},{"line_number":91,"context_line":"        self.assertTrue(router.exists())"},{"line_number":92,"context_line":"        router_name2 \u003d self.nb_api.get_router(router_id).get_external_ids()"},{"line_number":93,"context_line":"        self.assertTrue(router_name1 !\u003d router_name2)"},{"line_number":94,"context_line":"        router.close()"},{"line_number":95,"context_line":"        self.assertFalse(router.exists())"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_217a4324","line":93,"updated":"2016-05-23 18:06:13.000000000","message":"I don\u0027t love the approach of having different defaults for router name in the create and update calls, but I can\u0027t actually say it\u0027s a defect. But this code would be clearer to me if you passed two different router names. I wasted time wondering why the router name would change between the two calls to get_external_ids until I looked at the code of the create and update functions.","commit_id":"29d35d05ca73e27819a859babe1010d77b22be9d"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"f389488a7f4794e5428d2d1c0bb9fbe61dc9a247","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        router.update()"},{"line_number":91,"context_line":"        self.assertTrue(router.exists())"},{"line_number":92,"context_line":"        router_name2 \u003d self.nb_api.get_router(router_id).get_external_ids()"},{"line_number":93,"context_line":"        self.assertTrue(router_name1 !\u003d router_name2)"},{"line_number":94,"context_line":"        router.close()"},{"line_number":95,"context_line":"        self.assertFalse(router.exists())"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_b6481ece","line":93,"in_reply_to":"bab6814e_217a4324","updated":"2016-05-24 07:14:17.000000000","message":"Sorry to confuse you here, and we have the version field now, I\u0027ll modify the code, thx:)","commit_id":"29d35d05ca73e27819a859babe1010d77b22be9d"},{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"061a8da8409347c8baa686994018ba1cf0ebe46a","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        secgroup.update()"},{"line_number":187,"context_line":"        self.assertTrue(secgroup.exists())"},{"line_number":188,"context_line":"        sg_name2 \u003d self.nb_api.get_security_group(sg_id).get_external_ids()"},{"line_number":189,"context_line":"        self.assertTrue(sg_name1 !\u003d sg_name2)"},{"line_number":190,"context_line":"        secgroup.close()"},{"line_number":191,"context_line":"        self.assertFalse(secgroup.exists())"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_e1ff9b63","line":189,"updated":"2016-05-23 18:06:13.000000000","message":"Same comment as in the router section, this code doesn\u0027t show any clear reason why I would expect the name to change between the two seemingly identical calls to get_external_ids() without having to go look and see that create and update use different defaults.","commit_id":"29d35d05ca73e27819a859babe1010d77b22be9d"},{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"855582f0d73ac02931974434a31b819b2db88925","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        router.update()"},{"line_number":91,"context_line":"        self.assertTrue(router.exists())"},{"line_number":92,"context_line":"        version2 \u003d self.nb_api.get_router(router_id).get_version()"},{"line_number":93,"context_line":"        self.assertTrue(version1 !\u003d version2)"},{"line_number":94,"context_line":"        router.close()"},{"line_number":95,"context_line":"        self.assertFalse(router.exists())"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bab6814e_67b8ade6","line":93,"updated":"2016-05-24 13:52:32.000000000","message":"This is actually more confusing on casual reading but educational. It lead me to do a lot more digging into the code. I assumed that each router object must have a version field and that every call to update() must increment that version field. I found it in dragonflow/db/neutron/versionobjects_db.py so that was educational.","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":13070,"name":"Eran Gampel","email":"eran@gampel.net","username":"Gampel"},"change_message_id":"353f8986722fe74408d51b396587efa9088e59f8","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        router.update()"},{"line_number":91,"context_line":"        self.assertTrue(router.exists())"},{"line_number":92,"context_line":"        version2 \u003d self.nb_api.get_router(router_id).get_version()"},{"line_number":93,"context_line":"        self.assertTrue(version1 !\u003d version2)"},{"line_number":94,"context_line":"        router.close()"},{"line_number":95,"context_line":"        self.assertFalse(router.exists())"},{"line_number":96,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_9f49888e","line":93,"in_reply_to":"bab6814e_67b8ade6","updated":"2016-05-29 10:50:03.000000000","message":"Yes this is a new feature that was merged recently","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"}],"dragonflow/tests/fullstack/test_objects.py":[{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"855582f0d73ac02931974434a31b819b2db88925","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bab6814e_87fdd170","line":66,"updated":"2016-05-24 13:52:32.000000000","message":"I still think the inclusion of a default router name in update, different from the default router name in create is weird, but I guess not a defect.","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"19f4c6be7be756e8681c1497f055447162736dd9","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_755af351","line":66,"in_reply_to":"9abb7d3a_5f360004","updated":"2016-05-30 09:52:50.000000000","message":"Hi, Eran and Paul, thx for your review, but I don\u0027t understand why other fields are better than name field in the object update operation, in cli, neutron api or dashboard, it is a very common senario to change the name of object","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"657629ad56cec31dcc550e881913283cab6405ab","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_f4018439","line":66,"in_reply_to":"9abb7d3a_755af351","updated":"2016-05-31 13:31:25.000000000","message":"It\u0027s not the name specifically that bothers me, it\u0027s that it\u0027s totally non-obvious from outside the function. I\u0027d rather not have a default here, and instead call the function with the new name as a parameter. This function is simply called \u0027update\u0027, not \u0027update_name_to_myrouter2\u0027, so why should it have a hard coded name preference?\n\nAs Eran said, it\u0027s not a mandatory fix. I just think it\u0027s odd to have a default value in this function. I\u0027d prefer that you just pass the name as a parameter from the test where you call it.","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":13070,"name":"Eran Gampel","email":"eran@gampel.net","username":"Gampel"},"change_message_id":"e20b579589923cb910d9967f326eea53bbbd9497","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_8219157c","line":66,"in_reply_to":"9abb7d3a_755af351","updated":"2016-05-30 12:27:06.000000000","message":"No problem it is not a mandatory fix","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":20287,"name":"hujie","email":"mike.hu@huawei.com","username":"hujie"},"change_message_id":"4798d2e4e5ea12c38386094beba17c25123002c4","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_f3569a28","line":66,"in_reply_to":"9abb7d3a_f4018439","updated":"2016-06-01 01:12:57.000000000","message":"OK, I got you, thx for your patient explaination:)","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":13070,"name":"Eran Gampel","email":"eran@gampel.net","username":"Gampel"},"change_message_id":"353f8986722fe74408d51b396587efa9088e59f8","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.router_id \u003d new_router[\u0027router\u0027][\u0027id\u0027]"},{"line_number":64,"context_line":"        return self.router_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def update(self, router\u003d{\u0027name\u0027: \u0027myrouter2\u0027}):"},{"line_number":67,"context_line":"        router \u003d self.neutron.update_router("},{"line_number":68,"context_line":"                self.router_id, {\u0027router\u0027: router})"},{"line_number":69,"context_line":"        return router[\u0027router\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9abb7d3a_5f360004","line":66,"in_reply_to":"bab6814e_87fdd170","updated":"2016-05-29 10:50:03.000000000","message":"I agree that it would  be better to update one of the other fields","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"},{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"855582f0d73ac02931974434a31b819b2db88925","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        self.secgroup_id \u003d new_secgroup[\u0027security_group\u0027][\u0027id\u0027]"},{"line_number":116,"context_line":"        return self.secgroup_id"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def update(self, secgroup\u003d{\u0027name\u0027: \u0027mysecgroup2\u0027}):"},{"line_number":119,"context_line":"        update_secgroup \u003d self.neutron.update_security_group("},{"line_number":120,"context_line":"                self.secgroup_id, {\u0027security_group\u0027: secgroup})"},{"line_number":121,"context_line":"        return update_secgroup[\u0027security_group\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bab6814e_471129af","line":118,"updated":"2016-05-24 13:52:32.000000000","message":"Same here. Weird that calling create gives the security group a default name and calling update changes the name.","commit_id":"d4b301c89ea0eadae73412b040927012c7dc5d85"}]}