)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"d320b53dfa80d7984cddd3ffc7078294ed2d40ae","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     af49578a (Add BGP Dynamic Routing DB Model and Basic CRUD)"},{"line_number":2,"context_line":"Author:     vikram.choudhary \u003cvikram.choudhary@huawei.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2016-01-03 20:30:08 +0530"},{"line_number":4,"context_line":"Commit:     Ryan Tidwell \u003cryan.tidwell@hpe.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-01-06 16:44:03 -0800"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"fa69d971_693a8fe9","line":2,"updated":"2016-01-08 03:45:56.000000000","message":"I really got to learn GIT. Ryan plz go ahead and change the owner as it\u0027s your patch. In the meantime I will figure out what stupidity I am doing.","commit_id":"de32acca3446ab90045436cbb9d1828930cc0b52"}],"neutron/db/bgp_db.py":[{"author":{"_account_id":17455,"name":"Na Zhu","email":"juno.zhu@huawei.com","username":"Na"},"change_message_id":"f93cc2bf531aa47ad9cff1f74a7de7d0369f3a1a","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def get_advertised_routes(self, context, bgp_speaker_id):"},{"line_number":262,"context_line":"        routes \u003d self.get_routes_by_bgp_speaker_id(context, bgp_speaker_id)"},{"line_number":263,"context_line":"        return self._make_advertised_routes_dict(routes)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def _get_id_for(self, resource, id_name):"},{"line_number":266,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_33d515e4","line":263,"updated":"2015-11-30 13:01:59.000000000","message":"This is for command \"neutron bgp-speaker-advertiseroute-list\", do not call self._make_advertised_routes_dict at the end, just return the routes, otherwise the neutronclient function ListRoutesAdvertisedBySpeaker return nothing.","commit_id":"9ae3a556d63e82d13739a52324c7e63d0e43c13c"},{"author":{"_account_id":17455,"name":"Na Zhu","email":"juno.zhu@huawei.com","username":"Na"},"change_message_id":"c39a61db9d00853cefe54bc9b9cad08fc238fd73","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def get_advertised_routes(self, context, bgp_speaker_id):"},{"line_number":262,"context_line":"        routes \u003d self.get_routes_by_bgp_speaker_id(context, bgp_speaker_id)"},{"line_number":263,"context_line":"        return self._make_advertised_routes_dict(routes)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def _get_id_for(self, resource, id_name):"},{"line_number":266,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_34f715f7","line":263,"updated":"2015-12-01 03:59:24.000000000","message":"This is for command \"neutron bgp-speaker-advertiseroute-list\" right? if yes, do not call self._make_advertised_routes_dict at the end, just return the routes, otherwise the neutronclient function ListRoutesAdvertisedBySpeaker return nothing.","commit_id":"64ba7e2dd5f75c5cf4b717c1618b557a371962a3"},{"author":{"_account_id":17455,"name":"Na Zhu","email":"juno.zhu@huawei.com","username":"Na"},"change_message_id":"42a1026fd3a8a846b3982845fc46d59bd730fb3e","unresolved":false,"context_lines":[{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def get_advertised_routes(self, context, bgp_speaker_id):"},{"line_number":262,"context_line":"        routes \u003d self.get_routes_by_bgp_speaker_id(context, bgp_speaker_id)"},{"line_number":263,"context_line":"        return self._make_advertised_routes_dict(routes)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def _get_id_for(self, resource, id_name):"},{"line_number":266,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_32f821c3","line":263,"in_reply_to":"9a8ffd7b_34f715f7","updated":"2015-12-02 14:59:53.000000000","message":"The command \"neutron bgp-speaker-advertiseroute-list\" return nothing is because the neutronclient code is not right, no need to change this, pls ignore it, sorry.","commit_id":"64ba7e2dd5f75c5cf4b717c1618b557a371962a3"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"d36d6d4dd1016ae955cc86ff163dd248fd308f21","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            return self._make_bgp_speaker_dict(bgp_speaker, fields)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @log_helpers.log_method_call"},{"line_number":146,"context_line":"    @log_helpers.log_method_call"},{"line_number":147,"context_line":"    def update_bgp_speaker(self, context, bgp_speaker_id, bgp_speaker):"},{"line_number":148,"context_line":"        bp \u003d bgp_speaker[bgp_ext.BGP_SPEAKER_BODY_KEY_NAME]"},{"line_number":149,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa69d971_a7f53284","line":146,"updated":"2015-12-30 10:09:21.000000000","message":"Plz remove. Re-base mistake.","commit_id":"474f94c7ce3c897af1c71a213d86d46e719a1f12"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"d36d6d4dd1016ae955cc86ff163dd248fd308f21","unresolved":false,"context_lines":[{"line_number":749,"context_line":"            try:"},{"line_number":750,"context_line":"                return query.one()"},{"line_number":751,"context_line":"            except sa_exc.NoResultFound:"},{"line_number":752,"context_line":"                raise bgp_ext.NetworkNotBoundForIpVersion("},{"line_number":753,"context_line":"                                                        network_id\u003dnetwork_id,"},{"line_number":754,"context_line":"                                                        ip_version\u003dip_version)"},{"line_number":755,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa69d971_e7a32a4e","line":752,"updated":"2015-12-30 10:09:21.000000000","message":"This is a new exception. Please add it in extension/bgp.py.","commit_id":"474f94c7ce3c897af1c71a213d86d46e719a1f12"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"e76b5a136700f70eb2afd12ab2636bf8f798d7bf","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            bgp_speaker \u003d self._get_bgp_speaker(context, bgp_speaker_id)"},{"line_number":143,"context_line":"            return self._make_bgp_speaker_dict(bgp_speaker, fields)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @log_helpers.log_method_call"},{"line_number":146,"context_line":"    @log_helpers.log_method_call"},{"line_number":147,"context_line":"    def update_bgp_speaker(self, context, bgp_speaker_id, bgp_speaker):"},{"line_number":148,"context_line":"        bp \u003d bgp_speaker[bgp_ext.BGP_SPEAKER_BODY_KEY_NAME]"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa69d971_6cbfeffb","line":145,"updated":"2016-01-01 12:28:12.000000000","message":"remove this. bad rebase.","commit_id":"e6cef1afec1c01d9f23206fcd864f76d4f8d70c0"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"ea38b90edc5e3a26016edbb0fafea45c3d4a3ae8","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            res[\u0027peers\u0027] \u003d self.get_bgp_peers_by_bgp_speaker(context,"},{"line_number":152,"context_line":"                                                         bgp_speaker[\u0027id\u0027],"},{"line_number":153,"context_line":"                                                         fields\u003dbgp_peer_attrs)"},{"line_number":154,"context_line":"            res[\u0027advertised_routes\u0027] \u003d []"},{"line_number":155,"context_line":"            return res"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def update_bgp_speaker(self, context, bgp_speaker_id, bgp_speaker):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a5de9d1_b133b7aa","line":154,"updated":"2016-01-29 15:06:05.000000000","message":"I think you need to call get_routes_by_bgp_speaker_id.\n\nres[\u0027advertised_routes\u0027] \u003d get_routes_by_bgp_speaker_id","commit_id":"baa54704c5fdae9914a8367291429404d7ba45fe"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"60b16f567e8010c1d92e2e57a9949c71327f8324","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                                            bgp_peer_id)"},{"line_number":183,"context_line":"        return {\u0027bgp_peer_id\u0027: bgp_peer_id}"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @log_helpers.log_method_call"},{"line_number":186,"context_line":"    def remove_bgp_peer(self, context, bgp_speaker_id, bgp_peer_info):"},{"line_number":187,"context_line":"        bgp_peer_id \u003d self._get_id_for(bgp_peer_info, \u0027bgp_peer_id\u0027)"},{"line_number":188,"context_line":"        self._remove_bgp_speaker_peer_binding(context,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a5de9d1_d6018907","line":185,"updated":"2016-01-29 14:58:37.000000000","message":"log could be removed","commit_id":"baa54704c5fdae9914a8367291429404d7ba45fe"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":267,"context_line":"        except sa_exc.NoResultFound:"},{"line_number":268,"context_line":"            raise bgp_ext.BgpSpeakerNotFound(id\u003dbgp_speaker_id)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def _get_bgp_speakers_by_binding_network(self, context, network_id):"},{"line_number":271,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":272,"context_line":"            query \u003d context.session.query(BgpSpeakerNetworkBinding)"},{"line_number":273,"context_line":"            query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_71262318","side":"PARENT","line":270,"range":{"start_line":270,"start_character":8,"end_line":270,"end_character":44},"updated":"2016-02-03 05:17:09.000000000","message":"Why rename (and repurpose) a method that was added in a patch set that hasn\u0027t merged yet?  Just wondering.  Actually, I noticed the original method was never called in the original.  So, just remove it from the original.","commit_id":"9a27a5510374953eaf0cc1dfcf4835cbb2fe08a4"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":267,"context_line":"        except sa_exc.NoResultFound:"},{"line_number":268,"context_line":"            raise bgp_ext.BgpSpeakerNotFound(id\u003dbgp_speaker_id)"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    def _get_bgp_speakers_by_binding_network(self, context, network_id):"},{"line_number":271,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":272,"context_line":"            query \u003d context.session.query(BgpSpeakerNetworkBinding)"},{"line_number":273,"context_line":"            query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_845b5b54","side":"PARENT","line":270,"range":{"start_line":270,"start_character":8,"end_line":270,"end_character":44},"in_reply_to":"5a5ae5dd_71262318","updated":"2016-02-03 05:43:08.000000000","message":"Hmm, not sure what\u0027s going on here. Bad rebase? I\u0027ll clean this up. Lesson learned, don\u0027t implement methods until you need them","commit_id":"9a27a5510374953eaf0cc1dfcf4835cbb2fe08a4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":21,"context_line":"import sqlalchemy as sa"},{"line_number":22,"context_line":"from sqlalchemy import and_"},{"line_number":23,"context_line":"from sqlalchemy import orm"},{"line_number":24,"context_line":"from sqlalchemy.orm import aliased"},{"line_number":25,"context_line":"from sqlalchemy.orm import exc as sa_exc"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron.api.v2 import attributes as attr"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_c40dd338","line":24,"range":{"start_line":24,"start_character":27,"end_line":24,"end_character":34},"updated":"2016-02-03 05:17:09.000000000","message":"I suspect that most of the usage of aliased in this file is gratuitous.  But, I could be wrong.  Could you explain?  See my notes in the file.\n\nMy understanding of aliases is that they\u0027re useful if you need to reference the same table twice in a query for different reasons, different parts of the query.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":21,"context_line":"import sqlalchemy as sa"},{"line_number":22,"context_line":"from sqlalchemy import and_"},{"line_number":23,"context_line":"from sqlalchemy import orm"},{"line_number":24,"context_line":"from sqlalchemy.orm import aliased"},{"line_number":25,"context_line":"from sqlalchemy.orm import exc as sa_exc"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron.api.v2 import attributes as attr"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_79d1bd56","line":24,"range":{"start_line":24,"start_character":27,"end_line":24,"end_character":34},"in_reply_to":"5a5ae5dd_64d17ffb","updated":"2016-02-04 20:26:43.000000000","message":"See below.  I still don\u0027t think you need it.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":21,"context_line":"import sqlalchemy as sa"},{"line_number":22,"context_line":"from sqlalchemy import and_"},{"line_number":23,"context_line":"from sqlalchemy import orm"},{"line_number":24,"context_line":"from sqlalchemy.orm import aliased"},{"line_number":25,"context_line":"from sqlalchemy.orm import exc as sa_exc"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron.api.v2 import attributes as attr"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_64d17ffb","line":24,"range":{"start_line":24,"start_character":27,"end_line":24,"end_character":34},"in_reply_to":"5a5ae5dd_c40dd338","updated":"2016-02-03 05:43:08.000000000","message":"Yes, there are places where it is needed.  It\u0027s mostly a hack to get around the line-width restrictions as these module names and class names are so long.  We could probably tighten up the imports a little to help.  Under the covers I don\u0027t think it affects performance of the query.  There are probably ways to work without aliased in *most* of these queries.  Probably best to avoid unless necessary.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"420f0c2353b60a2e15e742a78e737015d07ce06c","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            res[\u0027peers\u0027] \u003d self.get_bgp_peers_by_bgp_speaker(context,"},{"line_number":152,"context_line":"                                                         bgp_speaker[\u0027id\u0027],"},{"line_number":153,"context_line":"                                                         fields\u003dbgp_peer_attrs)"},{"line_number":154,"context_line":"            res[\u0027advertised_routes\u0027] \u003d []"},{"line_number":155,"context_line":"            return res"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def update_bgp_speaker(self, context, bgp_speaker_id, bgp_speaker):"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a5de9d1_8112cd55","line":154,"updated":"2016-01-30 09:24:48.000000000","message":"I think you need to call get_routes_by_bgp_speaker_id. Currently, empty route list is being returned.\n\nres[\u0027advertised_routes\u0027] \u003d get_routes_by_bgp_speaker_id","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            res[\u0027peers\u0027] \u003d self.get_bgp_peers_by_bgp_speaker(context,"},{"line_number":152,"context_line":"                                                         bgp_speaker[\u0027id\u0027],"},{"line_number":153,"context_line":"                                                         fields\u003dbgp_peer_attrs)"},{"line_number":154,"context_line":"            res[\u0027advertised_routes\u0027] \u003d []"},{"line_number":155,"context_line":"            return res"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    def update_bgp_speaker(self, context, bgp_speaker_id, bgp_speaker):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_04652b44","line":154,"in_reply_to":"7a5de9d1_8112cd55","updated":"2016-02-03 05:43:08.000000000","message":"Done","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"420f0c2353b60a2e15e742a78e737015d07ce06c","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                                            bgp_peer_id)"},{"line_number":183,"context_line":"        return {\u0027bgp_peer_id\u0027: bgp_peer_id}"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @log_helpers.log_method_call"},{"line_number":186,"context_line":"    def remove_bgp_peer(self, context, bgp_speaker_id, bgp_peer_info):"},{"line_number":187,"context_line":"        bgp_peer_id \u003d self._get_id_for(bgp_peer_info, \u0027bgp_peer_id\u0027)"},{"line_number":188,"context_line":"        self._remove_bgp_speaker_peer_binding(context,"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a5de9d1_41185575","line":185,"updated":"2016-01-30 09:24:48.000000000","message":"remove log","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                                            bgp_peer_id)"},{"line_number":183,"context_line":"        return {\u0027bgp_peer_id\u0027: bgp_peer_id}"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @log_helpers.log_method_call"},{"line_number":186,"context_line":"    def remove_bgp_peer(self, context, bgp_speaker_id, bgp_peer_info):"},{"line_number":187,"context_line":"        bgp_peer_id \u003d self._get_id_for(bgp_peer_info, \u0027bgp_peer_id\u0027)"},{"line_number":188,"context_line":"        self._remove_bgp_speaker_peer_binding(context,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_c45e3376","line":185,"in_reply_to":"5a5ae5dd_b1fd7b70","updated":"2016-02-03 05:43:08.000000000","message":"Done","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                                            bgp_peer_id)"},{"line_number":183,"context_line":"        return {\u0027bgp_peer_id\u0027: bgp_peer_id}"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @log_helpers.log_method_call"},{"line_number":186,"context_line":"    def remove_bgp_peer(self, context, bgp_speaker_id, bgp_peer_info):"},{"line_number":187,"context_line":"        bgp_peer_id \u003d self._get_id_for(bgp_peer_info, \u0027bgp_peer_id\u0027)"},{"line_number":188,"context_line":"        self._remove_bgp_speaker_peer_binding(context,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_b1fd7b70","line":185,"in_reply_to":"7a5de9d1_41185575","updated":"2016-02-03 05:17:09.000000000","message":"Why not remove from the original patch.  It hasn\u0027t merged yet.  :)","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def _get_bgp_speaker_ids_by_router(self, context, router_id):"},{"line_number":278,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":279,"context_line":"            network_binding \u003d aliased(BgpSpeakerNetworkBinding)"},{"line_number":280,"context_line":"            r_port \u003d aliased(l3_db.RouterPort)"},{"line_number":281,"context_line":"            query \u003d context.session.query(network_binding.bgp_speaker_id)"},{"line_number":282,"context_line":"            query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_91ff1f34","line":279,"range":{"start_line":279,"start_character":30,"end_line":279,"end_character":37},"updated":"2016-02-03 05:17:09.000000000","message":"nit:  I\u0027m not sure you need aliased here.  Doesn\u0027t this work:\n\n  network_binding \u003d BgpSpeakerNetworkBinding\n\nI think aliased is useful if the same table is reference twice in a query for two different purposes.  (Or, is it and I missed it?)","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def _get_bgp_speaker_ids_by_router(self, context, router_id):"},{"line_number":278,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":279,"context_line":"            network_binding \u003d aliased(BgpSpeakerNetworkBinding)"},{"line_number":280,"context_line":"            r_port \u003d aliased(l3_db.RouterPort)"},{"line_number":281,"context_line":"            query \u003d context.session.query(network_binding.bgp_speaker_id)"},{"line_number":282,"context_line":"            query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_2eb77987","line":279,"range":{"start_line":279,"start_character":30,"end_line":279,"end_character":37},"in_reply_to":"5a5ae5dd_0409ab95","updated":"2016-02-04 20:26:43.000000000","message":"You sure?  I\u0027m pretty sure the following accomplishes exactly the same result (including getting around the line length limitations.\n\n  network_binding \u003d BgpSpeakerNetworkBinding","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def _get_bgp_speaker_ids_by_router(self, context, router_id):"},{"line_number":278,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":279,"context_line":"            network_binding \u003d aliased(BgpSpeakerNetworkBinding)"},{"line_number":280,"context_line":"            r_port \u003d aliased(l3_db.RouterPort)"},{"line_number":281,"context_line":"            query \u003d context.session.query(network_binding.bgp_speaker_id)"},{"line_number":282,"context_line":"            query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_0409ab95","line":279,"range":{"start_line":279,"start_character":30,"end_line":279,"end_character":37},"in_reply_to":"5a5ae5dd_91ff1f34","updated":"2016-02-03 05:43:08.000000000","message":"See comments at the top of the file.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    def _get_bgp_speaker_ids_by_router(self, context, router_id):"},{"line_number":278,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":279,"context_line":"            network_binding \u003d aliased(BgpSpeakerNetworkBinding)"},{"line_number":280,"context_line":"            r_port \u003d aliased(l3_db.RouterPort)"},{"line_number":281,"context_line":"            query \u003d context.session.query(network_binding.bgp_speaker_id)"},{"line_number":282,"context_line":"            query \u003d query.filter("},{"line_number":283,"context_line":"                      r_port.router_id \u003d\u003d router_id,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_718943ca","line":280,"range":{"start_line":280,"start_character":21,"end_line":280,"end_character":28},"updated":"2016-02-03 05:17:09.000000000","message":"ditto","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                      r_port.port_id \u003d\u003d models_v2.Port.id,"},{"line_number":286,"context_line":"                      models_v2.Port.network_id \u003d\u003d network_binding.network_id)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_511d27bc","line":288,"range":{"start_line":288,"start_character":63,"end_line":288,"end_character":69},"updated":"2016-02-03 05:17:09.000000000","message":"nit:  Usually, I think it isn\u0027t necessary to call .all(), especially just to iterate.  I think it ends up just creating an unnecessary data structure.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                      r_port.port_id \u003d\u003d models_v2.Port.id,"},{"line_number":286,"context_line":"                      models_v2.Port.network_id \u003d\u003d network_binding.network_id)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_e4624f16","line":288,"range":{"start_line":288,"start_character":63,"end_line":288,"end_character":69},"in_reply_to":"5a5ae5dd_511d27bc","updated":"2016-02-03 05:43:08.000000000","message":"My sqlalchemy is spotty, I\u0027m literate in some areas and weak in others.  Can you just say \"for binding in query\"?","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":285,"context_line":"                      r_port.port_id \u003d\u003d models_v2.Port.id,"},{"line_number":286,"context_line":"                      models_v2.Port.network_id \u003d\u003d network_binding.network_id)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_eeb16190","line":288,"range":{"start_line":288,"start_character":63,"end_line":288,"end_character":69},"in_reply_to":"5a5ae5dd_e4624f16","updated":"2016-02-04 20:26:43.000000000","message":"Yes, that works.  .all() is not necessary to iterate the result of a query.  In fact, it just creates a new data structure in memory unnecessarily.  Not that that\u0027s a big problem.  We have just made a habit of calling this out in reviews.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":292,"context_line":"            query \u003d context.session.query("},{"line_number":293,"context_line":"                                      BgpSpeakerNetworkBinding.bgp_speaker_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_117ecfe4","line":290,"range":{"start_line":290,"start_character":8,"end_line":290,"end_character":47},"updated":"2016-02-03 05:17:09.000000000","message":"This is still never used anywhere.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":292,"context_line":"            query \u003d context.session.query("},{"line_number":293,"context_line":"                                      BgpSpeakerNetworkBinding.bgp_speaker_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_e42a4fe7","line":290,"range":{"start_line":290,"start_character":8,"end_line":290,"end_character":47},"in_reply_to":"5a5ae5dd_117ecfe4","updated":"2016-02-03 05:43:08.000000000","message":"It gets used by the notifications and callbacks patch.  We can move it there.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":287,"context_line":""},{"line_number":288,"context_line":"            return [binding.bgp_speaker_id for binding in query.all()]"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _get_bgp_speaker_ids_by_binding_network(self, context, network_id):"},{"line_number":291,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":292,"context_line":"            query \u003d context.session.query("},{"line_number":293,"context_line":"                                      BgpSpeakerNetworkBinding.bgp_speaker_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_aeab691c","line":290,"range":{"start_line":290,"start_character":8,"end_line":290,"end_character":47},"in_reply_to":"5a5ae5dd_e42a4fe7","updated":"2016-02-04 20:26:43.000000000","message":"++","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":458,"context_line":"            fip_routes \u003d self._get_fip_host_routes_by_bgp_speaker("},{"line_number":459,"context_line":"                                                               context,"},{"line_number":460,"context_line":"                                                               bgp_speaker_id)"},{"line_number":461,"context_line":"            return itertools.chain(fip_routes, net_routes)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    def get_routes_by_bgp_speaker_binding(self, context,"},{"line_number":464,"context_line":"                                          bgp_speaker_id, network_id):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_e4732fb3","line":461,"range":{"start_line":461,"start_character":19,"end_line":461,"end_character":58},"updated":"2016-02-03 05:17:09.000000000","message":"This looks okay in this context but you might want to be conscious of the difference between returning an iterator from a \u0027get\u0027 function and returning an actual container like a list.  It can be surprising, mostly because an iterator can only be iterated once.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":458,"context_line":"            fip_routes \u003d self._get_fip_host_routes_by_bgp_speaker("},{"line_number":459,"context_line":"                                                               context,"},{"line_number":460,"context_line":"                                                               bgp_speaker_id)"},{"line_number":461,"context_line":"            return itertools.chain(fip_routes, net_routes)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    def get_routes_by_bgp_speaker_binding(self, context,"},{"line_number":464,"context_line":"                                          bgp_speaker_id, network_id):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_79069dcf","line":461,"range":{"start_line":461,"start_character":19,"end_line":461,"end_character":58},"in_reply_to":"5a5ae5dd_6402df40","updated":"2016-02-04 20:26:43.000000000","message":"Right, I think you get it.  The most important thing is to represent accurately what you\u0027re returning in the API.  If you feel you need an iterator, that is fine, just make sure it is reflected in the name of the method.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":458,"context_line":"            fip_routes \u003d self._get_fip_host_routes_by_bgp_speaker("},{"line_number":459,"context_line":"                                                               context,"},{"line_number":460,"context_line":"                                                               bgp_speaker_id)"},{"line_number":461,"context_line":"            return itertools.chain(fip_routes, net_routes)"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"    def get_routes_by_bgp_speaker_binding(self, context,"},{"line_number":464,"context_line":"                                          bgp_speaker_id, network_id):"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_6402df40","line":461,"range":{"start_line":461,"start_character":19,"end_line":461,"end_character":58},"in_reply_to":"5a5ae5dd_e4732fb3","updated":"2016-02-03 05:43:08.000000000","message":"Right, I\u0027ve been meaning to come back to this.  Thanks for the reminder.  Because we could potentially return a lot of data an iterator has a lot of upside.  Conversely, from an API perspective I agree it can be a little surprising which is never good.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":494,"context_line":"        \"\"\"Get floating IP host routes with the given router as nexthop.\"\"\""},{"line_number":495,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":496,"context_line":"            dest_alias \u003d aliased(l3_db.FloatingIP,"},{"line_number":497,"context_line":"                                 name\u003d\u0027destination\u0027)"},{"line_number":498,"context_line":"            next_hop_alias \u003d aliased(models_v2.IPAllocation,"},{"line_number":499,"context_line":"                                     name\u003d\u0027next_hop\u0027)"},{"line_number":500,"context_line":"            binding_alias \u003d aliased(BgpSpeakerNetworkBinding,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_b1433b6c","line":497,"range":{"start_line":497,"start_character":33,"end_line":497,"end_character":37},"updated":"2016-02-03 05:17:09.000000000","message":"In this application of \"aliased\" you might get some benefit by setting the name here but I don\u0027t see it.  Is there some benefit to doing it like this?\n\nIs it to make the underlying sql query more readable when you look at it?\n\nIt don\u0027t see any of the code making use of the name.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":494,"context_line":"        \"\"\"Get floating IP host routes with the given router as nexthop.\"\"\""},{"line_number":495,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":496,"context_line":"            dest_alias \u003d aliased(l3_db.FloatingIP,"},{"line_number":497,"context_line":"                                 name\u003d\u0027destination\u0027)"},{"line_number":498,"context_line":"            next_hop_alias \u003d aliased(models_v2.IPAllocation,"},{"line_number":499,"context_line":"                                     name\u003d\u0027next_hop\u0027)"},{"line_number":500,"context_line":"            binding_alias \u003d aliased(BgpSpeakerNetworkBinding,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_a49e9705","line":497,"range":{"start_line":497,"start_character":33,"end_line":497,"end_character":37},"in_reply_to":"5a5ae5dd_b1433b6c","updated":"2016-02-03 05:43:08.000000000","message":"Again, functionally gratuitous.  More for readability and shrinking the size of the line width to conform to pep8.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":494,"context_line":"        \"\"\"Get floating IP host routes with the given router as nexthop.\"\"\""},{"line_number":495,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":496,"context_line":"            dest_alias \u003d aliased(l3_db.FloatingIP,"},{"line_number":497,"context_line":"                                 name\u003d\u0027destination\u0027)"},{"line_number":498,"context_line":"            next_hop_alias \u003d aliased(models_v2.IPAllocation,"},{"line_number":499,"context_line":"                                     name\u003d\u0027next_hop\u0027)"},{"line_number":500,"context_line":"            binding_alias \u003d aliased(BgpSpeakerNetworkBinding,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_244e27c2","line":497,"range":{"start_line":497,"start_character":33,"end_line":497,"end_character":37},"in_reply_to":"5a5ae5dd_b1433b6c","updated":"2016-02-03 05:43:08.000000000","message":"Trying to help with readability and line-width constraints.  Again, functionally gratuitous.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7158148d19823ed50ccffbe92bbc8c001b46af3","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":507,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":508,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":509,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"},{"line_number":510,"context_line":"                binding_alias.network_id \u003d\u003d models_v2.Subnet.network_id,"},{"line_number":511,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d bgp_speaker_id,"},{"line_number":512,"context_line":"                binding_alias.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_a47bf77a","line":509,"range":{"start_line":509,"start_character":47,"end_line":509,"end_character":48},"updated":"2016-02-03 05:17:09.000000000","message":"Should probably use the constant for this before Brian looks at it.  :)","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"517d9024578a84345b12b1c5b5eca3b8ac1a92e7","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":507,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":508,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":509,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"},{"line_number":510,"context_line":"                binding_alias.network_id \u003d\u003d models_v2.Subnet.network_id,"},{"line_number":511,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d bgp_speaker_id,"},{"line_number":512,"context_line":"                binding_alias.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_99511911","line":509,"range":{"start_line":509,"start_character":47,"end_line":509,"end_character":48},"in_reply_to":"5a5ae5dd_64897fca","updated":"2016-02-04 20:26:43.000000000","message":"Maybe its Sean that we should be afraid of, I can\u0027t remember.\n\nI\u0027m just kidding anyway, I too like proper use of constants.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":507,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":508,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":509,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"},{"line_number":510,"context_line":"                binding_alias.network_id \u003d\u003d models_v2.Subnet.network_id,"},{"line_number":511,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d bgp_speaker_id,"},{"line_number":512,"context_line":"                binding_alias.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_444b23b1","line":509,"range":{"start_line":509,"start_character":47,"end_line":509,"end_character":48},"in_reply_to":"5a5ae5dd_a47bf77a","updated":"2016-02-03 05:43:08.000000000","message":"Done","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"780b974c45f13eb149b8853c8bc859cd9fefa5b3","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":507,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":508,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":509,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"},{"line_number":510,"context_line":"                binding_alias.network_id \u003d\u003d models_v2.Subnet.network_id,"},{"line_number":511,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d bgp_speaker_id,"},{"line_number":512,"context_line":"                binding_alias.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_64897fca","line":509,"range":{"start_line":509,"start_character":47,"end_line":509,"end_character":48},"in_reply_to":"5a5ae5dd_a47bf77a","updated":"2016-02-03 05:43:08.000000000","message":"Done","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"8d58cec6f733406ca9f4385125f8e159b653e8be","unresolved":false,"context_lines":[{"line_number":834,"context_line":"                raise bgp_ext.NetworkNotBound(network_id\u003dnetwork_id)"},{"line_number":835,"context_line":""},{"line_number":836,"context_line":"    def _bgp_speaker_for_gateway_network(self, context,"},{"line_number":837,"context_line":"                                         network_id, ip_version):"},{"line_number":838,"context_line":"        \"\"\"Return the BgpSpeaker by given gateway network and ip_version\"\"\""},{"line_number":839,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":840,"context_line":"            query \u003d context.session.query(BgpSpeaker)"}],"source_content_type":"text/x-python","patch_set":20,"id":"5a5ae5dd_85af3929","line":837,"range":{"start_line":837,"start_character":63,"end_line":837,"end_character":65},"updated":"2016-02-05 19:21:00.000000000","message":"Re-work and rename this method.  We can have multiple bindings per address family on a network. Return query.all() here to handle that case appropriately. Rename the method to reflect the fact that more than 1 bgp_speaker can be returned.","commit_id":"835b7c328233bab8256cb892c7fe511f7b27d33d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"024bc55e9aeb54251ac0e0253bf064999e04ff80","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                dest_alias.floating_network_id \u003d\u003d network_id,"},{"line_number":546,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":547,"context_line":"                l3_db.Router.id \u003d\u003d router_attrs.router_id,"},{"line_number":548,"context_line":"                router_attrs.distributed \u003d\u003d sa.sql.false(),"},{"line_number":549,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":550,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":551,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":31,"id":"7af24918_7df5da81","line":548,"range":{"start_line":548,"start_character":16,"end_line":548,"end_character":40},"updated":"2016-03-01 17:30:03.000000000","message":"Even though this attribute is non-nullable in the model, could a join result in a null value here?  To be safe, do this:\n\n  router_attrs.distributed !\u003d sa.sql.true()","commit_id":"0ccb48dc787391a7ecaa258fae0b676461187dcc"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"28429ebe9cb9850557388fa132caa9044a949af1","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                dest_alias.floating_network_id \u003d\u003d network_id,"},{"line_number":546,"context_line":"                dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":547,"context_line":"                l3_db.Router.id \u003d\u003d router_attrs.router_id,"},{"line_number":548,"context_line":"                router_attrs.distributed \u003d\u003d sa.sql.false(),"},{"line_number":549,"context_line":"                l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":550,"context_line":"                next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":551,"context_line":"                models_v2.Subnet.ip_version \u003d\u003d 4,"}],"source_content_type":"text/x-python","patch_set":31,"id":"7af24918_9187ea0e","line":548,"range":{"start_line":548,"start_character":16,"end_line":548,"end_character":40},"in_reply_to":"7af24918_7df5da81","updated":"2016-03-01 19:09:41.000000000","message":"Will introduce an outer join instead on router_attrs. Should provide the defensive approach you\u0027re suggesting.","commit_id":"0ccb48dc787391a7ecaa258fae0b676461187dcc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"024bc55e9aeb54251ac0e0253bf064999e04ff80","unresolved":false,"context_lines":[{"line_number":577,"context_line":"                 next_hop_alias.network_id \u003d\u003d speaker_binding.network_id,"},{"line_number":578,"context_line":"                 dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":579,"context_line":"                 l3_db.Router.id \u003d\u003d router_attrs.router_id,"},{"line_number":580,"context_line":"                 router_attrs.distributed \u003d\u003d sa.sql.false(),"},{"line_number":581,"context_line":"                 l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":582,"context_line":"                 next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":583,"context_line":"                 models_v2.Subnet.ip_version \u003d\u003d 4)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7af24918_1dddb614","line":580,"range":{"start_line":580,"start_character":17,"end_line":580,"end_character":41},"updated":"2016-03-01 17:30:03.000000000","message":"ditto (L548)","commit_id":"0ccb48dc787391a7ecaa258fae0b676461187dcc"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"28429ebe9cb9850557388fa132caa9044a949af1","unresolved":false,"context_lines":[{"line_number":577,"context_line":"                 next_hop_alias.network_id \u003d\u003d speaker_binding.network_id,"},{"line_number":578,"context_line":"                 dest_alias.router_id \u003d\u003d l3_db.Router.id,"},{"line_number":579,"context_line":"                 l3_db.Router.id \u003d\u003d router_attrs.router_id,"},{"line_number":580,"context_line":"                 router_attrs.distributed \u003d\u003d sa.sql.false(),"},{"line_number":581,"context_line":"                 l3_db.Router.gw_port_id \u003d\u003d next_hop_alias.port_id,"},{"line_number":582,"context_line":"                 next_hop_alias.subnet_id \u003d\u003d models_v2.Subnet.id,"},{"line_number":583,"context_line":"                 models_v2.Subnet.ip_version \u003d\u003d 4)"}],"source_content_type":"text/x-python","patch_set":31,"id":"7af24918_d1797231","line":580,"range":{"start_line":580,"start_character":17,"end_line":580,"end_character":41},"in_reply_to":"7af24918_1dddb614","updated":"2016-03-01 19:09:41.000000000","message":"Will introduce an outer join instead on router_attrs. Should provide the defensive approach you\u0027re suggesting.","commit_id":"0ccb48dc787391a7ecaa258fae0b676461187dcc"}],"neutron/extensions/bgp.py":[{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"fbc2d8f378647c494936f18058d80225a05bd426","unresolved":false,"context_lines":[{"line_number":130,"context_line":"                \"BGP speaker %(bgp_speaker_id)s.\")"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"class NetworkNotFound(exceptions.NotFound):"},{"line_number":134,"context_line":"    message \u003d _(\"Network %(network_id)s does not exist.\")"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa69d971_87c47c5f","line":133,"updated":"2016-01-01 15:53:47.000000000","message":"This should be NetworkNotBoundForIpVersion. Rebase issue. Not correctinh this as https://review.openstack.org/#/c/216994/14/neutron/db/bgp_db.py removes it\u0027s usage.","commit_id":"ea70013acf32c2e5cf0a1f5bd57d592741d05300"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"57ff4bdcc6825b334ad975e60cc071ded7d454a8","unresolved":false,"context_lines":[{"line_number":154,"context_line":"    message \u003d _(\"A password must be supplied when using auth_type md5.\")"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"class NetworkNotBoundForIpVersion(NetworkNotBound):"},{"line_number":158,"context_line":"    message \u003d _(\"Network %(network_id)s is not bound to a IPv%(ip_version)s \""},{"line_number":159,"context_line":"                \"BgpSpeaker.\")"},{"line_number":160,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"7af24918_599d3258","line":157,"range":{"start_line":157,"start_character":6,"end_line":157,"end_character":33},"updated":"2016-03-02 21:03:50.000000000","message":"You don\u0027t raise this anywhere.  Nor do you have any except clauses that catch it.","commit_id":"1c9e439e7a1bda5b665997816891111e679faf67"}],"neutron/tests/unit/db/test_db_base_plugin_v2.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"16cc4812cf6743e2a71969a81920be2e3c60a369","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":319,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":320,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":321,"context_line":"        if cidr:"},{"line_number":322,"context_line":"            data[\u0027subnet\u0027][\u0027cidr\u0027] \u003d cidr"},{"line_number":323,"context_line":"        for arg in (\u0027ip_version\u0027, \u0027tenant_id\u0027, \u0027subnetpool_id\u0027, \u0027prefixlen\u0027,"},{"line_number":324,"context_line":"                    \u0027enable_dhcp\u0027, \u0027allocation_pools\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_479005ed","line":321,"updated":"2016-02-03 06:23:03.000000000","message":"are you changing some sort of API? Why is this change necessary? What happens if you pass the cidr unconditionally?","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"e0e583637cf52c693bf3fb146777bf4dd94d2958","unresolved":false,"context_lines":[{"line_number":318,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":319,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":320,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":321,"context_line":"        if cidr:"},{"line_number":322,"context_line":"            data[\u0027subnet\u0027][\u0027cidr\u0027] \u003d cidr"},{"line_number":323,"context_line":"        for arg in (\u0027ip_version\u0027, \u0027tenant_id\u0027, \u0027subnetpool_id\u0027, \u0027prefixlen\u0027,"},{"line_number":324,"context_line":"                    \u0027enable_dhcp\u0027, \u0027allocation_pools\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_6b684cf6","line":321,"in_reply_to":"5a5ae5dd_479005ed","updated":"2016-02-03 16:37:56.000000000","message":"This test fixture didn\u0027t allow you to use it for subnet allocation from a pool.  This change allows you to allocate a subnet from a subnetpool with this fixture. It matters for BGP because address scopes and subnetpools have a bearing on what routes are advertised. As such, we need to setup address scopes and allocate subnets from a pool for unit tests in this patch.\n\nThis is a nice touch that should have been made when we implemented subnet pools, but we need it now.","commit_id":"329ff9ce5650a478a1c60da3a8ededfcdf61cdd6"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3a8599444c18a28d287103749afd73711ca52372","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    def _create_subnet(self, fmt, net_id, cidr,"},{"line_number":319,"context_line":"                       expected_res_status\u003dNone, **kwargs):"},{"line_number":320,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":321,"context_line":"                           \u0027cidr\u0027: cidr,"},{"line_number":322,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":323,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":324,"context_line":"        if cidr:"}],"source_content_type":"text/x-python","patch_set":33,"id":"7af24918_1051a6d8","side":"PARENT","line":321,"updated":"2016-03-03 06:06:25.000000000","message":"this removal looks odd. Care to elaborate? If the cidr is missing, this may only succeed in the presence of a default subnetpool, no?","commit_id":"2799f12901156d9df2a056b62d591a0cfded9cbe"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ce45f2b8087d10fc146cabd9a41724e870abf43b","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    def _create_subnet(self, fmt, net_id, cidr,"},{"line_number":319,"context_line":"                       expected_res_status\u003dNone, **kwargs):"},{"line_number":320,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":321,"context_line":"                           \u0027cidr\u0027: cidr,"},{"line_number":322,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":323,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":324,"context_line":"        if cidr:"}],"source_content_type":"text/x-python","patch_set":33,"id":"7af24918_e3da3216","side":"PARENT","line":321,"in_reply_to":"7af24918_09f79559","updated":"2016-03-04 00:20:56.000000000","message":"Ok, I see that now you pass cidr\u003dNone to these tests:\n\ntest_all_routes_by_bgp_speaker_different_tenant_address_scope\ntest__tenant_prefixes_by_router_no_gateway_port\n\nIt would have been cleaner if you had marked cidr\u003dNone on the method signature too, so that you wouldn\u0027t need to specify the cidr at all in the presence of the subnetpool. That said, there\u0027s still something that doesn\u0027t smell right.\n\n/me digs.","commit_id":"2799f12901156d9df2a056b62d591a0cfded9cbe"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"cc13d7d7e1fb7226fa3ad91a383624cf8e621fbb","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    def _create_subnet(self, fmt, net_id, cidr,"},{"line_number":319,"context_line":"                       expected_res_status\u003dNone, **kwargs):"},{"line_number":320,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":321,"context_line":"                           \u0027cidr\u0027: cidr,"},{"line_number":322,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":323,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":324,"context_line":"        if cidr:"}],"source_content_type":"text/x-python","patch_set":33,"id":"7af24918_09f79559","side":"PARENT","line":321,"in_reply_to":"7af24918_1051a6d8","updated":"2016-03-03 17:54:32.000000000","message":"This will succeed if you have a default subnetpool, but it will also succeed if you pass an explicit subnetpool_id.  This change makes it so cidr is not strictly required when you use it, thereby actually enabling this method to perform allocations from a subnetpool.  This really is aligned with the semantics of allocations from a subnetpool.","commit_id":"2799f12901156d9df2a056b62d591a0cfded9cbe"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9f2e70a0257ea6f3c021d6fcfde2602ad4512f4b","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    def _create_subnet(self, fmt, net_id, cidr,"},{"line_number":319,"context_line":"                       expected_res_status\u003dNone, **kwargs):"},{"line_number":320,"context_line":"        data \u003d {\u0027subnet\u0027: {\u0027network_id\u0027: net_id,"},{"line_number":321,"context_line":"                           \u0027cidr\u0027: cidr,"},{"line_number":322,"context_line":"                           \u0027ip_version\u0027: 4,"},{"line_number":323,"context_line":"                           \u0027tenant_id\u0027: self._tenant_id}}"},{"line_number":324,"context_line":"        if cidr:"}],"source_content_type":"text/x-python","patch_set":33,"id":"7af24918_c329f69c","side":"PARENT","line":321,"in_reply_to":"7af24918_e3da3216","updated":"2016-03-04 00:30:32.000000000","message":"ugh:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/tests/unit/db/test_db_base_plugin_v2.py#L593","commit_id":"2799f12901156d9df2a056b62d591a0cfded9cbe"}]}