)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d385ee58f0ab8977f80c3514bdef2ce6ddb924ba","unresolved":false,"context_lines":[{"line_number":12,"context_line":"bindings was not present.  The gw_port_host is more"},{"line_number":13,"context_line":"reliable."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Iac9598eb79f455c4ef3d3243a96bed524e3d2f7c"},{"line_number":16,"context_line":"Closes-bug: #1369721"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa98f980_d65d3b7b","line":15,"updated":"2014-09-17 06:29:54.000000000","message":"Wrong bug number.","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"4ecb7c9fce7275060c1bda755544e33a5f879f01","unresolved":false,"context_lines":[{"line_number":12,"context_line":"bindings was not present.  The gw_port_host is more"},{"line_number":13,"context_line":"reliable."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Iac9598eb79f455c4ef3d3243a96bed524e3d2f7c"},{"line_number":16,"context_line":"Closes-bug: #1369721"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa98f980_4eab6475","line":15,"in_reply_to":"fa98f980_d65d3b7b","updated":"2014-09-17 15:11:00.000000000","message":"yup - wrong bug # in PS1.  The # in PS2 is correct.","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"40f9268792fc11795303ed88ab0cc12a9a959915","unresolved":false,"context_lines":[{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path.  The change"},{"line_number":11,"context_line":"on the l3_agent addresses a corner case where the ext_port"},{"line_number":12,"context_line":"bindings was not present.  The gw_port_host is more"},{"line_number":13,"context_line":"reliable."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"da9df570_4721494c","line":10,"updated":"2014-09-24 16:08:06.000000000","message":"please split the l3-agent out of this patch","commit_id":"4e85b451c32e12158e24a3e843ad669d1b84dd76"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"f852efd36f031e0d26f00414d54b9c26aaa8f579","unresolved":false,"context_lines":[{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path.  The change"},{"line_number":11,"context_line":"on the l3_agent addresses a corner case where the ext_port"},{"line_number":12,"context_line":"bindings was not present.  The gw_port_host is more"},{"line_number":13,"context_line":"reliable."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"da9df570_623552d9","line":10,"in_reply_to":"da9df570_4721494c","updated":"2014-09-24 21:42:04.000000000","message":"Done","commit_id":"4e85b451c32e12158e24a3e843ad669d1b84dd76"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"be6dc94fc68e6968475d6c91684390b730a2bc65","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michael Smith \u003cmichael.smith6@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-09-24 14:35:52 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da9df570_c8ba9dbf","line":7,"updated":"2014-09-24 23:53:19.000000000","message":"Please consider a commit message that clearly states what this patch is fixing. The bug report says that when moving the router to a different agent a 404 is returned.\n\nThis commit message is pretty vague, and is open to interpretation; does the operation actually completes, but the server ends up in a bad state?\n\nFor instance:\n\nFix NotFound error when migrating a DVR router to a different dvr_snat L3 agent\n\nMoving a router to a different agent is accomplished by the admin with a remove + add operation. For DVR routers the remove operation succeed but it left the snat binding behind. The add operation instead failed because the new binding did not existed... etc\n\nThis patch fix the issue, and allows the admin to successfully move the SNAT part of a DVR router from one dvr_snat agent to another.","commit_id":"e82b4a2f5f77a43cae4c49511bb815d35c7407df"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"77f7add49297f38b67a29aab3329cfe7b9e4b6ca","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michael Smith \u003cmichael.smith6@hp.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-09-24 14:35:52 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da9df570_84dce274","line":7,"in_reply_to":"da9df570_c8ba9dbf","updated":"2014-09-25 22:01:58.000000000","message":"Done","commit_id":"e82b4a2f5f77a43cae4c49511bb815d35c7407df"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"be6dc94fc68e6968475d6c91684390b730a2bc65","unresolved":false,"context_lines":[{"line_number":10,"context_line":"snat bindings in the add/remove router path."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iac9598eb79f455c4ef3d3243a96bed524e3d2f7c"},{"line_number":13,"context_line":"Closes-bug: #1369721"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da9df570_08b585cc","line":13,"updated":"2014-09-24 23:53:19.000000000","message":"this should be partial-bug now","commit_id":"e82b4a2f5f77a43cae4c49511bb815d35c7407df"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"77f7add49297f38b67a29aab3329cfe7b9e4b6ca","unresolved":false,"context_lines":[{"line_number":10,"context_line":"snat bindings in the add/remove router path."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iac9598eb79f455c4ef3d3243a96bed524e3d2f7c"},{"line_number":13,"context_line":"Closes-bug: #1369721"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da9df570_c971a345","line":13,"in_reply_to":"da9df570_08b585cc","updated":"2014-09-25 22:01:58.000000000","message":"Done","commit_id":"e82b4a2f5f77a43cae4c49511bb815d35c7407df"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"b5cbb51a9fbd34806417e593916289db6050b1b9","unresolved":false,"context_lines":[{"line_number":13,"context_line":"updating this table properly for the remove and add"},{"line_number":14,"context_line":"router commands."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"For DVR routers the remove operation succeed but"},{"line_number":17,"context_line":"it left the snat binding behind. The add operation"},{"line_number":18,"context_line":"instead failed because the new binding did not exist."},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"da9df570_171bb873","line":16,"updated":"2014-09-26 11:21:39.000000000","message":"succeed -\u003e succeeded","commit_id":"47d66b3b58b910f5c2d03774da9552525eddd598"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"466b2022c99fedba580bd2ea2336ca40fb28d19c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"updating this table properly for the remove and add"},{"line_number":14,"context_line":"router commands."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"For DVR routers the remove operation succeed but"},{"line_number":17,"context_line":"it left the snat binding behind. The add operation"},{"line_number":18,"context_line":"instead failed because the new binding did not exist."},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"baa201ad_d873a8af","line":16,"updated":"2014-09-29 22:16:31.000000000","message":"Fix the comment from PS-10., succeed --\u003e succeeded","commit_id":"caf2729cf63a30b4e47bf6b0da9fe6f84a96688d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dda8a2b75e063e383ddc3cc6a21ddbecf208a42d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path.  A new"},{"line_number":11,"context_line":"db table was created to map the snat functionality for"},{"line_number":12,"context_line":"a given router to an agent/host.  The plugin was not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"3acd31a7_2339db41","line":9,"updated":"2015-05-04 19:16:18.000000000","message":"nit:  This is worded more like a bug report than a commit message.  I\u0027d rather a commit message describe what is changing rather than what was wrong.  At least, start out with that and if something needs justification, then maybe talk about what was wrong.","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"d3f682ada80fc8e036154dbb5abea892a9a82c9f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The plugin was not correctly binding and unbinding the"},{"line_number":10,"context_line":"snat bindings in the add/remove router path.  A new"},{"line_number":11,"context_line":"db table was created to map the snat functionality for"},{"line_number":12,"context_line":"a given router to an agent/host.  The plugin was not"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":32,"id":"1aca2d91_ea7426d0","line":9,"updated":"2015-05-06 21:56:56.000000000","message":"Ila I think you forgot to address the commit message.\nAs carl mentioned in the previous patch, just edit the commit message to reflect the problem first and say the current patch fixes the issue.","commit_id":"3261e565edef47435994eb6ea9e016f35b207338"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"15136afeb68eb4e532f076d19a606f816e5f81cc","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"manual add/remove router for dvr_snat agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch is to address the failure of manual move of "},{"line_number":10,"context_line":"dvr_snat routers from one service node to another."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The entry in the csnat_l3_agent_bindings table is now removed "}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"9a0c5dc1_a249c1d2","line":9,"updated":"2015-05-12 13:49:54.000000000","message":"nit: some extraneous whitespace at end of several lines.","commit_id":"3ad59451901af7061193e0869373c8cb0c4ca227"}],"neutron/agent/l3_agent.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"40f9268792fc11795303ed88ab0cc12a9a959915","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"                if f[\u0027subnet_id\u0027] \u003d\u003d subnet_id:"},{"line_number":1197,"context_line":"                    return port"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def _check_external_port_exists(self, router_id, port_id):"},{"line_number":1200,"context_line":"        \"\"\"Return True if external gateway port is present.\"\"\""},{"line_number":1201,"context_line":"        return (ip_lib.device_exists("},{"line_number":1202,"context_line":"                    self.get_external_device_name(port_id),"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9df570_e7307da0","line":1199,"updated":"2014-09-24 16:08:06.000000000","message":"this new addition, and the lack of UT coverage, leads me to believe now that changes to l3_agent.py should belong to a dependent patch. Please keep this patch just about the server-side logic.","commit_id":"4e85b451c32e12158e24a3e843ad669d1b84dd76"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"f852efd36f031e0d26f00414d54b9c26aaa8f579","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"                if f[\u0027subnet_id\u0027] \u003d\u003d subnet_id:"},{"line_number":1197,"context_line":"                    return port"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def _check_external_port_exists(self, router_id, port_id):"},{"line_number":1200,"context_line":"        \"\"\"Return True if external gateway port is present.\"\"\""},{"line_number":1201,"context_line":"        return (ip_lib.device_exists("},{"line_number":1202,"context_line":"                    self.get_external_device_name(port_id),"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9df570_c2519e0f","line":1199,"in_reply_to":"da9df570_e7307da0","updated":"2014-09-24 21:42:04.000000000","message":"Done","commit_id":"4e85b451c32e12158e24a3e843ad669d1b84dd76"}],"neutron/api/rpc/agentnotifiers/l3_rpc_agent_api.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            return"},{"line_number":108,"context_line":"        if utils.is_extension_supported("},{"line_number":109,"context_line":"                plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):"},{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            if schedule_routers:"},{"line_number":113,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_81283887","line":110,"updated":"2014-10-24 21:04:19.000000000","message":"nit:  Put this after L112","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"965af8ba9384c8f242146d1e1a48cb7107454ce9","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            if schedule_routers:"},{"line_number":113,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":114,"context_line":"            self._agent_notification("},{"line_number":115,"context_line":"                context, method, router_ids, operation, shuffle_agents)"},{"line_number":116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_170d8ae9","line":113,"updated":"2014-10-23 18:30:05.000000000","message":"in the case of manual snat move, we don\u0027t want to schedule the router again since we are moving it manually.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            if schedule_routers:"},{"line_number":113,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":114,"context_line":"            self._agent_notification("},{"line_number":115,"context_line":"                context, method, router_ids, operation, shuffle_agents)"},{"line_number":116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_a10c5cee","line":113,"in_reply_to":"9aa7fdbe_170d8ae9","updated":"2014-10-24 21:04:19.000000000","message":"Could you refactor a bit move this call to plugin.schedule_routers out of this method entirely and put in to routers_updated?  It was a bit of a surprise to me to see it here in the first place.\n\nIf that were done, \"_notification\" could be renamed to \"notification\" (making it \"public\") and you can call it directly from neutron/db/l3_dvrscheduler_db.py without passing a boolean down to the stack to disable a behavior that probably shouldn\u0027t have been here in the first place.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d3540bee967ede51247aca91ca6c8f5bf5e7e2e8","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            if schedule_routers:"},{"line_number":113,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":114,"context_line":"            self._agent_notification("},{"line_number":115,"context_line":"                context, method, router_ids, operation, shuffle_agents)"},{"line_number":116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_3c1ce169","line":113,"in_reply_to":"9aa7fdbe_9214e8e9","updated":"2014-10-28 04:42:49.000000000","message":"I think we\u0027re on the same page here.  The call to schedule_routers would move to routers_updated so that path would still call it.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"83d49cc455a9a7c3d28fcea5eccab9b9e497b10a","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            if schedule_routers:"},{"line_number":113,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":114,"context_line":"            self._agent_notification("},{"line_number":115,"context_line":"                context, method, router_ids, operation, shuffle_agents)"},{"line_number":116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_9214e8e9","line":113,"in_reply_to":"9aa7fdbe_a10c5cee","updated":"2014-10-27 16:55:54.000000000","message":"I was surprised as well to see schedule_routers() here.  I can refactor as you suggest, but we\u0027d still leave the default path (via routers_updated) to call schedule_routers() right?","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"f867b8174b5e00ce4a272b2ea81b2e3108d87b86","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                        \u0027agents with the message %s\u0027), method)"},{"line_number":105,"context_line":"        return plugin"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def notification(self, context, method, router_ids, operation,"},{"line_number":108,"context_line":"                     shuffle_agents):"},{"line_number":109,"context_line":"        \"\"\"Notify all the agents that are hosting the routers.\"\"\""},{"line_number":110,"context_line":"        plugin \u003d self._get_plugin(method)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_d2e3a4a2","line":107,"updated":"2014-12-15 17:46:46.000000000","message":"I don\u0027t think this should be made a public method.  Having it called elsewhere will make it more difficult to keep track of which methods are being called over this rpc interface.","commit_id":"5e12ee0227f8651c67b4059db238f631244a03b5"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"a201436ed4f779c23cd1a9fe3712f5a2644c5499","unresolved":false,"context_lines":[{"line_number":104,"context_line":"                        \u0027agents with the message %s\u0027), method)"},{"line_number":105,"context_line":"        return plugin"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def notification(self, context, method, router_ids, operation,"},{"line_number":108,"context_line":"                     shuffle_agents):"},{"line_number":109,"context_line":"        \"\"\"Notify all the agents that are hosting the routers.\"\"\""},{"line_number":110,"context_line":"        plugin \u003d self._get_plugin(method)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_36bbf576","line":107,"in_reply_to":"3a961159_d2e3a4a2","updated":"2014-12-15 21:34:54.000000000","message":"This change was requested by another reviewer in PS12.","commit_id":"5e12ee0227f8651c67b4059db238f631244a03b5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            plugin.schedule_routers(adminContext, router_ids)"},{"line_number":113,"context_line":"            if agent_notify:"},{"line_number":114,"context_line":"                self._agent_notification("},{"line_number":115,"context_line":"                    context, method, router_ids,"},{"line_number":116,"context_line":"                    operation, shuffle_agents)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_afc72b11","line":113,"updated":"2015-02-10 16:13:51.000000000","message":"I mean, a notification method where you don\u0027t notify? I don\u0027t think this is at all necessary.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"30c2eb99acbe207620f3a581e8bb168575eda340","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":111,"context_line":"                            context or context.elevated())"},{"line_number":112,"context_line":"            plugin.schedule_routers(adminContext, router_ids)"},{"line_number":113,"context_line":"            if agent_notify:"},{"line_number":114,"context_line":"                self._agent_notification("},{"line_number":115,"context_line":"                    context, method, router_ids,"},{"line_number":116,"context_line":"                    operation, shuffle_agents)"}],"source_content_type":"text/x-python","patch_set":24,"id":"ba7be1f8_fd93a44c","line":113,"in_reply_to":"da86d52c_afc72b11","updated":"2015-02-24 01:17:14.000000000","message":"My bad.  Sorry armax - I need to rework this.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        self._notification_fanout(context, \u0027router_deleted\u0027, router_id)"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    def routers_updated(self, context, router_ids, operation\u003dNone, data\u003dNone,"},{"line_number":140,"context_line":"                        shuffle_agents\u003dFalse, agent_notify\u003dTrue):"},{"line_number":141,"context_line":"        if router_ids:"},{"line_number":142,"context_line":"            self._notification(context, \u0027routers_updated\u0027, router_ids,"},{"line_number":143,"context_line":"                               operation, shuffle_agents, agent_notify)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_4a0e9d33","line":140,"updated":"2015-02-10 16:13:51.000000000","message":"I think we should consider introducing a dvr- related notifier (that includes the methods below, like add_arp_entry, and del_arp_entry), and possibly load the right l3_rpc_notifier depending on the extensions associated to the l3 service plugin.\n\nThis is the best way to specialize the behavior; this flag is a hack.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dda8a2b75e063e383ddc3cc6a21ddbecf208a42d","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):"},{"line_number":113,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":114,"context_line":"                            context or context.elevated())"},{"line_number":115,"context_line":"            if schedule_routers:"},{"line_number":116,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":117,"context_line":"            self._agent_notification("},{"line_number":118,"context_line":"                context, method, router_ids, operation, shuffle_agents)"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_a31ccbc1","line":115,"updated":"2015-05-04 19:16:18.000000000","message":"It doesn\u0027t make sense to me why a _notification would trigger the scheduling of a router.  It is very surprising to find this here.  It makes me think there is something else wrong.  I think I\u0027d like to look a bit deeper.","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"e4ae77ace6b0ec7228f4fdac25464fcb72053ed5","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):"},{"line_number":113,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":114,"context_line":"                            context or context.elevated())"},{"line_number":115,"context_line":"            if schedule_routers:"},{"line_number":116,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":117,"context_line":"            self._agent_notification("},{"line_number":118,"context_line":"                context, method, router_ids, operation, shuffle_agents)"}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_ec556bc6","line":115,"in_reply_to":"3acd31a7_a31ccbc1","updated":"2015-05-06 18:32:21.000000000","message":"Hi carl, yes this schedule_routers was introduced even before Juno.\nSo I think right now the DVR routers heavily depends upon this call for scheduling the SNAT namespace on the service node.\n\nIf we have to remove this, then it should be added to the \"auto_schedule_routers\" function where all the legacy routers are scheduled.\n\nMay be we should address this in a separate patch based on your scheduling changes blueprint that you have submitted.\n\nDoes it makes sense?","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"d85cbb472ec6084c2661efd8c799092b3eb502cc","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):"},{"line_number":113,"context_line":"            adminContext \u003d (context.is_admin and"},{"line_number":114,"context_line":"                            context or context.elevated())"},{"line_number":115,"context_line":"            if schedule_routers:"},{"line_number":116,"context_line":"                plugin.schedule_routers(adminContext, router_ids)"},{"line_number":117,"context_line":"            self._agent_notification("},{"line_number":118,"context_line":"                context, method, router_ids, operation, shuffle_agents)"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_c831f85b","line":115,"in_reply_to":"3acd31a7_a31ccbc1","updated":"2015-05-05 22:54:08.000000000","message":"If i remove plugin.schedule_routers(adminContext, router_ids), then snat name space is not getting created. So, we need to have this line. Looks like this line is there even before dvr functionality","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"}],"neutron/db/l3_agentschedulers_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":153,"context_line":"            raise l3agentscheduler.RouterL3AgentMismatch("},{"line_number":154,"context_line":"                router_type\u003drouter_type, router_id\u003drouter[\u0027id\u0027],"},{"line_number":155,"context_line":"                agent_mode\u003dagent_mode, agent_id\u003dagent[\u0027id\u0027])"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":158,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":159,"context_line":"            not agent[\u0027admin_state_up\u0027] or"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_e910925b","side":"PARENT","line":156,"updated":"2014-09-17 16:15:42.000000000","message":"removing this line seems to be unintended.","commit_id":"c214e6bce59838f0b2f34477dcdac03a04c28c94"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                agent_mode\u003dagent_mode, agent_id\u003dagent[\u0027id\u0027])"},{"line_number":156,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":157,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":158,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":159,"context_line":"            (not self.get_l3_agent_candidates(context, router, [agent]) and"},{"line_number":160,"context_line":"            not self.get_snat_candidates(router, [agent]))"},{"line_number":161,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_e40d17ea","line":158,"updated":"2014-09-17 16:15:42.000000000","message":"looking closer this \"agent[\u0027admin_state_up\u0027]\" is not really needed as it\u0027s been tested in both get_l3_agent_candidates and get_snat_candidates","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"3b5b370d8d2ed05ac98ecaa998d56608bdd88cae","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                agent_mode\u003dagent_mode, agent_id\u003dagent[\u0027id\u0027])"},{"line_number":156,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":157,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":158,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":159,"context_line":"            (not self.get_l3_agent_candidates(context, router, [agent]) and"},{"line_number":160,"context_line":"            not self.get_snat_candidates(router, [agent]))"},{"line_number":161,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_dcb9cb16","line":158,"in_reply_to":"fa98f980_e40d17ea","updated":"2014-09-17 20:19:03.000000000","message":"Done","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":158,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":159,"context_line":"            (not self.get_l3_agent_candidates(context, router, [agent]) and"},{"line_number":160,"context_line":"            not self.get_snat_candidates(router, [agent]))"},{"line_number":161,"context_line":"        )"},{"line_number":162,"context_line":"        if is_wrong_type_or_unsuitable_agent:"},{"line_number":163,"context_line":"            raise l3agentscheduler.InvalidL3Agent(id\u003dagent[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_446d0399","line":160,"updated":"2014-09-17 16:15:42.000000000","message":"this bool condition is becoming a bit convoluted now. I am wondering if we could split this to make it more digestible. For instance, now we could just do:\n\nis_suitable_agent \u003d (\n    self.get_l3_agent_candidates(context, router, [agent]) or    \n    self.get_snat_candidates(router, [agent])\n)\n\nis_wrong_type_or_unsuitable_agent \u003d (\n    agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or\n    not is_suitable_agent\n)","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"3b5b370d8d2ed05ac98ecaa998d56608bdd88cae","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":158,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":159,"context_line":"            (not self.get_l3_agent_candidates(context, router, [agent]) and"},{"line_number":160,"context_line":"            not self.get_snat_candidates(router, [agent]))"},{"line_number":161,"context_line":"        )"},{"line_number":162,"context_line":"        if is_wrong_type_or_unsuitable_agent:"},{"line_number":163,"context_line":"            raise l3agentscheduler.InvalidL3Agent(id\u003dagent[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_fc35afa8","line":160,"in_reply_to":"fa98f980_446d0399","updated":"2014-09-17 20:19:03.000000000","message":"Done","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        \"\"\""},{"line_number":172,"context_line":"        router_id \u003d router[\u0027id\u0027]"},{"line_number":173,"context_line":"        agent_id \u003d agent[\u0027id\u0027]"},{"line_number":174,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":175,"context_line":"            bindings \u003d self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":176,"context_line":"        else:"},{"line_number":177,"context_line":"            query \u003d context.session.query(RouterL3AgentBinding)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_64c20736","line":174,"updated":"2014-09-17 16:15:42.000000000","message":"is it possible to isolate this change so that we avoid polluting this base logic? We punted this before, but I don\u0027t like the idea of keep adding if distributed branches in the logic.\n\nFor instance this could be a method called\n\nget_router_agent_binding that could be overriden in the dvrscheduler class.","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"3b5b370d8d2ed05ac98ecaa998d56608bdd88cae","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        \"\"\""},{"line_number":172,"context_line":"        router_id \u003d router[\u0027id\u0027]"},{"line_number":173,"context_line":"        agent_id \u003d agent[\u0027id\u0027]"},{"line_number":174,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":175,"context_line":"            bindings \u003d self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":176,"context_line":"        else:"},{"line_number":177,"context_line":"            query \u003d context.session.query(RouterL3AgentBinding)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_3ae2da01","line":174,"in_reply_to":"fa98f980_64c20736","updated":"2014-09-17 20:19:03.000000000","message":"I originally implemented these in the dvrscheduler class but I hit issues in UTs.   I can re-try that path, and post what I end up with.","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        \"\"\"Create router to agent binding.\"\"\""},{"line_number":194,"context_line":"        router_id \u003d router[\u0027id\u0027]"},{"line_number":195,"context_line":"        agent_id \u003d agent[\u0027id\u0027]"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":198,"context_line":"                self.bind_snat_router(context, router[\u0027id\u0027], agent)"},{"line_number":199,"context_line":"                self.bind_dvr_router_servicenode(context, router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_44cc2362","line":196,"updated":"2014-09-17 16:15:42.000000000","message":"same consideration here","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7827a8068135c0e54500b5fb76ca067b13a2f069","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        which leads to re-schedule or be added to another agent manually."},{"line_number":231,"context_line":"        \"\"\""},{"line_number":232,"context_line":"        agent \u003d self._get_agent(context, agent_id)"},{"line_number":233,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":234,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":235,"context_line":"            # unbind_snat_servicenode() calls router_removed_from_agent()"},{"line_number":236,"context_line":"            self.unbind_snat_servicenode(context, router[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_a4022f70","line":233,"updated":"2014-09-17 16:15:42.000000000","message":"and here","commit_id":"0ab588777b7ca04628bcf8a91a801d2658438b62"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"17c84dfcbc805f83b4911da86f6ab904504197a7","unresolved":false,"context_lines":[{"line_number":180,"context_line":"        for binding in bindings:"},{"line_number":181,"context_line":"            if binding.l3_agent_id \u003d\u003d agent_id:"},{"line_number":182,"context_line":"                # router already bound to the agent we need"},{"line_number":183,"context_line":"                return False"},{"line_number":184,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":185,"context_line":"            return False"},{"line_number":186,"context_line":"        # non-dvr case: centralized router is already bound to some agent"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_bfbcf907","line":183,"updated":"2014-09-17 18:34:16.000000000","message":"Is this required since you are now handling the distributed cases in a different file.","commit_id":"7e66d0996fec9048fdc665eeec92ae04525264ca"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ea08adac6f310beab9dadd9c843d47492c1e4a69","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":158,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":159,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":160,"context_line":"            not self.get_l3_agent_candidates(context, router, [agent])"},{"line_number":161,"context_line":"        )"},{"line_number":162,"context_line":"        if is_wrong_type_or_unsuitable_agent:"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9df570_947366e6","side":"PARENT","line":159,"updated":"2014-09-24 13:26:57.000000000","message":"I don\u0027t think the clause above should have been removed. Agents administratively down should be considered unsuitable for deployment.","commit_id":"52c967aeebad4ab53d9d8efb2f580b3dae117e51"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"7cffce2b04938a0aa8fd2f7450b2f2c0ec5da187","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":158,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":159,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":160,"context_line":"            not self.get_l3_agent_candidates(context, router, [agent])"},{"line_number":161,"context_line":"        )"},{"line_number":162,"context_line":"        if is_wrong_type_or_unsuitable_agent:"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9df570_3172c7e5","side":"PARENT","line":159,"in_reply_to":"da9df570_947366e6","updated":"2014-09-24 15:29:48.000000000","message":"Good point.  I will update.","commit_id":"52c967aeebad4ab53d9d8efb2f580b3dae117e51"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"5ae2beef14cdc8babe45109777e5251b49b479cf","unresolved":false,"context_lines":[{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":158,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":159,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":160,"context_line":"            not self.get_l3_agent_candidates(context, router, [agent])"},{"line_number":161,"context_line":"        )"},{"line_number":162,"context_line":"        if is_wrong_type_or_unsuitable_agent:"}],"source_content_type":"text/x-python","patch_set":6,"id":"da9df570_c489b727","side":"PARENT","line":159,"in_reply_to":"da9df570_947366e6","updated":"2014-09-24 15:43:24.000000000","message":"Line 428 in get_l3_agent_candidates\n\n    if not l3_agent.admin_state_up:\n         continue\n\nSo here, it looks like it\u0027s redundant, but it does not do any harm if we leave it in.","commit_id":"52c967aeebad4ab53d9d8efb2f580b3dae117e51"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        is_wrong_type_or_unsuitable_agent \u003d ("},{"line_number":172,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":173,"context_line":"            not agent[\u0027admin_state_up\u0027] or"},{"line_number":174,"context_line":"            not self.get_l3_agent_candidates(context, router, [agent])"},{"line_number":175,"context_line":"        )"},{"line_number":176,"context_line":"        if is_wrong_type_or_unsuitable_agent:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_16d3ac04","side":"PARENT","line":173,"updated":"2014-10-24 21:04:19.000000000","message":"(no action, thinking out loud) It seems get_l3_agent_candidates checks admin_state_up so removing this should be okay.","commit_id":"2eef0dc03d4fd6d7363cdb8fc02c4d5d188c1a88"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":176,"context_line":"            not is_suitable_agent"},{"line_number":177,"context_line":"        )"},{"line_number":178,"context_line":"        if is_wrong_type_or_unsuitable_agent:"},{"line_number":179,"context_line":"            raise l3agentscheduler.InvalidL3Agent(id\u003dagent[\u0027id\u0027])"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_f638e83e","line":178,"updated":"2014-10-24 21:04:19.000000000","message":"nit:  You might take this a bit further and decouple the two conditions:\n\n  if is_wrong_type or not is_suitable_agent:","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"83d49cc455a9a7c3d28fcea5eccab9b9e497b10a","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3 or"},{"line_number":176,"context_line":"            not is_suitable_agent"},{"line_number":177,"context_line":"        )"},{"line_number":178,"context_line":"        if is_wrong_type_or_unsuitable_agent:"},{"line_number":179,"context_line":"            raise l3agentscheduler.InvalidL3Agent(id\u003dagent[\u0027id\u0027])"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_72f064c9","line":178,"in_reply_to":"9aa7fdbe_f638e83e","updated":"2014-10-27 16:55:54.000000000","message":"I like your suggestion","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5109b14cae9147f783bd3eec0ed4f490467f9704","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        if self.router_scheduler:"},{"line_number":219,"context_line":"            try:"},{"line_number":220,"context_line":"                if router[\u0027external_gateway_info\u0027] and ("},{"line_number":221,"context_line":"                        router.get(\u0027distributed\u0027)):"},{"line_number":222,"context_line":"                    self.bind_snat_router(context, router[\u0027id\u0027], agent)"},{"line_number":223,"context_line":"                    self.bind_dvr_router_servicenode(context,"},{"line_number":224,"context_line":"                                                     router[\u0027id\u0027], agent)"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_c482e051","line":221,"updated":"2015-01-21 18:34:39.000000000","message":"nit: can we make a single call to \"schedule_snat\", that would internally call the bind_dvr and bind_snat router.","commit_id":"044743fa99e8837224d3f755598e606ec34a2085"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            if binding.l3_agent_id \u003d\u003d agent_id:"},{"line_number":203,"context_line":"                # router already bound to the agent we need"},{"line_number":204,"context_line":"                return False"},{"line_number":205,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":206,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":207,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":208,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_8ff7c758","line":205,"updated":"2015-02-10 16:13:51.000000000","message":"This should be moved elsewhere entirely. It doesn\u0027t belong here at all, as the if router.get(\u0027distributed\u0027) didn\u0027t in the first place.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":14440,"name":"weibo","email":"weibohome@163.com","username":"weibo"},"change_message_id":"30ed45060b5132635d5cac97dc652503981d672b","unresolved":false,"context_lines":[{"line_number":204,"context_line":"                return False"},{"line_number":205,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":206,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":207,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":208,"context_line":"            return False"},{"line_number":209,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":210,"context_line":"        raise l3agentscheduler.RouterHostedByL3Agent("}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_e178c12d","line":207,"updated":"2015-01-31 07:16:50.000000000","message":"this code should relocate before {bindings \u003d query.filter_by(router_id\u003drouter_id).all()}\nbecause if router have binding on this agent?there will return False not return not self.get_snat_bindings(context, router[\u0027id\u0027]) so snat binding will random scheduler and not bind on the agent we specify","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":217,"context_line":"        agent_id \u003d agent[\u0027id\u0027]"},{"line_number":218,"context_line":"        if self.router_scheduler:"},{"line_number":219,"context_line":"            try:"},{"line_number":220,"context_line":"                if router[\u0027external_gateway_info\u0027] and ("},{"line_number":221,"context_line":"                        router.get(\u0027distributed\u0027)):"},{"line_number":222,"context_line":"                    self.bind_snat_router(context, router[\u0027id\u0027], agent)"},{"line_number":223,"context_line":"                    self.bind_dvr_router_servicenode(context,"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_cff1bf6b","line":220,"updated":"2015-02-10 16:13:51.000000000","message":"DVR specific  logic in a base class? This must not be here.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dda8a2b75e063e383ddc3cc6a21ddbecf208a42d","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                agent_id\u003dagent[\u0027id\u0027])"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        is_suitable_agent \u003d ("},{"line_number":157,"context_line":"            agentschedulers_db.services_available(agent[\u0027admin_state_up\u0027]) and"},{"line_number":158,"context_line":"            self.get_l3_agent_candidates(context, router,"},{"line_number":159,"context_line":"                                         [agent],"},{"line_number":160,"context_line":"                                         ignore_admin_state\u003dTrue) or"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_c34fc7bb","line":157,"updated":"2015-05-04 19:16:18.000000000","message":"Is this logic correct?  I personally prefer to put parentheses in to make these kinds of statements clearer even if they don\u0027t change the expression logically.  With parens, this expression is this:\n\n  (services_available and get_l3_agent_candidates) or get_snat_candidates\n\nThis means that if get_snat_candidates is true then the whole statement is true even if services_available is False.  Is this right?  It might surprise some reviewers who were not careful about the order of evaluation of this expression.","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"3e26d4417b961bde3530844d053d926d9289c447","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                agent_id\u003dagent[\u0027id\u0027])"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        is_suitable_agent \u003d ("},{"line_number":157,"context_line":"            agentschedulers_db.services_available(agent[\u0027admin_state_up\u0027]) and"},{"line_number":158,"context_line":"            self.get_l3_agent_candidates(context, router,"},{"line_number":159,"context_line":"                                         [agent],"},{"line_number":160,"context_line":"                                         ignore_admin_state\u003dTrue) or"}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_6a2b7683","line":157,"in_reply_to":"3acd31a7_8800a067","updated":"2015-05-06 21:36:48.000000000","message":"Done","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"d85cbb472ec6084c2661efd8c799092b3eb502cc","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                agent_id\u003dagent[\u0027id\u0027])"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        is_suitable_agent \u003d ("},{"line_number":157,"context_line":"            agentschedulers_db.services_available(agent[\u0027admin_state_up\u0027]) and"},{"line_number":158,"context_line":"            self.get_l3_agent_candidates(context, router,"},{"line_number":159,"context_line":"                                         [agent],"},{"line_number":160,"context_line":"                                         ignore_admin_state\u003dTrue) or"}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_8800a067","line":157,"in_reply_to":"3acd31a7_c34fc7bb","updated":"2015-05-05 22:54:08.000000000","message":"I will change the if condition to \nif (services_available and (get_l3_agent_candidates or get_snat_candidates)","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"03ad0f5189185c1ef34b8383cdaefd15806a5e81","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            if binding.l3_agent_id \u003d\u003d agent_id:"},{"line_number":180,"context_line":"                # router already bound to the agent we need"},{"line_number":181,"context_line":"                return False"},{"line_number":182,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":183,"context_line":"            return False"},{"line_number":184,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":185,"context_line":"        raise l3agentscheduler.RouterHostedByL3Agent("}],"source_content_type":"text/x-python","patch_set":35,"id":"fa32b979_62d2fdd2","side":"PARENT","line":182,"updated":"2015-06-25 03:29:27.000000000","message":"I don\u0027t think it is a good idea to remove this if block. To distributed router, we can have three scenarios in this method: a) no binding found, b) binding found on target agent, c) binding found but not on target agent.\n\nI think this if block is used for case #c, after removing it, this method will return message like \"The router X has been already hosted by the L3 Agent Y.\", but that\u0027s not true.\n\nAnd what\u0027s more, I think we have code to stop manually add/remove dvr router on/from dvr agent, if we don\u0027t have code changed quickly and my memory is fresh.","commit_id":"4bbe791e976f663105884e8eed933681b5949794"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"12925750bdb662036c63e2600e02d73a18b8ec5c","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                                         ignore_admin_state\u003dTrue) or"},{"line_number":162,"context_line":"            self.get_snat_candidates(router, [agent]))"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        is_wrong_type \u003d ("},{"line_number":165,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3"},{"line_number":166,"context_line":"        )"},{"line_number":167,"context_line":"        if is_wrong_type or not is_suitable_agent:"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_ebcca86b","line":164,"updated":"2015-07-09 16:13:41.000000000","message":"this check should be done in the beginning to fail fast","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                                         ignore_admin_state\u003dTrue) or"},{"line_number":162,"context_line":"            self.get_snat_candidates(router, [agent]))"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        is_wrong_type \u003d ("},{"line_number":165,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3"},{"line_number":166,"context_line":"        )"},{"line_number":167,"context_line":"        if is_wrong_type or not is_suitable_agent:"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_deef5085","line":164,"in_reply_to":"ba3cc151_76bfedaa","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"fded7ffe1ae47f8c4ac05d6092f8e5bbe2af7f04","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                                         ignore_admin_state\u003dTrue) or"},{"line_number":162,"context_line":"            self.get_snat_candidates(router, [agent]))"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        is_wrong_type \u003d ("},{"line_number":165,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3"},{"line_number":166,"context_line":"        )"},{"line_number":167,"context_line":"        if is_wrong_type or not is_suitable_agent:"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_76bfedaa","line":164,"in_reply_to":"ba3cc151_e3fcac0e","updated":"2015-07-10 08:00:28.000000000","message":"is_wrong_type - this is something that we can figure out very easily in the beginning and not go through 136-163 for no good reason","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"1ab63fa13a5eb8a19d0b78d3c482fcc24d013de6","unresolved":false,"context_lines":[{"line_number":161,"context_line":"                                         ignore_admin_state\u003dTrue) or"},{"line_number":162,"context_line":"            self.get_snat_candidates(router, [agent]))"},{"line_number":163,"context_line":"        )"},{"line_number":164,"context_line":"        is_wrong_type \u003d ("},{"line_number":165,"context_line":"            agent[\u0027agent_type\u0027] !\u003d constants.AGENT_TYPE_L3"},{"line_number":166,"context_line":"        )"},{"line_number":167,"context_line":"        if is_wrong_type or not is_suitable_agent:"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_e3fcac0e","line":164,"in_reply_to":"ba3cc151_ebcca86b","updated":"2015-07-09 23:40:19.000000000","message":"Oleg, do you want to move the if condition if is_wrong_type or not is_suitable_agent as the first if condition in this method?","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"}],"neutron/db/l3_dvrscheduler_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d5e8cf1d475fe40967ee041cbab7e89b2624f1ea","unresolved":false,"context_lines":[{"line_number":312,"context_line":"                  self).create_router_to_agent_binding("},{"line_number":313,"context_line":"                    context, agent, router)"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":316,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":317,"context_line":"            query \u003d self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":318,"context_line":"            agent_id \u003d agent[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_421fb05b","line":315,"updated":"2014-09-17 19:06:28.000000000","message":"be careful when you override! This method is supposed to return something and you forgot to do so!","commit_id":"7e66d0996fec9048fdc665eeec92ae04525264ca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d5e8cf1d475fe40967ee041cbab7e89b2624f1ea","unresolved":false,"context_lines":[{"line_number":323,"context_line":"                    # router already bound to the agent we need"},{"line_number":324,"context_line":"                    return False"},{"line_number":325,"context_line":"        else:"},{"line_number":326,"context_line":"            super(L3_DVRsch_db_mixin,"},{"line_number":327,"context_line":"                  self).check_agent_router_scheduling_needed("},{"line_number":328,"context_line":"                    context, agent, router)"},{"line_number":329,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa98f980_c232c0f1","line":326,"updated":"2014-09-17 19:06:28.000000000","message":"here you should do return ...","commit_id":"7e66d0996fec9048fdc665eeec92ae04525264ca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7db879e4b90822a64aa756f41dd7bf96b94f9d04","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":316,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":317,"context_line":"            if not self.get_snat_bindings(context, router[\u0027id\u0027]):"},{"line_number":318,"context_line":"                return True"},{"line_number":319,"context_line":"            else:"},{"line_number":320,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_16640a48","line":317,"updated":"2014-09-17 23:11:17.000000000","message":"This is simply:\n\nreturn not self.get_snat_bindings(context, router[\u0027id\u0027])","commit_id":"ac818730bf51e82fabf7043cc9e815af0c47cff4"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"abe35260a58af451a38e6f17926b5bc560e2d86a","unresolved":false,"context_lines":[{"line_number":314,"context_line":""},{"line_number":315,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":316,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":317,"context_line":"            if not self.get_snat_bindings(context, router[\u0027id\u0027]):"},{"line_number":318,"context_line":"                return True"},{"line_number":319,"context_line":"            else:"},{"line_number":320,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_b66de5ce","line":317,"in_reply_to":"fa98f980_16640a48","updated":"2014-09-18 17:48:18.000000000","message":"Done","commit_id":"ac818730bf51e82fabf7043cc9e815af0c47cff4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7db879e4b90822a64aa756f41dd7bf96b94f9d04","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                return True"},{"line_number":319,"context_line":"            else:"},{"line_number":320,"context_line":"                return False"},{"line_number":321,"context_line":"        else:"},{"line_number":322,"context_line":"            return super(L3_DVRsch_db_mixin,"},{"line_number":323,"context_line":"                         self).check_agent_router_scheduling_needed("},{"line_number":324,"context_line":"                            context, agent, router)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_16cb2a1f","line":321,"updated":"2014-09-17 23:11:17.000000000","message":"you can skip the else and move the return one level of indentation back","commit_id":"ac818730bf51e82fabf7043cc9e815af0c47cff4"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"abe35260a58af451a38e6f17926b5bc560e2d86a","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                return True"},{"line_number":319,"context_line":"            else:"},{"line_number":320,"context_line":"                return False"},{"line_number":321,"context_line":"        else:"},{"line_number":322,"context_line":"            return super(L3_DVRsch_db_mixin,"},{"line_number":323,"context_line":"                         self).check_agent_router_scheduling_needed("},{"line_number":324,"context_line":"                            context, agent, router)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa98f980_d670a9a6","line":321,"in_reply_to":"fa98f980_16cb2a1f","updated":"2014-09-18 17:48:18.000000000","message":"Done","commit_id":"ac818730bf51e82fabf7043cc9e815af0c47cff4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        self.bind_snat_router(context, router_id, chosen_snat_agent)"},{"line_number":240,"context_line":"        return chosen_snat_agent"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def unbind_snat_servicenode(self, context, router_id, snat_only\u003dFalse):"},{"line_number":243,"context_line":"        \"\"\"Unbind the snat router to the chosen l3 service agent.\"\"\""},{"line_number":244,"context_line":"        vm_ports \u003d []"},{"line_number":245,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_a10b7c8c","line":242,"updated":"2014-10-24 21:04:19.000000000","message":"Instead of passing a boolean to this method to disable some of the method\u0027s behavior, could you refactor the method so that there are two with more clearly defined and simpler behaviors?\n\n  unbind_snat_sevicenode()\n  unbind_snat_and_router_servicenode()\n\nOr, decouple them entirely:\n\n  unbind_snat_servicenode()\n  unbind_router_servicenode()\n\nI\u0027m not sure which would be better.  Maybe you have a better idea.  It just seems very strange to me to call this (not to mention I hate passing parameters to enable/disable or otherwise control the behavior of a method):\n\n   unbind_snat_servicenode(..., snat_only\u003dFalse)\n\nWhat does it mean?  What else does the method do?  It is unclear without reading through the method what it does in this case.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"83d49cc455a9a7c3d28fcea5eccab9b9e497b10a","unresolved":false,"context_lines":[{"line_number":239,"context_line":"        self.bind_snat_router(context, router_id, chosen_snat_agent)"},{"line_number":240,"context_line":"        return chosen_snat_agent"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def unbind_snat_servicenode(self, context, router_id, snat_only\u003dFalse):"},{"line_number":243,"context_line":"        \"\"\"Unbind the snat router to the chosen l3 service agent.\"\"\""},{"line_number":244,"context_line":"        vm_ports \u003d []"},{"line_number":245,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_32425cae","line":242,"in_reply_to":"9aa7fdbe_a10b7c8c","updated":"2014-10-27 16:55:54.000000000","message":"I like this suggestion - the method does more than is obvious by its name which by itself begs for a refactor.  I\u0027ll take a look at this.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"965af8ba9384c8f242146d1e1a48cb7107454ce9","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                                   l3_agent_id\u003dagent_id)."},{"line_number":278,"context_line":"                         delete(synchronize_session\u003dFalse))"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        if not snat_only:"},{"line_number":281,"context_line":"            self.l3_rpc_notifier.router_removed_from_agent("},{"line_number":282,"context_line":"                context, router_id, host)"},{"line_number":283,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_97f89ac4","line":280,"updated":"2014-10-23 18:30:05.000000000","message":"don\u0027t remove the router entirely, just unbind snat and update the router","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":315,"context_line":"            self.bind_snat_router(context, router[\u0027id\u0027], agent)"},{"line_number":316,"context_line":"            self.bind_dvr_router_servicenode(context, router[\u0027id\u0027], agent)"},{"line_number":317,"context_line":"        else:"},{"line_number":318,"context_line":"            super(L3_DVRsch_db_mixin,"},{"line_number":319,"context_line":"                  self).create_router_to_agent_binding("},{"line_number":320,"context_line":"                    context, agent, router)"},{"line_number":321,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_567ff4aa","line":318,"updated":"2014-10-24 21:04:19.000000000","message":"See L343","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":322,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":323,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":324,"context_line":"            return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":325,"context_line":"        return super(L3_DVRsch_db_mixin,"},{"line_number":326,"context_line":"                     self).check_agent_router_scheduling_needed("},{"line_number":327,"context_line":"                        context, agent, router)"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_96979ceb","line":325,"updated":"2014-10-24 21:04:19.000000000","message":"See L343","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"965af8ba9384c8f242146d1e1a48cb7107454ce9","unresolved":false,"context_lines":[{"line_number":332,"context_line":"              context, agent_id, router_id)"},{"line_number":333,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":334,"context_line":"            self.l3_rpc_notifier.routers_updated(context, [router_id],"},{"line_number":335,"context_line":"                                                 schedule_routers\u003dFalse)"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"},{"line_number":338,"context_line":"        router \u003d self.get_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_d7fea2ad","line":335,"updated":"2014-10-23 18:30:05.000000000","message":"when snat is moved to a new agent, the other agents rules needed to be updated, therefore notify them here","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"29f7c1916b419aaadee128e851e353ed7c381ef3","unresolved":false,"context_lines":[{"line_number":340,"context_line":"            self.unbind_snat_servicenode(context, router[\u0027id\u0027],"},{"line_number":341,"context_line":"                                         snat_only\u003dTrue)"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            super(L3_DVRsch_db_mixin,"},{"line_number":344,"context_line":"                  self).remove_router_from_l3_agent("},{"line_number":345,"context_line":"                    context, agent_id, router_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_36019038","line":343,"updated":"2014-10-24 21:04:19.000000000","message":"I\u0027m not thrilled about methods that only call super conditionally.  It makes the inheritance a bit fragile.  For example, the method may be added to another class in the hierarchy and it may be surprising that the new method may or may not be called depending on some non-obvious reasons like ordering of super calls in python and conditions that exist in another class.\n\nCan you assure me this is the right thing to do here?","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"83d49cc455a9a7c3d28fcea5eccab9b9e497b10a","unresolved":false,"context_lines":[{"line_number":340,"context_line":"            self.unbind_snat_servicenode(context, router[\u0027id\u0027],"},{"line_number":341,"context_line":"                                         snat_only\u003dTrue)"},{"line_number":342,"context_line":"        else:"},{"line_number":343,"context_line":"            super(L3_DVRsch_db_mixin,"},{"line_number":344,"context_line":"                  self).remove_router_from_l3_agent("},{"line_number":345,"context_line":"                    context, agent_id, router_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9aa7fdbe_b2b9eccd","line":343,"in_reply_to":"9aa7fdbe_36019038","updated":"2014-10-27 16:55:54.000000000","message":"I agree this is a bit of a \"brute force\" method I\u0027ve done here.  Let me confirm  not calling the super is required in all cases or see if there is a more elegant approach that can be done.  But in some cases we want to only do what is done here, and not what is done in the super.  But to your point perhaps just modifying the super would be a better approach, essentially limiting the actions in the super for the special cases seen here.","commit_id":"6fc5a78883d250f292f88957c94cd7de1666beb3"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"375c70ce8dae3c6612c20b2074abd05d37fd025f","unresolved":false,"context_lines":[{"line_number":219,"context_line":"        for bind in query:"},{"line_number":220,"context_line":"            if bind.l3_agent_id \u003d\u003d chosen_snat_agent.id:"},{"line_number":221,"context_line":"                LOG.debug(\u0027Distributed Router %(router_id)s already hosted \u0027"},{"line_number":222,"context_line":"                          \u0027on snatunbind_snat l3_agent %(snat_id)s\u0027,"},{"line_number":223,"context_line":"                          {\u0027router_id\u0027: router_id,"},{"line_number":224,"context_line":"                           \u0027snat_id\u0027: chosen_snat_agent.id})"},{"line_number":225,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":13,"id":"9aa7fdbe_a7afb07c","line":222,"updated":"2014-10-29 14:03:16.000000000","message":"snatunbind_snat?","commit_id":"1d7d2a4c5077a03ecb0ea48fa2d4a9cf21303489"},{"author":{"_account_id":10971,"name":"Michael Smith","email":"michael.smith6@hp.com","username":"mrsmith"},"change_message_id":"398a2b0b84edd6dd8b2d526a377fb78b34fa4bad","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":328,"context_line":"            self.l3_rpc_notifier.routers_updated(context, [router_id])"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"},{"line_number":331,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":332,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":333,"context_line":"            if self.unbind_snat(context, router_id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9aa7fdbe_0fa0a5a4","line":330,"updated":"2014-10-28 22:23:01.000000000","message":"Carl - I didn\u0027t alter this one since the \u0027remove\u0027 is fundamentally different for the corner case we are implementing.  Here, only the snat is unbound and the notify has to be a \u0027routers_updated\u0027 not \u0027router_removed\u0027 since the latter would rmove snat and qrouter namespaces.  I could move the logic below to the super, but it would a completely different path in the method.  Let me know your vote.  Thanks.","commit_id":"1d7d2a4c5077a03ecb0ea48fa2d4a9cf21303489"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"ef21b7536325ceb975100be9095dcb412aa6138e","unresolved":false,"context_lines":[{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def unbind_snat_servicenode(self, context, router_id):"},{"line_number":296,"context_line":"        \"\"\"Unbind snat AND the router from the current agent.\"\"\""},{"line_number":297,"context_line":"        binding \u003d self.unbind_snat(context, router_id)"},{"line_number":298,"context_line":"        if binding:"},{"line_number":299,"context_line":"            self.unbind_router_servicenode(context, router_id, binding)"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"5a890539_1506fdcd","line":297,"updated":"2014-12-05 02:35:55.000000000","message":"nit: this naming is confusing. This is not a must. But it would be better if it have been \"unbinded\".","commit_id":"8bc4c86b7aa74bf402c7167be1640a0f88bd90ce"},{"author":{"_account_id":9077,"name":"Rajeev Grover","email":"rgrover687@gmail.com","username":"rajeev"},"change_message_id":"a8958c749beb9da731408474498d219db5751791","unresolved":false,"context_lines":[{"line_number":336,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"},{"line_number":337,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":338,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":339,"context_line":"            if self.unbind_snat(context, router_id):"},{"line_number":340,"context_line":"                self.l3_rpc_notifier.notification(context, \u0027routers_updated\u0027,"},{"line_number":341,"context_line":"                                                  [router_id], None, False)"},{"line_number":342,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":17,"id":"3a961159_a1785df9","line":339,"updated":"2014-12-16 20:30:52.000000000","message":"Should the agent_id be matched too before unbinding or is that validation taken care of by the caller already?","commit_id":"f8c86ff6ba00fc72aaca189cf641c06b79deedd8"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5109b14cae9147f783bd3eec0ed4f490467f9704","unresolved":false,"context_lines":[{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        return binding"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"    def unbind_router_servicenode(self, context, router_id, binding):"},{"line_number":266,"context_line":"        \"\"\"Unbind the router from the chosen l3 service agent.\"\"\""},{"line_number":267,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":268,"context_line":"            port_found \u003d False"}],"source_content_type":"text/x-python","patch_set":23,"id":"1a930d6b_04e94880","line":265,"updated":"2015-01-21 18:34:39.000000000","message":"nit: There are two many unbind_router for the snat, is it possible to have appropriate name and doc strings that would explain the details.","commit_id":"044743fa99e8837224d3f755598e606ec34a2085"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":338,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":339,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":340,"context_line":"            if self.unbind_snat(context, router_id):"},{"line_number":341,"context_line":"                self.l3_rpc_notifier.routers_updated(context,"},{"line_number":342,"context_line":"                                                     \u0027routers_updated\u0027,"},{"line_number":343,"context_line":"                                                     [router_id], None, False)"},{"line_number":344,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_2f77bbd5","line":341,"updated":"2015-02-10 16:13:51.000000000","message":"if the base l3_rpc_notifier does not suit your need, doing something unintuitive like having an l3_rpc_notifier where you can do notify\u003dFalse seems pretty hackish if you ask to me. It should either be possible to redefine the l3_rpc_notifier","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dda8a2b75e063e383ddc3cc6a21ddbecf208a42d","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        binding \u003d None"},{"line_number":256,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":257,"context_line":"            query \u003d (context.session."},{"line_number":258,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_63ba33c2","line":255,"updated":"2015-05-04 19:16:18.000000000","message":"nit:  No need to initialize this here.  To a C/Java programmer the scoping probably looks funny but since binding will always be set as long as no exception is raised, it will always be set before L272.","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"3e26d4417b961bde3530844d053d926d9289c447","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        binding \u003d None"},{"line_number":256,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":257,"context_line":"            query \u003d (context.session."},{"line_number":258,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":31,"id":"1aca2d91_4a20ba9b","line":255,"in_reply_to":"3acd31a7_48d048f6","updated":"2015-05-06 21:36:48.000000000","message":"Done","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"d85cbb472ec6084c2661efd8c799092b3eb502cc","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        binding \u003d None"},{"line_number":256,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":257,"context_line":"            query \u003d (context.session."},{"line_number":258,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":31,"id":"3acd31a7_48d048f6","line":255,"in_reply_to":"3acd31a7_63ba33c2","updated":"2015-05-05 22:54:08.000000000","message":"I will remove L254, binding \u003d None","commit_id":"3422de9ab85e3277360cb69e50b02ea6717536bf"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"10893fa2bf2a18a4ceba6cde070e607c7e8bb529","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        return chosen_snat_agent"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":256,"context_line":"            query \u003d (context.session."},{"line_number":257,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_ad4cd242","line":254,"updated":"2015-05-13 18:33:41.000000000","message":"nit: This fix is not part of this patch. Should be handled in a separate patch.","commit_id":"d2429ced1658d788f65af0d7f6c7e732390d5af4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"03ad0f5189185c1ef34b8383cdaefd15806a5e81","unresolved":false,"context_lines":[{"line_number":346,"context_line":"                                                  active\u003dTrue)"},{"line_number":347,"context_line":"        return self._get_dvr_sync_data(context, host, agent,"},{"line_number":348,"context_line":"                                       router_ids\u003drouter_ids, active\u003dTrue)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":351,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa32b979_a21d6574","line":349,"updated":"2015-06-25 03:29:27.000000000","message":"Is it really necessary to overwrite the following four methods?","commit_id":"d2429ced1658d788f65af0d7f6c7e732390d5af4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"03ad0f5189185c1ef34b8383cdaefd15806a5e81","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    def check_agent_router_scheduling_needed(self, context, agent, router):"},{"line_number":351,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":353,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":354,"context_line":"            return False"},{"line_number":355,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":356,"context_line":"        return super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":35,"id":"fa32b979_35b2f106","line":353,"updated":"2015-06-25 03:29:27.000000000","message":"Shall we also support only using l3-agent-router-add to manually move dvr-snat router, if a dvr-snat router already has a binding on another dvr-snat agent ?","commit_id":"d2429ced1658d788f65af0d7f6c7e732390d5af4"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"3e69cd3d8fc62524faabf7440d6c6bb8a3d3e4c5","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        return chosen_snat_agent"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":256,"context_line":"            query \u003d (context.session."},{"line_number":257,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_562d09a7","line":254,"updated":"2015-07-10 08:29:47.000000000","message":"there is no chosen l3 agent here, this just unbinds from _any_ agent.","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":251,"context_line":"        return chosen_snat_agent"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def unbind_snat(self, context, router_id):"},{"line_number":254,"context_line":"        \"\"\"Unbind snat from the chosen l3 service agent.\"\"\""},{"line_number":255,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":256,"context_line":"            query \u003d (context.session."},{"line_number":257,"context_line":"                     query(CentralizedSnatL3AgentBinding)."}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_1efa9843","line":254,"in_reply_to":"ba3cc151_562d09a7","updated":"2015-07-15 08:33:20.000000000","message":"will add optional agent_id parameter","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"12925750bdb662036c63e2600e02d73a18b8ec5c","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def unbind_snat_servicenode(self, context, router_id):"},{"line_number":308,"context_line":"        \"\"\"Unbind snat AND the router from the current agent.\"\"\""},{"line_number":309,"context_line":"        binding \u003d self.unbind_snat(context, router_id)"},{"line_number":310,"context_line":"        if binding:"},{"line_number":311,"context_line":"            self.unbind_router_servicenode(context, router_id, binding)"},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_861711a2","line":309,"updated":"2015-07-09 16:13:41.000000000","message":"should we start transaction in the beginning?","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"fded7ffe1ae47f8c4ac05d6092f8e5bbe2af7f04","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def unbind_snat_servicenode(self, context, router_id):"},{"line_number":308,"context_line":"        \"\"\"Unbind snat AND the router from the current agent.\"\"\""},{"line_number":309,"context_line":"        binding \u003d self.unbind_snat(context, router_id)"},{"line_number":310,"context_line":"        if binding:"},{"line_number":311,"context_line":"            self.unbind_router_servicenode(context, router_id, binding)"},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_963dd12e","line":309,"in_reply_to":"ba3cc151_236a9482","updated":"2015-07-10 08:00:28.000000000","message":"This is true and these transactions should not be removed. My suggestion is to start main transaction here to cover both unbind_snat and unbind_router_servicenode subtransactions, so if one of them fails everything is rolled back.","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":10076,"name":"Ila Palanisamy","email":"ilavajuthy.palanisamy@hp.com","username":"ilavajuthy_palanisamy"},"change_message_id":"1ab63fa13a5eb8a19d0b78d3c482fcc24d013de6","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def unbind_snat_servicenode(self, context, router_id):"},{"line_number":308,"context_line":"        \"\"\"Unbind snat AND the router from the current agent.\"\"\""},{"line_number":309,"context_line":"        binding \u003d self.unbind_snat(context, router_id)"},{"line_number":310,"context_line":"        if binding:"},{"line_number":311,"context_line":"            self.unbind_router_servicenode(context, router_id, binding)"},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_236a9482","line":309,"in_reply_to":"ba3cc151_861711a2","updated":"2015-07-09 23:40:19.000000000","message":"I feel having transaction in the individual methods is good as they can be called separately","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    def unbind_snat_servicenode(self, context, router_id):"},{"line_number":308,"context_line":"        \"\"\"Unbind snat AND the router from the current agent.\"\"\""},{"line_number":309,"context_line":"        binding \u003d self.unbind_snat(context, router_id)"},{"line_number":310,"context_line":"        if binding:"},{"line_number":311,"context_line":"            self.unbind_router_servicenode(context, router_id, binding)"},{"line_number":312,"context_line":""}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_3ef75c5a","line":309,"in_reply_to":"ba3cc151_963dd12e","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"12925750bdb662036c63e2600e02d73a18b8ec5c","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":353,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":354,"context_line":"            return False"},{"line_number":355,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":356,"context_line":"        return super(L3_DVRsch_db_mixin,"},{"line_number":357,"context_line":"                     self).check_agent_router_scheduling_needed("}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_4645896c","line":354,"updated":"2015-07-09 16:13:41.000000000","message":"should we check for dvr serviceable ports on agent\u0027s host as well?","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        if router.get(\u0027distributed\u0027):"},{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":353,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":354,"context_line":"            return False"},{"line_number":355,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":356,"context_line":"        return super(L3_DVRsch_db_mixin,"},{"line_number":357,"context_line":"                     self).check_agent_router_scheduling_needed("}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_5e0ee03b","line":354,"in_reply_to":"ba3cc151_4645896c","updated":"2015-07-15 08:33:20.000000000","message":"For distributed routers only SNAT portion sheduling makes sense","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"12925750bdb662036c63e2600e02d73a18b8ec5c","unresolved":false,"context_lines":[{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":353,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":354,"context_line":"            return False"},{"line_number":355,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":356,"context_line":"        return super(L3_DVRsch_db_mixin,"},{"line_number":357,"context_line":"                     self).check_agent_router_scheduling_needed("},{"line_number":358,"context_line":"                     context, agent, router)"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_2b429068","line":355,"updated":"2015-07-09 16:13:41.000000000","message":"the comment should be removed","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":352,"context_line":"            if router[\u0027external_gateway_info\u0027]:"},{"line_number":353,"context_line":"                return not self.get_snat_bindings(context, router[\u0027id\u0027])"},{"line_number":354,"context_line":"            return False"},{"line_number":355,"context_line":"        # non-dvr case: centralized router is already bound to some agent"},{"line_number":356,"context_line":"        return super(L3_DVRsch_db_mixin,"},{"line_number":357,"context_line":"                     self).check_agent_router_scheduling_needed("},{"line_number":358,"context_line":"                     context, agent, router)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_bea12c48","line":355,"in_reply_to":"ba3cc151_2b429068","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"3e69cd3d8fc62524faabf7440d6c6bb8a3d3e4c5","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        super(L3_DVRsch_db_mixin, self).add_router_to_l3_agent("},{"line_number":379,"context_line":"              context, agent_id, router_id)"},{"line_number":380,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":381,"context_line":"            self.l3_rpc_notifier.routers_updated(context, [router_id],"},{"line_number":382,"context_line":"                                                 schedule_routers\u003dFalse)"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_317c0feb","line":381,"updated":"2015-07-10 08:29:47.000000000","message":"super() already notifies the agent about added router, why need to notify all agents (chosen agent will receive two notifications)?","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        super(L3_DVRsch_db_mixin, self).add_router_to_l3_agent("},{"line_number":379,"context_line":"              context, agent_id, router_id)"},{"line_number":380,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":381,"context_line":"            self.l3_rpc_notifier.routers_updated(context, [router_id],"},{"line_number":382,"context_line":"                                                 schedule_routers\u003dFalse)"},{"line_number":383,"context_line":""},{"line_number":384,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_d900ca03","line":381,"in_reply_to":"ba3cc151_317c0feb","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"3e69cd3d8fc62524faabf7440d6c6bb8a3d3e4c5","unresolved":false,"context_lines":[{"line_number":384,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"},{"line_number":385,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":386,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":387,"context_line":"            binding \u003d self.unbind_snat(context, router_id)"},{"line_number":388,"context_line":"            if binding:"},{"line_number":389,"context_line":"                notification_not_sent \u003d self.unbind_router_servicenode(context,"},{"line_number":390,"context_line":"                                             router_id, binding)"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_d106031c","line":387,"updated":"2015-07-10 08:29:47.000000000","message":"it was requested to unbing from particular agent_id but instead it unbinds from _any_ agent","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":384,"context_line":"    def remove_router_from_l3_agent(self, context, agent_id, router_id):"},{"line_number":385,"context_line":"        router \u003d self.get_router(context, router_id)"},{"line_number":386,"context_line":"        if router[\u0027external_gateway_info\u0027] and router.get(\u0027distributed\u0027):"},{"line_number":387,"context_line":"            binding \u003d self.unbind_snat(context, router_id)"},{"line_number":388,"context_line":"            if binding:"},{"line_number":389,"context_line":"                notification_not_sent \u003d self.unbind_router_servicenode(context,"},{"line_number":390,"context_line":"                                             router_id, binding)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_9906c21a","line":387,"in_reply_to":"ba3cc151_d106031c","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"3e69cd3d8fc62524faabf7440d6c6bb8a3d3e4c5","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                                             router_id, binding)"},{"line_number":391,"context_line":"                if notification_not_sent:"},{"line_number":392,"context_line":"                    self.l3_rpc_notifier.routers_updated(context,"},{"line_number":393,"context_line":"                                                     [router_id],"},{"line_number":394,"context_line":"                                                     schedule_routers\u003dFalse)"},{"line_number":395,"context_line":"        else:"},{"line_number":396,"context_line":"            super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":38,"id":"ba3cc151_f170e7a6","line":393,"updated":"2015-07-10 08:29:47.000000000","message":"wrong indent","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d4092323dceef2145f3de0805b742569e1263ab8","unresolved":false,"context_lines":[{"line_number":390,"context_line":"                                             router_id, binding)"},{"line_number":391,"context_line":"                if notification_not_sent:"},{"line_number":392,"context_line":"                    self.l3_rpc_notifier.routers_updated(context,"},{"line_number":393,"context_line":"                                                     [router_id],"},{"line_number":394,"context_line":"                                                     schedule_routers\u003dFalse)"},{"line_number":395,"context_line":"        else:"},{"line_number":396,"context_line":"            super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a41bdd9_b90b8623","line":393,"in_reply_to":"ba3cc151_f170e7a6","updated":"2015-07-15 08:33:20.000000000","message":"Done","commit_id":"14ed778ed1683219c049e63fae7a0fbb817bef31"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"c9a0bfff3c0b1675923a312e7dd12a3e4a4ef6df","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                binding \u003d query.one()"},{"line_number":266,"context_line":"            except exc.NoResultFound:"},{"line_number":267,"context_line":"                LOG.debug(\u0027no snat router binding found for router: %(\u0027"},{"line_number":268,"context_line":"                          \u0027router)s, agent: %(agent)s\u0027,"},{"line_number":269,"context_line":"                          {\u0027router\u0027: router_id, \u0027agent\u0027: agent_id or \u0027any\u0027})"},{"line_number":270,"context_line":"                return"},{"line_number":271,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9a41bdd9_900e8739","line":268,"updated":"2015-07-15 13:20:35.000000000","message":"Nit: for readability, could put \u0027%(\u0027 from end of previous line, on this line.","commit_id":"eaf5ff87b7d7f2556e71a6813620ee649758e42b"}],"neutron/plugins/nec/nec_router.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9987f4fa965c7299c901f59392e940d87615165a","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            context.session, nconst.ROUTER_PROVIDER_L3AGENT, router_ids)"},{"line_number":303,"context_line":"        super(L3AgentNotifyAPI, self)._notification("},{"line_number":304,"context_line":"            context, method, router_ids, operation, shuffle_agents,"},{"line_number":305,"context_line":"            agent_notify)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"def load_driver(plugin, ofc_manager):"}],"source_content_type":"text/x-python","patch_set":24,"id":"da86d52c_cf4d3f9b","line":305,"updated":"2015-02-10 16:13:51.000000000","message":"I really do not want to see this change here. It smells bad.","commit_id":"aafdac69b9eadf2c9864ce57810ba1f9acbfe882"}],"neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8c149564abee5feb04e2a48a6f3afd9e41a52a7e","unresolved":false,"context_lines":[{"line_number":1036,"context_line":""},{"line_number":1037,"context_line":"    def _create_dist_router(self):"},{"line_number":1038,"context_line":"        with self.subnet() as s1:"},{"line_number":1039,"context_line":"            self._register_agent_states()"},{"line_number":1040,"context_line":"            self._set_net_external(s1[\u0027subnet\u0027][\u0027network_id\u0027])"},{"line_number":1041,"context_line":"            data \u003d {\u0027router\u0027: {\u0027tenant_id\u0027: uuidutils.generate_uuid()}}"},{"line_number":1042,"context_line":"            data[\u0027router\u0027][\u0027name\u0027] \u003d \u0027distrouter1\u0027"}],"source_content_type":"text/x-python","patch_set":42,"id":"3a50d1a3_dd0db5e9","line":1039,"updated":"2015-07-24 07:17:05.000000000","message":"this creates legacy agents, not supporting DVR","commit_id":"bf8af9fb89ee9a78201577f15de8ea62170cce84"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8c149564abee5feb04e2a48a6f3afd9e41a52a7e","unresolved":false,"context_lines":[{"line_number":1045,"context_line":"            router_req \u003d self.new_create_request(\u0027routers\u0027, data, self.fmt)"},{"line_number":1046,"context_line":"            res \u003d router_req.get_response(self.ext_api)"},{"line_number":1047,"context_line":"            router \u003d self.deserialize(self.fmt, res)"},{"line_number":1048,"context_line":"            router[\u0027distributed\u0027] \u003d True"},{"line_number":1049,"context_line":"        return router"},{"line_number":1050,"context_line":""},{"line_number":1051,"context_line":"    def test_router_remove_dvr_agent(self):"}],"source_content_type":"text/x-python","patch_set":42,"id":"3a50d1a3_1d2b1d35","line":1048,"updated":"2015-07-24 07:17:05.000000000","message":"in db router is legacy, nothing much changes after updating result dict","commit_id":"bf8af9fb89ee9a78201577f15de8ea62170cce84"}],"neutron/tests/unit/plugins/openvswitch/test_agent_scheduler.py":[{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"10893fa2bf2a18a4ceba6cde070e607c7e8bb529","unresolved":false,"context_lines":[{"line_number":1169,"context_line":"        with self.router() as router1:"},{"line_number":1170,"context_line":"            self._register_agent_states()"},{"line_number":1171,"context_line":"            self._verify_router_add_to_l3_agent(router1)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"    def _verify_router_add_to_l3_agent(self, router, admin_state_up\u003dTrue):"},{"line_number":1174,"context_line":"        hosta_id \u003d self._get_agent_id(constants.AGENT_TYPE_L3,"},{"line_number":1175,"context_line":"                                      L3_HOSTA)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_ad0df23f","line":1172,"updated":"2015-05-13 18:33:41.000000000","message":"Is there a possibility to add a test case to make sure the \"add_router\" and \"remove_router\" with the \"external_gateway\" info are calling the right bind and unbind functions that was created in this patch.","commit_id":"d2429ced1658d788f65af0d7f6c7e732390d5af4"}]}