)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ad060dafbcaf44620cefe216b0b9042896d8a7df","unresolved":false,"context_lines":[{"line_number":9,"context_line":"For HA router ports, add only flood flow entries(table 22) and"},{"line_number":10,"context_line":"no unicast flow entries(table 20) in agents."},{"line_number":11,"context_line":"As HA routers use distributed ports, above flows will be created"},{"line_number":12,"context_line":"on all the agents that are hosting the HA router."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"When a HA router created/updated,"},{"line_number":15,"context_line":"1) l2pop fanout notification sent to all agents consists of -"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"da6ed579_dae603b0","line":12,"updated":"2016-01-16 06:07:35.000000000","message":"Can you add some explanation in the commit message about why this is being done? Is it fixing broken behavior, or is it just an optimization?","commit_id":"52c474d9ef6d62cdbd831bb4329bb27b4166f1f5"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f03872dcd3d8df532c4fe5af1e344dbe1adce83f","unresolved":false,"context_lines":[{"line_number":3,"context_line":"AuthorDate: 2016-02-28 08:06:00 +0000"},{"line_number":4,"context_line":"Commit:     venkata anil \u003canilvenkata@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-05-02 07:00:04 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"l2pop fdb flows for HA router ports"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When router interface port is up on master, l2pop creates flood"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"dab17558_3053d818","line":6,"updated":"2016-05-11 12:05:02.000000000","message":"The commit message is confusing, I\u0027d separate the parts of what we have now, and what we get after the patch. \n\nI\u0027d do something like\n\n\nAfter this patch: \n\n  ... ... ....\n\n\nBefore: \n\n   .... ..... .....\n   .... ..... .....\n\n\nSo it\u0027s much more clear how we\u0027re changing the behaviour. The approach makes sense IMO.","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"f9eecaf2db8b8bc2b62dd4a5ebbdcb4e9b476fb7","unresolved":false,"context_lines":[{"line_number":9,"context_line":"All HA agents(active and backup) call update_device_up/down after wiring"},{"line_number":10,"context_line":"the ports. But l2pop driver is called for only active agent as port"},{"line_number":11,"context_line":"binding in DB reflects active agent."},{"line_number":12,"context_line":"Then l2pop creats unicast and multicast flows for active agent."},{"line_number":13,"context_line":"On failover, flows to new active agent is created. For this to happen -"},{"line_number":14,"context_line":"all of database, messaging server, neutron-server and destination L3"},{"line_number":15,"context_line":"agent should be active during failover. But when any of the above"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"9ad45d7e_2b8c4ad9","line":12,"range":{"start_line":12,"start_character":10,"end_line":12,"end_character":16},"updated":"2016-08-10 17:36:50.000000000","message":"creates","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"adf3472d2f62b82fc8a31801f5f5ff2d2e63aa3d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"All HA agents(active and backup) call update_device_up/down after wiring"},{"line_number":10,"context_line":"the ports. But l2pop driver is called for only active agent as port"},{"line_number":11,"context_line":"binding in DB reflects active agent."},{"line_number":12,"context_line":"Then l2pop creats unicast and multicast flows for active agent."},{"line_number":13,"context_line":"On failover, flows to new active agent is created. For this to happen -"},{"line_number":14,"context_line":"all of database, messaging server, neutron-server and destination L3"},{"line_number":15,"context_line":"agent should be active during failover. But when any of the above"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":38,"id":"9ad45d7e_d48ed477","line":12,"range":{"start_line":12,"start_character":10,"end_line":12,"end_character":16},"in_reply_to":"9ad45d7e_2b8c4ad9","updated":"2016-08-11 06:40:59.000000000","message":"ok","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"}],"neutron/db/l3_hamode_db.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"24ac927bb3ef021b7ea56024ba3ff5aa53dd8b17","unresolved":false,"context_lines":[{"line_number":788,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":789,"context_line":""},{"line_number":790,"context_line":""},{"line_number":791,"context_line":"def ha_router_bound_to_host(session, router_id, host):"},{"line_number":792,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":793,"context_line":"                join(agents_db.Agent)."},{"line_number":794,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_4dacbbed","line":791,"range":{"start_line":791,"start_character":4,"end_line":791,"end_character":27},"updated":"2016-08-08 16:13:39.000000000","message":"Please rename to \"is_ha_router_bound_to_host\"","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"7db63129e6951968afd34271df1b617b0a81f983","unresolved":false,"context_lines":[{"line_number":788,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":789,"context_line":""},{"line_number":790,"context_line":""},{"line_number":791,"context_line":"def ha_router_bound_to_host(session, router_id, host):"},{"line_number":792,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":793,"context_line":"                join(agents_db.Agent)."},{"line_number":794,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_e13e37b7","line":791,"range":{"start_line":791,"start_character":4,"end_line":791,"end_character":27},"in_reply_to":"9ad45d7e_4dacbbed","updated":"2016-08-09 07:29:52.000000000","message":"sure, thanks","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"24ac927bb3ef021b7ea56024ba3ff5aa53dd8b17","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                       router_id).first())"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"def ha_router_port(session, port):"},{"line_number":800,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":801,"context_line":"        return True"},{"line_number":802,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_ROUTER_SNAT:"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_ed812f4f","line":799,"range":{"start_line":799,"start_character":4,"end_line":799,"end_character":18},"updated":"2016-08-08 16:13:39.000000000","message":"Please rename to \"is_ha_router_port\".\n\nAlso, I think \u0027is_ha_port\" is more clear and consistent with other functions, such as \"_create_ha_interfaces\" and \"add_ha_port\".","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"715adf0f4b6fba973dc8faeaddeb6bf1a0e1fbc7","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                       router_id).first())"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"def ha_router_port(session, port):"},{"line_number":800,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":801,"context_line":"        return True"},{"line_number":802,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_ROUTER_SNAT:"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_b47c7b73","line":799,"range":{"start_line":799,"start_character":4,"end_line":799,"end_character":18},"in_reply_to":"9ad45d7e_61524715","updated":"2016-08-09 07:34:16.000000000","message":"Probably not then. \"is_ha_router_port\" is good enough in my book :)","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"7db63129e6951968afd34271df1b617b0a81f983","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                       router_id).first())"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"def ha_router_port(session, port):"},{"line_number":800,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":801,"context_line":"        return True"},{"line_number":802,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_ROUTER_SNAT:"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_61524715","line":799,"range":{"start_line":799,"start_character":4,"end_line":799,"end_character":18},"in_reply_to":"9ad45d7e_ed812f4f","updated":"2016-08-09 07:29:52.000000000","message":"In _create_ha_interfaces and add_ha_port, the ports are HA network ports i.e DEVICE_OWNER_ROUTER_HA_INTF. \n\nBut the ports in this function are HA router interface ports i.e\nDEVICE_OWNER_HA_REPLICATED_INT and DEVICE_OWNER_ROUTER_SNAT.\n\nSo, should I modify it to is_ha_port?","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":787,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":788,"context_line":""},{"line_number":789,"context_line":""},{"line_number":790,"context_line":"def is_ha_router_bound_to_host(session, router_id, host):"},{"line_number":791,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":792,"context_line":"                join(agents_db.Agent)."},{"line_number":793,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_83679fe9","line":790,"updated":"2016-08-24 23:37:11.000000000","message":"Can you use neutron/db/l3_agentschedulers_db.py:L3AgentSchedulerDbMixin:list_router_ids_on_host?\n\nAlso, why not query the RouterL3AgentBinding model instead of two models with a join?","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":787,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":788,"context_line":""},{"line_number":789,"context_line":""},{"line_number":790,"context_line":"def is_ha_router_bound_to_host(session, router_id, host):"},{"line_number":791,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":792,"context_line":"                join(agents_db.Agent)."},{"line_number":793,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_980fc087","line":790,"in_reply_to":"1ac06dbe_83679fe9","updated":"2016-08-25 17:30:25.000000000","message":"Thanks. Done in PS46.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":788,"context_line":""},{"line_number":789,"context_line":""},{"line_number":790,"context_line":"def is_ha_router_bound_to_host(session, router_id, host):"},{"line_number":791,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":792,"context_line":"                join(agents_db.Agent)."},{"line_number":793,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"},{"line_number":794,"context_line":"                       L3HARouterAgentPortBinding.router_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_c1a9977d","line":791,"updated":"2016-08-24 19:29:36.000000000","message":"nit: Personally, I\u0027d prefer this query be split out in to a few lines of code with bool() around the last one instead of trying to wrap it all inside a bool().","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":788,"context_line":""},{"line_number":789,"context_line":""},{"line_number":790,"context_line":"def is_ha_router_bound_to_host(session, router_id, host):"},{"line_number":791,"context_line":"    return bool(session.query(L3HARouterAgentPortBinding)."},{"line_number":792,"context_line":"                join(agents_db.Agent)."},{"line_number":793,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"},{"line_number":794,"context_line":"                       L3HARouterAgentPortBinding.router_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_985d007c","line":791,"in_reply_to":"1ac06dbe_c1a9977d","updated":"2016-08-25 17:30:25.000000000","message":"Thanks Carl. Removed this function in PS46 and using list_router_ids_on_hos.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":792,"context_line":"                join(agents_db.Agent)."},{"line_number":793,"context_line":"                filter(agents_db.Agent.host \u003d\u003d host,"},{"line_number":794,"context_line":"                       L3HARouterAgentPortBinding.router_id \u003d\u003d"},{"line_number":795,"context_line":"                       router_id).first())"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"def is_ha_router_port(session, port):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_a1d0dbf8","line":795,"range":{"start_line":795,"start_character":34,"end_line":795,"end_character":41},"updated":"2016-08-24 19:29:36.000000000","message":"nit: I tend to like .limit(1).count(). But, this is pretty much the same efficiency.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                       router_id).first())"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"def is_ha_router_port(session, port):"},{"line_number":799,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":800,"context_line":"        return True"},{"line_number":801,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_639263a1","line":798,"updated":"2016-08-24 23:37:11.000000000","message":"I would highly suggest to send only the information that you need instead of the entire port. Just device_owner and the router_id.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":795,"context_line":"                       router_id).first())"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"def is_ha_router_port(session, port):"},{"line_number":799,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":800,"context_line":"        return True"},{"line_number":801,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_184ab0ac","line":798,"in_reply_to":"1ac06dbe_639263a1","updated":"2016-08-25 17:30:25.000000000","message":"done in PS46.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":799,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":800,"context_line":"        return True"},{"line_number":801,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":802,"context_line":"        return bool(session.query(l3_attrs_db.RouterExtraAttributes)."},{"line_number":803,"context_line":"                    filter_by(ha\u003dTrue)."},{"line_number":804,"context_line":"                    filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":805,"context_line":"                           port[\u0027device_id\u0027]).count())"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_43c927ac","line":802,"updated":"2016-08-24 23:37:11.000000000","message":"Lines 802 to 805 should be extracted to an in-line/internal function called something like \u0027is_ha_router(router_id)\u0027","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":799,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":800,"context_line":"        return True"},{"line_number":801,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":802,"context_line":"        return bool(session.query(l3_attrs_db.RouterExtraAttributes)."},{"line_number":803,"context_line":"                    filter_by(ha\u003dTrue)."},{"line_number":804,"context_line":"                    filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":805,"context_line":"                           port[\u0027device_id\u0027]).count())"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_411007a9","line":802,"updated":"2016-08-24 19:29:36.000000000","message":"nit: ditto (L791).","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":799,"context_line":"    if port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":800,"context_line":"        return True"},{"line_number":801,"context_line":"    elif port[\u0027device_owner\u0027] \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":802,"context_line":"        return bool(session.query(l3_attrs_db.RouterExtraAttributes)."},{"line_number":803,"context_line":"                    filter_by(ha\u003dTrue)."},{"line_number":804,"context_line":"                    filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":805,"context_line":"                           port[\u0027device_id\u0027]).count())"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_b8d1dccf","line":802,"in_reply_to":"1ac06dbe_43c927ac","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":802,"context_line":"        return bool(session.query(l3_attrs_db.RouterExtraAttributes)."},{"line_number":803,"context_line":"                    filter_by(ha\u003dTrue)."},{"line_number":804,"context_line":"                    filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":805,"context_line":"                           port[\u0027device_id\u0027]).count())"},{"line_number":806,"context_line":"    else:"},{"line_number":807,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_a13e1b27","line":805,"range":{"start_line":805,"start_character":46,"end_line":805,"end_character":53},"updated":"2016-08-24 19:29:36.000000000","message":"nit: .limit(1).count() is a good pattern for this. But, this is probably not much less efficient.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":802,"context_line":"        return bool(session.query(l3_attrs_db.RouterExtraAttributes)."},{"line_number":803,"context_line":"                    filter_by(ha\u003dTrue)."},{"line_number":804,"context_line":"                    filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":805,"context_line":"                           port[\u0027device_id\u0027]).count())"},{"line_number":806,"context_line":"    else:"},{"line_number":807,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_18e1109b","line":805,"range":{"start_line":805,"start_character":46,"end_line":805,"end_character":53},"in_reply_to":"1ac06dbe_a13e1b27","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"626fa6fd86c6bb0712a3ea8872c17e20f10ff8b5","unresolved":false,"context_lines":[{"line_number":788,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":789,"context_line":""},{"line_number":790,"context_line":""},{"line_number":791,"context_line":"def is_ha_router_port(device_owner, router_id):"},{"line_number":792,"context_line":"    session \u003d db_api.get_session()"},{"line_number":793,"context_line":"    if device_owner \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":794,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_09b94679","line":791,"updated":"2016-08-29 20:56:38.000000000","message":"This seems to be missing a unit test.","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a0e9762c995a40a18655b61132d82372d5a7951b","unresolved":false,"context_lines":[{"line_number":788,"context_line":"    return cfg.CONF.l3_ha"},{"line_number":789,"context_line":""},{"line_number":790,"context_line":""},{"line_number":791,"context_line":"def is_ha_router_port(device_owner, router_id):"},{"line_number":792,"context_line":"    session \u003d db_api.get_session()"},{"line_number":793,"context_line":"    if device_owner \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":794,"context_line":"        return True"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_cd2cb804","line":791,"in_reply_to":"fa7ab95a_09b94679","updated":"2016-08-30 05:53:15.000000000","message":"I will add","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f33ecddd0e3ded5cf2565344111faf311009843","unresolved":false,"context_lines":[{"line_number":794,"context_line":"    if device_owner \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":795,"context_line":"        return True"},{"line_number":796,"context_line":"    elif device_owner \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":797,"context_line":"        def is_ha_router():"},{"line_number":798,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":799,"context_line":"            query \u003d query.filter_by(ha\u003dTrue)"},{"line_number":800,"context_line":"            query \u003d query.filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_a1fe3f36","line":797,"updated":"2016-08-31 11:10:51.000000000","message":"there doesn\u0027t seem to be a purpose for this being in a closure since you immediately call it","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"553b1716d87e067462dff2934deefd5d366ac6a3","unresolved":false,"context_lines":[{"line_number":794,"context_line":"    if device_owner \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":795,"context_line":"        return True"},{"line_number":796,"context_line":"    elif device_owner \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":797,"context_line":"        def is_ha_router():"},{"line_number":798,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":799,"context_line":"            query \u003d query.filter_by(ha\u003dTrue)"},{"line_number":800,"context_line":"            query \u003d query.filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_f37ba527","line":797,"in_reply_to":"fa7ab95a_a1fe3f36","updated":"2016-08-31 16:53:37.000000000","message":"ok, but got a comment to add a function for this :) so added it.","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"83e173c2bc1d1247666d40fcb76beda5c1b7aa5f","unresolved":false,"context_lines":[{"line_number":794,"context_line":"    if device_owner \u003d\u003d constants.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":795,"context_line":"        return True"},{"line_number":796,"context_line":"    elif device_owner \u003d\u003d constants.DEVICE_OWNER_ROUTER_SNAT:"},{"line_number":797,"context_line":"        def is_ha_router():"},{"line_number":798,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":799,"context_line":"            query \u003d query.filter_by(ha\u003dTrue)"},{"line_number":800,"context_line":"            query \u003d query.filter(l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_da622d16","line":797,"in_reply_to":"fa7ab95a_f37ba527","updated":"2016-08-31 19:00:43.000000000","message":"If you\u0027re referring to my comment [1], this isn\u0027t what I meant. I just meant to do what you did below, splitting the massive multi-line statement in to separate statements\n\n [1] https://review.openstack.org/#/c/255237/45/neutron/db/l3_hamode_db.py@791","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"}],"neutron/plugins/ml2/db.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5f46325e21f873e6fe2576ec0b07c14071229682","unresolved":false,"context_lines":[{"line_number":307,"context_line":"    return bool(context.session.query(models_v2.Subnet)."},{"line_number":308,"context_line":"                enable_eagerloads(False).filter_by(enable_dhcp\u003dTrue)."},{"line_number":309,"context_line":"                filter(models_v2.Subnet.id.in_(subnet_ids)).count())"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"def ha_router_bound_to_host(session, router_id, host):"},{"line_number":313,"context_line":"    return bool(session.query(l3_hamode_db.L3HARouterAgentPortBinding)."}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_4a3ef6da","line":310,"updated":"2016-08-04 12:44:55.000000000","message":"I believe below methods should be in l3_hamode_db rather than in ml2 db","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a481e4e410c00f192286403acc8e6b484b8d74e5","unresolved":false,"context_lines":[{"line_number":307,"context_line":"    return bool(context.session.query(models_v2.Subnet)."},{"line_number":308,"context_line":"                enable_eagerloads(False).filter_by(enable_dhcp\u003dTrue)."},{"line_number":309,"context_line":"                filter(models_v2.Subnet.id.in_(subnet_ids)).count())"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"def ha_router_bound_to_host(session, router_id, host):"},{"line_number":313,"context_line":"    return bool(session.query(l3_hamode_db.L3HARouterAgentPortBinding)."}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_81b3b099","line":310,"in_reply_to":"bacf61ea_4a3ef6da","updated":"2016-08-04 13:45:39.000000000","message":"sure, I will move","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"}],"neutron/plugins/ml2/drivers/l2pop/db.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ef231b1941f1c54a625865913c985ded237e1b97","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            routerport \u003d orm.aliased(l3_db.RouterPort, name\u003d\"routerport\")"},{"line_number":148,"context_line":"            agentbinding \u003d orm.aliased("},{"line_number":149,"context_line":"                l3_hamode_db.L3HARouterAgentPortBinding, name\u003d\"agentbinding\")"},{"line_number":150,"context_line":"            return session.query(routerport.port_id).join(agentbinding,"},{"line_number":151,"context_line":"                routerport.router_id \u003d\u003d agentbinding.router_id).filter("},{"line_number":152,"context_line":"                agentbinding.port_id \u003d\u003d port_id,"},{"line_number":153,"context_line":"                routerport.port_type \u003d\u003d const.DEVICE_OWNER_ROUTER_INTF)"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_segment_info_from_port(self, session, port_id):"},{"line_number":156,"context_line":"        with session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_a143e66b","line":153,"range":{"start_line":150,"start_character":19,"end_line":153,"end_character":71},"updated":"2015-12-14 18:54:24.000000000","message":"Should not it be .all() here?","commit_id":"c323727f1f16b867d95b4bc1ebf394712e24afc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ef231b1941f1c54a625865913c985ded237e1b97","unresolved":false,"context_lines":[{"line_number":159,"context_line":"                segment.network_id \u003d\u003d models_v2.Port.network_id).filter("},{"line_number":160,"context_line":"                models_v2.Port.id \u003d\u003d port_id)"},{"line_number":161,"context_line":"            for s in segments_qry:"},{"line_number":162,"context_line":"                return (s.network_id, s.network_type, s.segmentation_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_5c96a995","line":162,"updated":"2015-12-14 18:54:24.000000000","message":"May be use .first() for query above and just return (segments_qry.id, segments_qry.network_type, segments_qry.segmentation_id)?","commit_id":"c323727f1f16b867d95b4bc1ebf394712e24afc5"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3c34c2f8f2b33c88d3e5bffd4a91ab5ba0999b2a","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":90,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":91,"context_line":"    # Ignore HA router ports"},{"line_number":92,"context_line":"    if len(ha_ports):"},{"line_number":93,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_ports))"},{"line_number":94,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":95,"context_line":"            if get_agent_ip(agent)]"}],"source_content_type":"text/x-python","patch_set":15,"id":"7af24918_4c8760d7","line":92,"range":{"start_line":92,"start_character":7,"end_line":92,"end_character":11},"updated":"2016-03-02 12:04:17.000000000","message":"I don\u0027t understand why you check len() in all places. You can check just \"if ha_ports\" and if ha_ports is empty it will be false.","commit_id":"ec59a140a0da1881827b5a3b6d59980e6b90228b"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"cb54541adf73dc0d9a6c4720a24b4736092c3f25","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":90,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":91,"context_line":"    # Ignore HA router ports"},{"line_number":92,"context_line":"    if len(ha_ports):"},{"line_number":93,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_ports))"},{"line_number":94,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":95,"context_line":"            if get_agent_ip(agent)]"}],"source_content_type":"text/x-python","patch_set":15,"id":"7af24918_4450c95e","line":92,"range":{"start_line":92,"start_character":7,"end_line":92,"end_character":11},"in_reply_to":"7af24918_4c8760d7","updated":"2016-03-02 13:44:46.000000000","message":"Thanks Ann. I will do that.","commit_id":"ec59a140a0da1881827b5a3b6d59980e6b90228b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"4a21aa94045faf01f51fb1ab8b04c7148a7a6946","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":124,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":125,"context_line":"                             const.DEVICE_OWNER_ROUTER_INTF)"},{"line_number":126,"context_line":"        ha_router_ports \u003d [port.id for port in query.all()]"},{"line_number":127,"context_line":"        query \u003d query.join(ml2_models.PortBinding,"},{"line_number":128,"context_line":"                           l3_hamode_db.L3HARouterAgentPortBinding.port_id \u003d\u003d"},{"line_number":129,"context_line":"                           ml2_models.PortBinding.port_id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_22e30da5","line":126,"updated":"2016-03-08 06:02:01.000000000","message":"\u0027.all()\u0027 isn\u0027t necessary","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"4a21aa94045faf01f51fb1ab8b04c7148a7a6946","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                           ml2_models.PortBinding.port_id \u003d\u003d"},{"line_number":168,"context_line":"                           ha_binding.port_id)"},{"line_number":169,"context_line":"        query \u003d query.filter(ha_binding.router_id.in_(router_ids))"},{"line_number":170,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":171,"context_line":"            if get_agent_ip(agent)]"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_82ee6189","line":170,"updated":"2016-03-08 06:02:01.000000000","message":"if you need to respin you can get rid of the \u0027.all()\u0027 here. When you iterate over the query it will execute with an iterator.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ba72b50fe7c7cb54876f035747666ad00658018e","unresolved":false,"context_lines":[{"line_number":174,"context_line":"def get_router_extra_attributes(session, router_id):"},{"line_number":175,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":176,"context_line":"        try:"},{"line_number":177,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":178,"context_line":"            router_attr \u003d query.filter("},{"line_number":179,"context_line":"                l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d router_id"},{"line_number":180,"context_line":"            ).one()"},{"line_number":181,"context_line":"        except exc.NoResultFound:"},{"line_number":182,"context_line":"            raise l3.RouterNotFound(router_id\u003drouter_id)"},{"line_number":183,"context_line":"        return router_attr"}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_d5baa509","line":180,"range":{"start_line":177,"start_character":12,"end_line":180,"end_character":19},"updated":"2016-03-09 09:10:55.000000000","message":"It will be better to move transaction in try block.\n\n  try:\n       with session.begin(subtransactions\u003dTrue):\n           ...\n  except\n\n  return","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"2b9383df9952c11d36f5eb9c57f436e4d921960e","unresolved":false,"context_lines":[{"line_number":174,"context_line":"def get_router_extra_attributes(session, router_id):"},{"line_number":175,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":176,"context_line":"        try:"},{"line_number":177,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":178,"context_line":"            router_attr \u003d query.filter("},{"line_number":179,"context_line":"                l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d router_id"},{"line_number":180,"context_line":"            ).one()"},{"line_number":181,"context_line":"        except exc.NoResultFound:"},{"line_number":182,"context_line":"            raise l3.RouterNotFound(router_id\u003drouter_id)"},{"line_number":183,"context_line":"        return router_attr"}],"source_content_type":"text/x-python","patch_set":17,"id":"3afc51ec_9b5fe440","line":180,"range":{"start_line":177,"start_character":12,"end_line":180,"end_character":19},"in_reply_to":"3afc51ec_8e732780","updated":"2016-03-14 08:36:35.000000000","message":"Yes, this should work properly without transaction.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"b9b5f119db0fb2abd90d83294a19fe60d765c3dc","unresolved":false,"context_lines":[{"line_number":174,"context_line":"def get_router_extra_attributes(session, router_id):"},{"line_number":175,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":176,"context_line":"        try:"},{"line_number":177,"context_line":"            query \u003d session.query(l3_attrs_db.RouterExtraAttributes)"},{"line_number":178,"context_line":"            router_attr \u003d query.filter("},{"line_number":179,"context_line":"                l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d router_id"},{"line_number":180,"context_line":"            ).one()"},{"line_number":181,"context_line":"        except exc.NoResultFound:"},{"line_number":182,"context_line":"            raise l3.RouterNotFound(router_id\u003drouter_id)"},{"line_number":183,"context_line":"        return router_attr"}],"source_content_type":"text/x-python","patch_set":17,"id":"3afc51ec_8e732780","line":180,"range":{"start_line":177,"start_character":12,"end_line":180,"end_character":19},"in_reply_to":"5aef4532_d5baa509","updated":"2016-03-14 07:11:20.000000000","message":"@Ann\nWe are only querying the database, I don\u0027t hink we need a transaction here. Am I correct?","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bb4cd9e1a50094d42880cec0d8353ea9495739fe","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"def get_ha_router_ports(session, network_id):"},{"line_number":79,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":80,"context_line":"        query \u003d session.query(models_v2.Port)"},{"line_number":81,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":82,"context_line":"                           models_v2.Port.device_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_35676eea","line":79,"updated":"2016-03-23 16:03:49.000000000","message":"this is a single query. there is no point in starting a subtransaction","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"0018e145143f70395f2a9b7135443198ea40d007","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"def get_ha_router_ports(session, network_id):"},{"line_number":79,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":80,"context_line":"        query \u003d session.query(models_v2.Port)"},{"line_number":81,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":82,"context_line":"                           models_v2.Port.device_id \u003d\u003d"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_701bb173","line":79,"in_reply_to":"fa0719c6_35676eea","updated":"2016-03-23 16:55:30.000000000","message":"sure, thanks","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bb4cd9e1a50094d42880cec0d8353ea9495739fe","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        return query.all()"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def get_nondvr_active_network_ports(session, network_id, ha_ports):"},{"line_number":91,"context_line":"    query \u003d _get_active_network_ports(session, network_id)"},{"line_number":92,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_9560fad0","line":90,"updated":"2016-03-23 16:03:49.000000000","message":"call the ha_ports params something like \u0027ignored_ports\u0027. from the perspective of this function it hasn\u0027t nothing to do with ha ports","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"0018e145143f70395f2a9b7135443198ea40d007","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        return query.all()"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"def get_nondvr_active_network_ports(session, network_id, ha_ports):"},{"line_number":91,"context_line":"    query \u003d _get_active_network_ports(session, network_id)"},{"line_number":92,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_303c99c7","line":90,"in_reply_to":"fa0719c6_9560fad0","updated":"2016-03-23 16:55:30.000000000","message":"sure, thanks","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bb4cd9e1a50094d42880cec0d8353ea9495739fe","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        query \u003d query.join(ml2_models.PortBinding,"},{"line_number":128,"context_line":"                           l3_hamode_db.L3HARouterAgentPortBinding.port_id \u003d\u003d"},{"line_number":129,"context_line":"                           ml2_models.PortBinding.port_id)"},{"line_number":130,"context_line":"        query3 \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d agent_host,"},{"line_number":131,"context_line":"                              models_v2.Port.status \u003d\u003d"},{"line_number":132,"context_line":"                              const.PORT_STATUS_ACTIVE)"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_755d6614","line":130,"updated":"2016-03-23 16:03:49.000000000","message":"These query var names are a distaster. query query1 query2 query3\n\nName them something more appropriate to their purpose or break the query generation down into independent functions so it\u0027s easier to tell how each of these are being used.","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"0018e145143f70395f2a9b7135443198ea40d007","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        query \u003d query.join(ml2_models.PortBinding,"},{"line_number":128,"context_line":"                           l3_hamode_db.L3HARouterAgentPortBinding.port_id \u003d\u003d"},{"line_number":129,"context_line":"                           ml2_models.PortBinding.port_id)"},{"line_number":130,"context_line":"        query3 \u003d query.filter(ml2_models.PortBinding.host \u003d\u003d agent_host,"},{"line_number":131,"context_line":"                              models_v2.Port.status \u003d\u003d"},{"line_number":132,"context_line":"                              const.PORT_STATUS_ACTIVE)"},{"line_number":133,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_b0754999","line":130,"in_reply_to":"fa0719c6_755d6614","updated":"2016-03-23 16:55:30.000000000","message":"sure, thanks","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bb4cd9e1a50094d42880cec0d8353ea9495739fe","unresolved":false,"context_lines":[{"line_number":157,"context_line":"def get_ha_router_port_bindings(session, router_ids):"},{"line_number":158,"context_line":"    if not router_ids:"},{"line_number":159,"context_line":"        return []"},{"line_number":160,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":161,"context_line":"        ha_binding \u003d orm.aliased("},{"line_number":162,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding, name\u003d\"ha_binding\")"},{"line_number":163,"context_line":"        query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_d59c32c2","line":160,"updated":"2016-03-23 16:03:49.000000000","message":"not worth a subtransaction, this is a single query","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"0018e145143f70395f2a9b7135443198ea40d007","unresolved":false,"context_lines":[{"line_number":157,"context_line":"def get_ha_router_port_bindings(session, router_ids):"},{"line_number":158,"context_line":"    if not router_ids:"},{"line_number":159,"context_line":"        return []"},{"line_number":160,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":161,"context_line":"        ha_binding \u003d orm.aliased("},{"line_number":162,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding, name\u003d\"ha_binding\")"},{"line_number":163,"context_line":"        query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa0719c6_d070bda6","line":160,"in_reply_to":"fa0719c6_d59c32c2","updated":"2016-03-23 16:55:30.000000000","message":"agree","commit_id":"1eb36e5b20f65a20920686b8f1e5a6962f46f637"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"d04304d2e92b511470d0e3d6f2f15cb6b86e54a8","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if get_agent_ip(agent)]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def get_nondistributed_active_port_count(session, agent_host, network_id):"},{"line_number":102,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":103,"context_line":"    query \u003d query.join(ml2_models.PortBinding)"},{"line_number":104,"context_line":"    query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"da0c15f0_520076aa","line":101,"range":{"start_line":101,"start_character":4,"end_line":101,"end_character":40},"updated":"2016-03-29 11:42:28.000000000","message":"There is a lot of code duplication in the following functions. Consider using something like: http://pastebin.com/kew0hdVH\n\nAlso, most of these functions are private (only used in this module), so add a \u0027_\u0027 before their names to indicate there are only used locally (easier for reviewers).","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"61f16133ae645f1025444f2e317efb4c734d7d16","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if get_agent_ip(agent)]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def get_nondistributed_active_port_count(session, agent_host, network_id):"},{"line_number":102,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":103,"context_line":"    query \u003d query.join(ml2_models.PortBinding)"},{"line_number":104,"context_line":"    query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_21e31ba5","line":101,"range":{"start_line":101,"start_character":4,"end_line":101,"end_character":40},"in_reply_to":"ba0121b8_9bd37938","updated":"2016-03-29 15:50:17.000000000","message":"May be it can be taken as a follow up patch. In this change I am splitting get_agent_network_active_port_count into two functions without any re-factoring.\n\nIf I follow the suggestion, the code will look like\ndef _get_active_port_count(session, agent_host, network_id, model):\n    with session.begin(subtransactions\u003dTrue):\n        query \u003d session.query(models_v2.Port)\n        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,                              \n                             model.host \u003d\u003d agent_host)\n        \ndef get_nondistributed_active_port_count(session, agent_host, network_id):\n    query \u003d _get_active_port_count(ml2_models.PortBinding).filter(\n        models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,\n        ~models_v2.Port.device_owner.in_(DISTRIBUTED_INTERFACE_OWNERS))\n    return query.count()\n        \ndef get_dvr_active_port_count(session, agent_host, network_id):\n    query \u003d _get_active_port_count(ml2_models.DVRPortBinding).filter(\n        ml2_models.DVRPortBinding.status \u003d\u003d const.PORT_STATUS_ACTIVE,                                                                     \n        models_v2.Port.device_owner \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE)\n    return query.count()\n\nHere, the code duplication I am avoiding is for two functions get_nondist and get_dvr and avoiding filter for network_id and agent_host.But with the original approach it will be easier for the reviewers to see the diff between the changes and understand it.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"2c1a753f2e53c9bccbcffa257e98b281a583727d","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if get_agent_ip(agent)]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def get_nondistributed_active_port_count(session, agent_host, network_id):"},{"line_number":102,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":103,"context_line":"    query \u003d query.join(ml2_models.PortBinding)"},{"line_number":104,"context_line":"    query \u003d query.filter("}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_9bd37938","line":101,"range":{"start_line":101,"start_character":4,"end_line":101,"end_character":40},"in_reply_to":"da0c15f0_520076aa","updated":"2016-03-29 14:39:17.000000000","message":"sure, thanks","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"d04304d2e92b511470d0e3d6f2f15cb6b86e54a8","unresolved":false,"context_lines":[{"line_number":120,"context_line":"    return query.count()"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"def get_ha_active_port_count(session, agent_host, network_id):"},{"line_number":124,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":125,"context_line":"    query \u003d query.join("},{"line_number":126,"context_line":"        l3_hamode_db.L3HARouterAgentPortBinding,"}],"source_content_type":"text/x-python","patch_set":19,"id":"da0c15f0_72cada6e","line":123,"range":{"start_line":123,"start_character":4,"end_line":123,"end_character":28},"updated":"2016-03-29 11:42:28.000000000","message":"The pastebin I pasted in line 101 can be made to work with this function if you really want it to :) otherwise, it\u0027s also similar to the 2 above functions.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"d04304d2e92b511470d0e3d6f2f15cb6b86e54a8","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    return query.count()"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":142,"context_line":"    return ("},{"line_number":143,"context_line":"        get_nondistributed_active_port_count(session, agent_host, network_id) +"},{"line_number":144,"context_line":"        get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":19,"id":"da0c15f0_7c2c6c02","line":141,"range":{"start_line":141,"start_character":4,"end_line":141,"end_character":39},"updated":"2016-03-29 11:42:28.000000000","message":"Looking at who\u0027s using this function, there are 3 such occurrences in mech_driver.py. What really intrigues me is that 2 just check if the count is 0, and another checks to see if it\u0027s 1.\n\nThis logic seems really heavy for such a simple check. The logic triggers here involves 4 joins and 12 filters - just to see if it\u0027s 0 or 1.\n\nIs there anyway we can \u0027trim the fat\u0027 here?","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3b03a2a4ad6a51f6e285fdc5f20e6a9c394d00d5","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    return query.count()"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":142,"context_line":"    return ("},{"line_number":143,"context_line":"        get_nondistributed_active_port_count(session, agent_host, network_id) +"},{"line_number":144,"context_line":"        get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_4d0488bf","line":141,"range":{"start_line":141,"start_character":4,"end_line":141,"end_character":39},"in_reply_to":"ba0121b8_1bcbe954","updated":"2016-03-29 14:43:56.000000000","message":"Sounds good to me.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"2c1a753f2e53c9bccbcffa257e98b281a583727d","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    return query.count()"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":142,"context_line":"    return ("},{"line_number":143,"context_line":"        get_nondistributed_active_port_count(session, agent_host, network_id) +"},{"line_number":144,"context_line":"        get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_1bcbe954","line":141,"range":{"start_line":141,"start_character":4,"end_line":141,"end_character":39},"in_reply_to":"da0c15f0_7c2c6c02","updated":"2016-03-29 14:39:17.000000000","message":"This logic is important to add/remove tunnels(and flood flows) between agents. May be we can have a follow up patch if we want to make it better.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"cf640c778abe440a85445f4291812132f8dbe846","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_88ca394e","line":144,"updated":"2016-04-15 15:16:19.000000000","message":"If we aren\u0027t going to combine this into a single query, should this at least be done in a transaction? It runs 3 queries returning counts where the db could change between each query.","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"b50979848b9889bcd3f7c8308f0b75b080a0b63f","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_5a640348","line":144,"in_reply_to":"1a122d0e_19163455","updated":"2016-04-19 06:41:49.000000000","message":"done(adding transaction)","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"53c53d16efafc30171fd3682127cd78833994cda","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_89f5e0cc","line":144,"in_reply_to":"1a122d0e_3a021070","updated":"2016-04-18 13:06:24.000000000","message":"@John: I do not think that is correct, at least by default in MySQL Innodb:\n\nhttp://dev.mysql.com/doc/refman/5.7/en/set-transaction.html#isolevel_repeatable-read","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"7bebed7099cfa2309cdcece1a9746994ac458ff1","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_19163455","line":144,"in_reply_to":"1a122d0e_56735be7","updated":"2016-04-18 16:00:52.000000000","message":"I would like to add that it won\u0027t return data that is any more stale than it would be if this was optimized into a single query. I must go now, I have more nuttiness to spread. :p","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"69d46bf5eec21c6d6bd1f2ba6dc98ba6657ca562","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_56735be7","line":144,"in_reply_to":"1a122d0e_56e35b05","updated":"2016-04-18 15:35:07.000000000","message":"Me and Terry had a long talk about this on IRC (today, around 15:00 UTC, see eavesdrop [1]). Adding a transaction here will make the data more consistent at the price of possibly returning stale data, which is IMO safe here since all that is returned is a count (and if this happens concurrently, the later call will have the correct data anyway). Please see the eavesdrop [1] for more thorough discussion on this.\n\n[1]: http://eavesdrop.openstack.org/irclogs/%23openstack-neutron/latest.log.html#t2016-04-18T14:33:31","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"321066b6be5cc7e886dc648d86cb856a5fa35836","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_eb10de50","line":144,"in_reply_to":"1a122d0e_88ca394e","updated":"2016-04-18 06:30:38.000000000","message":"Thanks Terry, I will update the patch with the suggestion.","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8b91f89cbcdfa592e4457ca414e0b6432c69aa29","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_56e35b05","line":144,"in_reply_to":"1a122d0e_89f5e0cc","updated":"2016-04-18 15:27:09.000000000","message":"Maybe add a TODO about optimizing this into a single query, but for now wrapping in a transaction that, at least on systems with repeatable read (which mysql defaults to), will treat it as a single transaction from a data consistency standpoint.","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"9e9ecd94d8a526cc9fa04e841160a73af4808784","unresolved":false,"context_lines":[{"line_number":141,"context_line":""},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"def get_agent_network_active_port_count(session, agent_host, network_id):"},{"line_number":144,"context_line":"    return ("},{"line_number":145,"context_line":"        _get_nondistributed_active_port_count("},{"line_number":146,"context_line":"            session, agent_host, network_id) +"},{"line_number":147,"context_line":"        _get_dvr_active_port_count(session, agent_host, network_id) +"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a122d0e_3a021070","line":144,"in_reply_to":"1a122d0e_eb10de50","updated":"2016-04-18 11:36:56.000000000","message":"@Terry, I\u0027m unclear on what surrounding these 3 calls in a transaction gives us. Since all 3 of these calls are just DB reads they happen immediately (and the commit will actually do nothing afaiu). Can you elaborate on why a transaction is needed here?","commit_id":"245fc280d930ba56904dbcfade677d2ed2c5a511"},{"author":{"_account_id":13770,"name":"Adolfo Duarte","email":"adduarte@redhat.com","username":"adolfoduarte"},"change_message_id":"422d3091164bfc0bd59dd77835e87658d9a6f99d","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DISTRIBUTED_INTERFACE_OWNERS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                                const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a122d0e_7f10914e","line":27,"range":{"start_line":26,"start_character":34,"end_line":27,"end_character":65},"updated":"2016-05-05 19:42:00.000000000","message":"hmm not sure you will need to add \"DEVICE_OWNER_HA_REPLICATED_INT.\" \nTake a look at patch https://review.openstack.org/#/c/307526/\nif that lands interfaces for distributed routers will not be marked as DEVICE_OWNER_HA_REPLICATED_INT","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"04af9be9c9ceb574bf92ab8d1db45955252a5df5","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DISTRIBUTED_INTERFACE_OWNERS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                                const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_a5bbc80d","line":27,"range":{"start_line":26,"start_character":34,"end_line":27,"end_character":65},"in_reply_to":"1a122d0e_7f10914e","updated":"2016-05-06 08:15:24.000000000","message":"If a router is only HA router(and not HADVR), then we need DEVICE_OWNER_HA_REPLICATED_INT.\nIf a router is DVR router(and not HADVR), then we need DEVICE_OWNER_DVR_INTERFACE.\nIf a router is HADVR router, then we need  \n1) only DEVICE_OWNER_HA_REPLICATED_INT according to present u/s code \n2) only DEVICE_OWNER_DVR_INTERFACE according to your change\n https://review.openstack.org/#/c/307526/\nA router port can\u0027t be both DEVICE_OWNER_HA_REPLICATED_INT and DEVICE_OWNER_DVR_INTERFACE\n\nSo DISTRIBUTED_INTERFACE_OWNERS with both the flags is always valid.","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DISTRIBUTED_INTERFACE_OWNERS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                                const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_e7335960","line":27,"range":{"start_line":26,"start_character":34,"end_line":27,"end_character":65},"in_reply_to":"dab17558_a498216b","updated":"2016-05-17 09:36:09.000000000","message":"This change doesn\u0027t take care of DVRHA routers. A separate patch will be followed for DVRHA routers","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DISTRIBUTED_INTERFACE_OWNERS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                                const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_a498216b","line":27,"range":{"start_line":26,"start_character":34,"end_line":27,"end_character":65},"in_reply_to":"dab17558_a5bbc80d","updated":"2016-05-11 14:46:27.000000000","message":"From what I understood the changes are not mutually exclusive or incompatible. may be I\u0027m missing something.","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def _get_nondistributed_active_port_count(session, agent_host, network_id):"},{"line_number":102,"context_line":"    # REVISIT(anilvenkata): avoid code duplication in"},{"line_number":103,"context_line":"    # _get_xxx_active_port_count functions"},{"line_number":104,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":105,"context_line":"    query \u003d query.join(ml2_models.PortBinding)"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_2420b1f6","line":102,"updated":"2016-05-11 14:46:27.000000000","message":"TBH I\u0027m not even sure if there\u0027s a non-overcomplicated way of doing it. What\u0027s the plan? :)","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"def _get_nondistributed_active_port_count(session, agent_host, network_id):"},{"line_number":102,"context_line":"    # REVISIT(anilvenkata): avoid code duplication in"},{"line_number":103,"context_line":"    # _get_xxx_active_port_count functions"},{"line_number":104,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":105,"context_line":"    query \u003d query.join(ml2_models.PortBinding)"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_a7c95128","line":102,"in_reply_to":"dab17558_2420b1f6","updated":"2016-05-17 09:36:09.000000000","message":"Not thinking of refactoring in this patch. It can be done as a separate patch.","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":13770,"name":"Adolfo Duarte","email":"adduarte@redhat.com","username":"adolfoduarte"},"change_message_id":"422d3091164bfc0bd59dd77835e87658d9a6f99d","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        ml2_models.PortBinding.port_id)"},{"line_number":135,"context_line":"    query \u003d query.filter("},{"line_number":136,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":137,"context_line":"        models_v2.Port.device_owner \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":138,"context_line":"        models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":139,"context_line":"        ml2_models.PortBinding.host \u003d\u003d agent_host)"},{"line_number":140,"context_line":"    return query.count()"}],"source_content_type":"text/x-python","patch_set":26,"id":"faac7902_72e538a7","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":76},"updated":"2016-05-05 19:42:00.000000000","message":"This will not catch any ports which are associated with a dvrha routers. again see patch https://review.openstack.org/#/c/307526/","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"04af9be9c9ceb574bf92ab8d1db45955252a5df5","unresolved":false,"context_lines":[{"line_number":134,"context_line":"        ml2_models.PortBinding.port_id)"},{"line_number":135,"context_line":"    query \u003d query.filter("},{"line_number":136,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":137,"context_line":"        models_v2.Port.device_owner \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":138,"context_line":"        models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":139,"context_line":"        ml2_models.PortBinding.host \u003d\u003d agent_host)"},{"line_number":140,"context_line":"    return query.count()"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_25b998ea","line":137,"range":{"start_line":137,"start_character":8,"end_line":137,"end_character":76},"in_reply_to":"faac7902_72e538a7","updated":"2016-05-06 08:15:24.000000000","message":"For DVRHA router interface port, current u/s code uses const.DEVICE_OWNER_HA_REPLICATED_INT. So for HADVR port, get_ha_active_port_count will include this port.\n\nIf https://review.openstack.org/#/c/307526/ goes through, DVRHA router interface port will be represented as DEVICE_OWNER_DVR_INTERFACE and _get_ha_active_port_count will ignore this port, but will be processed by _get_dvr_active_port_count.\n\nSo this change is not breaking u/s code base and is needed for HA only router and HADVR router(for u/s code)","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            _get_ha_active_port_count(session, agent_host, network_id))"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"def get_ha_router_port_bindings(session, router_ids\u003dNone, network_id\u003dNone):"},{"line_number":153,"context_line":"    query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"},{"line_number":154,"context_line":"    query \u003d query.join(agents_db.Agent,"},{"line_number":155,"context_line":"                       agents_db.Agent.host \u003d\u003d ml2_models.PortBinding.host)"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_c40a8525","line":152,"range":{"start_line":152,"start_character":17,"end_line":152,"end_character":18},"updated":"2016-05-11 14:46:27.000000000","message":"nit: a bit of Docstring explaning what will be the result, would be interesting,","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            _get_ha_active_port_count(session, agent_host, network_id))"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"def get_ha_router_port_bindings(session, router_ids\u003dNone, network_id\u003dNone):"},{"line_number":153,"context_line":"    query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"},{"line_number":154,"context_line":"    query \u003d query.join(agents_db.Agent,"},{"line_number":155,"context_line":"                       agents_db.Agent.host \u003d\u003d ml2_models.PortBinding.host)"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_a7f03168","line":152,"range":{"start_line":152,"start_character":17,"end_line":152,"end_character":18},"in_reply_to":"dab17558_c40a8525","updated":"2016-05-17 09:36:09.000000000","message":"done","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":157,"context_line":"                       ml2_models.PortBinding.port_id \u003d\u003d"},{"line_number":158,"context_line":"                       l3_hamode_db.L3HARouterAgentPortBinding.port_id)"},{"line_number":159,"context_line":"    if router_ids:"},{"line_number":160,"context_line":"        query \u003d query.filter("},{"line_number":161,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id.in_("},{"line_number":162,"context_line":"                router_ids))"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_2458b155","line":159,"updated":"2016-05-11 14:46:27.000000000","message":"this indicates that may be you should split that in two functions, one for routers, another for networks. Moving the common parts (query, final filtering..)  to auxiliary functions, of course.\n\nThere\u0027s nothing preventing anyone from calling this with router_ids and network_id","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":156,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":157,"context_line":"                       ml2_models.PortBinding.port_id \u003d\u003d"},{"line_number":158,"context_line":"                       l3_hamode_db.L3HARouterAgentPortBinding.port_id)"},{"line_number":159,"context_line":"    if router_ids:"},{"line_number":160,"context_line":"        query \u003d query.filter("},{"line_number":161,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id.in_("},{"line_number":162,"context_line":"                router_ids))"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_07001d33","line":159,"in_reply_to":"dab17558_2458b155","updated":"2016-05-17 09:36:09.000000000","message":"done","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":13770,"name":"Adolfo Duarte","email":"adduarte@redhat.com","username":"adolfoduarte"},"change_message_id":"422d3091164bfc0bd59dd77835e87658d9a6f99d","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":166,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":167,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":168,"context_line":"                             const.DEVICE_OWNER_HA_REPLICATED_INT)"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        return []"},{"line_number":171,"context_line":"    return [(bind, agent) for bind, agent in query"}],"source_content_type":"text/x-python","patch_set":26,"id":"faac7902_5290f4ec","line":168,"range":{"start_line":168,"start_character":30,"end_line":168,"end_character":66},"updated":"2016-05-05 19:42:00.000000000","message":"Same comment as above","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":166,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":167,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":168,"context_line":"                             const.DEVICE_OWNER_HA_REPLICATED_INT)"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        return []"},{"line_number":171,"context_line":"    return [(bind, agent) for bind, agent in query"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_879a4d12","line":168,"range":{"start_line":168,"start_character":30,"end_line":168,"end_character":66},"in_reply_to":"dab17558_65cba073","updated":"2016-05-17 09:36:09.000000000","message":"A separate patch will be followed for DVRHA routers","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"04af9be9c9ceb574bf92ab8d1db45955252a5df5","unresolved":false,"context_lines":[{"line_number":165,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":166,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":167,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":168,"context_line":"                             const.DEVICE_OWNER_HA_REPLICATED_INT)"},{"line_number":169,"context_line":"    else:"},{"line_number":170,"context_line":"        return []"},{"line_number":171,"context_line":"    return [(bind, agent) for bind, agent in query"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_65cba073","line":168,"range":{"start_line":168,"start_character":30,"end_line":168,"end_character":66},"in_reply_to":"faac7902_5290f4ec","updated":"2016-05-06 08:15:24.000000000","message":"Same comment as above","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5f46325e21f873e6fe2576ec0b07c14071229682","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        return query"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _get_ha_dvr_snat_ports(session, network_id):"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_aa0cd242","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":20},"updated":"2016-08-04 12:44:55.000000000","message":"can\u0027t see filters for dvr and snat in the query","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a481e4e410c00f192286403acc8e6b484b8d74e5","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        return query"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def _get_ha_dvr_snat_ports(session, network_id):"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_21e3e4da","line":81,"range":{"start_line":81,"start_character":12,"end_line":81,"end_character":20},"in_reply_to":"bacf61ea_aa0cd242","updated":"2016-08-04 13:45:39.000000000","message":"I missed that. So this function will return HA distributed router ports i.e\n1) port with device_owner const.DEVICE_OWNER_HA_REPLICATED_INT for only HA router\n2) port with device_owner const.DEVICE_OWNER_ROUTER_SNAT for HA+DVR router\nI will update this patch.","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5f46325e21f873e6fe2576ec0b07c14071229682","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"},{"line_number":85,"context_line":"            l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":86,"context_line":"            models_v2.Port.device_id).filter_by(ha\u003dTrue).filter("},{"line_number":87,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id)"},{"line_number":88,"context_line":"    return [port.id for port in query.all()]"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_7806311d","line":86,"range":{"start_line":86,"start_character":38,"end_line":86,"end_character":56},"updated":"2016-08-04 12:44:55.000000000","message":"will this work given we query models_v2.Port?","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a481e4e410c00f192286403acc8e6b484b8d74e5","unresolved":false,"context_lines":[{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"},{"line_number":85,"context_line":"            l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":86,"context_line":"            models_v2.Port.device_id).filter_by(ha\u003dTrue).filter("},{"line_number":87,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id)"},{"line_number":88,"context_line":"    return [port.id for port in query.all()]"},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_819c500f","line":86,"range":{"start_line":86,"start_character":38,"end_line":86,"end_character":56},"in_reply_to":"bacf61ea_7806311d","updated":"2016-08-04 13:45:39.000000000","message":"Yes. https://review.openstack.org/#/c/255237/30/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py@166\n\ni.e UT test__get_ha_dvr_snat_ports_with_ha_dvr_router","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"5f46325e21f873e6fe2576ec0b07c14071229682","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":113,"context_line":"                             const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":114,"context_line":"    dvr_bindings \u003d [(bind, agent) for bind, agent in query.all()"},{"line_number":115,"context_line":"            if get_agent_ip(agent)]"},{"line_number":116,"context_line":"    ha_bindings \u003d get_ha_distributed_active_network_ports(session, network_id)"},{"line_number":117,"context_line":"    return dvr_bindings + ha_bindings"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_98116555","line":115,"range":{"start_line":115,"start_character":12,"end_line":115,"end_character":35},"updated":"2016-08-04 12:44:55.000000000","message":"please indent under the opening [","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a481e4e410c00f192286403acc8e6b484b8d74e5","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                             models_v2.Port.device_owner \u003d\u003d"},{"line_number":113,"context_line":"                             const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":114,"context_line":"    dvr_bindings \u003d [(bind, agent) for bind, agent in query.all()"},{"line_number":115,"context_line":"            if get_agent_ip(agent)]"},{"line_number":116,"context_line":"    ha_bindings \u003d get_ha_distributed_active_network_ports(session, network_id)"},{"line_number":117,"context_line":"    return dvr_bindings + ha_bindings"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"bacf61ea_a187d4b3","line":115,"range":{"start_line":115,"start_character":12,"end_line":115,"end_character":35},"in_reply_to":"bacf61ea_98116555","updated":"2016-08-04 13:45:39.000000000","message":"sure.","commit_id":"195d66869b8b6860c7742313ede3ad4945592285"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"24ac927bb3ef021b7ea56024ba3ff5aa53dd8b17","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    # Get DEVICE_OWNER_HA_REPLICATED_INT, DEVICE_OWNER_ROUTER_SNAT"},{"line_number":81,"context_line":"    # interface ids on the given network"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"},{"line_number":85,"context_line":"            l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":86,"context_line":"            models_v2.Port.device_id).filter_by(ha\u003dTrue).filter("},{"line_number":87,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":88,"context_line":"            models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":89,"context_line":"    return [port.id for port in query.all()]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_e8a0fda7","line":88,"range":{"start_line":83,"start_character":46,"end_line":88,"end_character":61},"updated":"2016-08-08 16:13:39.000000000","message":"For consistency, can we please use multiple query assignments (such as done in all other functions? i.e. - query \u003d session.query() ; query \u003d query.filter_by() ; ...","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"7db63129e6951968afd34271df1b617b0a81f983","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    # Get DEVICE_OWNER_HA_REPLICATED_INT, DEVICE_OWNER_ROUTER_SNAT"},{"line_number":81,"context_line":"    # interface ids on the given network"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port).join("},{"line_number":84,"context_line":"            l3_attrs_db.RouterExtraAttributes,"},{"line_number":85,"context_line":"            l3_attrs_db.RouterExtraAttributes.router_id \u003d\u003d"},{"line_number":86,"context_line":"            models_v2.Port.device_id).filter_by(ha\u003dTrue).filter("},{"line_number":87,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":88,"context_line":"            models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":89,"context_line":"    return [port.id for port in query.all()]"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_21fa0f43","line":88,"range":{"start_line":83,"start_character":46,"end_line":88,"end_character":61},"in_reply_to":"9ad45d7e_e8a0fda7","updated":"2016-08-09 07:29:52.000000000","message":"sure","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"24ac927bb3ef021b7ea56024ba3ff5aa53dd8b17","unresolved":false,"context_lines":[{"line_number":120,"context_line":"    return dvr_bindings + ha_bindings"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"def get_ha_network_ports_binding(session):"},{"line_number":124,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":125,"context_line":"        query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"},{"line_number":126,"context_line":"        query \u003d query.join(agents_db.Agent,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_880061ac","line":123,"range":{"start_line":123,"start_character":4,"end_line":123,"end_character":32},"updated":"2016-08-08 16:13:39.000000000","message":"This function doesn\u0027t return a binding - it return bindings\u0027 query, so it should be renamed as such (\"get_ha_network_port_bindings_query\"?)","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"7db63129e6951968afd34271df1b617b0a81f983","unresolved":false,"context_lines":[{"line_number":120,"context_line":"    return dvr_bindings + ha_bindings"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"def get_ha_network_ports_binding(session):"},{"line_number":124,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":125,"context_line":"        query \u003d session.query(ml2_models.PortBinding, agents_db.Agent)"},{"line_number":126,"context_line":"        query \u003d query.join(agents_db.Agent,"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_61c1a7e4","line":123,"range":{"start_line":123,"start_character":4,"end_line":123,"end_character":32},"in_reply_to":"9ad45d7e_880061ac","updated":"2016-08-09 07:29:52.000000000","message":"sure","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"f9eecaf2db8b8bc2b62dd4a5ebbdcb4e9b476fb7","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"HA_ROUTER_PORTS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                   const.DEVICE_OWNER_ROUTER_SNAT)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9ad45d7e_4b2b7e94","line":27,"updated":"2016-08-10 17:36:50.000000000","message":"Should this be added to neutron-lib or constants file in neutron and then moved to neutron lib.","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"adf3472d2f62b82fc8a31801f5f5ff2d2e63aa3d","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"HA_ROUTER_PORTS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":27,"context_line":"                   const.DEVICE_OWNER_ROUTER_SNAT)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":38,"id":"9ad45d7e_d4e53438","line":27,"in_reply_to":"9ad45d7e_4b2b7e94","updated":"2016-08-11 06:40:59.000000000","message":"HA_ROUTER_PORTS is used only in this file. So no need to add to constants file.","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c44bcb57eb897715410b975298e206c23ac3af1a","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"HA_ROUTER_PORTS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":28,"context_line":"                   const.DEVICE_OWNER_ROUTER_SNAT)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_a7afe228","line":28,"updated":"2016-08-22 19:32:17.000000000","message":"nit: this should be in constants.py","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"53fdc17c4d61e192a47c584671433c1c4d41e309","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"HA_ROUTER_PORTS \u003d (const.DEVICE_OWNER_HA_REPLICATED_INT,"},{"line_number":28,"context_line":"                   const.DEVICE_OWNER_ROUTER_SNAT)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def get_agent_ip_by_host(session, agent_host):"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_cd22fd51","line":28,"in_reply_to":"1ac06dbe_a7afe228","updated":"2016-08-22 19:51:31.000000000","message":"only this file uses HA_ROUTER_PORTS. So I prefer not to move it to constants.py","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c44bcb57eb897715410b975298e206c23ac3af1a","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":80,"context_line":"    # Get DEVICE_OWNER_HA_REPLICATED_INT, DEVICE_OWNER_ROUTER_SNAT"},{"line_number":81,"context_line":"    # interface ids on the given network"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port)"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_072eb6a4","line":80,"updated":"2016-08-22 19:32:17.000000000","message":"nit: and you probably don\u0027t need this and just rely on the constant definition","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"53fdc17c4d61e192a47c584671433c1c4d41e309","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":80,"context_line":"    # Get DEVICE_OWNER_HA_REPLICATED_INT, DEVICE_OWNER_ROUTER_SNAT"},{"line_number":81,"context_line":"    # interface ids on the given network"},{"line_number":82,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":83,"context_line":"        query \u003d session.query(models_v2.Port)"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_cde95de8","line":80,"in_reply_to":"1ac06dbe_072eb6a4","updated":"2016-08-22 19:51:31.000000000","message":"sure, I will remove the comment.","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c44bcb57eb897715410b975298e206c23ac3af1a","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        return (query1.count() + query2.count() + ha_port_count)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"def get_ha_port_count(session, agent_host, network_id):"},{"line_number":193,"context_line":"    # Return num of HA router interfaces on the given network and host"},{"line_number":194,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":195,"context_line":"        query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_a2e6b012","line":192,"updated":"2016-08-22 19:32:17.000000000","message":"nit: the other methods in this file have the word \"active\" in their name when they filter PORT_STATUS_ACTIVE ports.","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"53fdc17c4d61e192a47c584671433c1c4d41e309","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        return (query1.count() + query2.count() + ha_port_count)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"def get_ha_port_count(session, agent_host, network_id):"},{"line_number":193,"context_line":"    # Return num of HA router interfaces on the given network and host"},{"line_number":194,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":195,"context_line":"        query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"}],"source_content_type":"text/x-python","patch_set":44,"id":"1ac06dbe_2d0c71ec","line":192,"in_reply_to":"1ac06dbe_a2e6b012","updated":"2016-08-22 19:51:31.000000000","message":"I will change the method name.","commit_id":"686835ef68810654a3ba12f578cade4e6f770e70"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        return query"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":80,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":81,"context_line":"        query \u003d session.query(models_v2.Port)"},{"line_number":82,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_548283c9","line":79,"updated":"2016-08-24 19:29:36.000000000","message":"Note: I\u0027m really not liking this HA and DVR stuff in the l2pop mechanism driver. I can\u0027t really blame this patch though since the DVR stuff is already here. Yuck.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        query \u003d query.filter("},{"line_number":86,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":87,"context_line":"            models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":88,"context_line":"    return [port.id for port in query.all()]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"def get_nondistributed_active_network_ports(session, network_id):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_c1e1979c","line":88,"range":{"start_line":88,"start_character":37,"end_line":88,"end_character":43},"updated":"2016-08-24 19:29:36.000000000","message":"This runs the query outside of the context of the subtransaction. What is the point of this subtransaction? You can do a simple select query without a transaction.\n\nAlso, .all() isn\u0027t needed and just generates a temporary list. You can iterate over the query itself.\n\n  return [port.id for port in query]","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        query \u003d query.filter("},{"line_number":86,"context_line":"            models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":87,"context_line":"            models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":88,"context_line":"    return [port.id for port in query.all()]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"def get_nondistributed_active_network_ports(session, network_id):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_7895a4e2","line":88,"range":{"start_line":88,"start_character":37,"end_line":88,"end_character":43},"in_reply_to":"1ac06dbe_c1e1979c","updated":"2016-08-25 17:30:25.000000000","message":"done in PS46. I was following the existing code in this file, for example L99.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":131,"context_line":"                             models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":132,"context_line":"                             models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":133,"context_line":"    return [(None, agent) for agent in query.all()"},{"line_number":134,"context_line":"            if get_agent_ip(agent)]"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_d24843cf","line":133,"range":{"start_line":133,"start_character":44,"end_line":133,"end_character":50},"updated":"2016-08-24 19:29:36.000000000","message":"ditto L88","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":131,"context_line":"                             models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":132,"context_line":"                             models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":133,"context_line":"    return [(None, agent) for agent in query.all()"},{"line_number":134,"context_line":"            if get_agent_ip(agent)]"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_58bf0860","line":133,"range":{"start_line":133,"start_character":44,"end_line":133,"end_character":50},"in_reply_to":"1ac06dbe_d24843cf","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":137,"context_line":"def get_ha_agents_query(session):"},{"line_number":138,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":139,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":140,"context_line":"    # For this, we join again with \u0027Agent\u0027 table on same host, to get all"},{"line_number":141,"context_line":"    # agent entires(i.e ovs, l3, dhcp) on all HA hosts."},{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_7264efe4","line":140,"range":{"start_line":140,"start_character":66,"end_line":140,"end_character":73},"updated":"2016-08-24 19:29:36.000000000","message":"Should you filter it to only return the L2 agents? Or, L2 and L3?","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":137,"context_line":"def get_ha_agents_query(session):"},{"line_number":138,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":139,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":140,"context_line":"    # For this, we join again with \u0027Agent\u0027 table on same host, to get all"},{"line_number":141,"context_line":"    # agent entires(i.e ovs, l3, dhcp) on all HA hosts."},{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_5866e829","line":140,"range":{"start_line":140,"start_character":66,"end_line":140,"end_character":73},"in_reply_to":"1ac06dbe_6780ae47","updated":"2016-08-25 17:30:25.000000000","message":"Yes i.e et all agents on a host and then filter according to the \u0027tunneling_ip\u0027 attribute","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":137,"context_line":"def get_ha_agents_query(session):"},{"line_number":138,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":139,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":140,"context_line":"    # For this, we join again with \u0027Agent\u0027 table on same host, to get all"},{"line_number":141,"context_line":"    # agent entires(i.e ovs, l3, dhcp) on all HA hosts."},{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_6780ae47","line":140,"range":{"start_line":140,"start_character":66,"end_line":140,"end_character":73},"in_reply_to":"1ac06dbe_7264efe4","updated":"2016-08-24 23:37:11.000000000","message":"Looks like this is used in line 157. It seems common across L2 code to get all agents on a host and then filter according to the \u0027tunneling_ip\u0027 attribute. I think the idea is to support out of tree L2 agents. Perhaps we could filter out DHCP, L3 and metadata agents but I don\u0027t know if there\u0027s any value there.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":140,"context_line":"    # For this, we join again with \u0027Agent\u0027 table on same host, to get all"},{"line_number":141,"context_line":"    # agent entires(i.e ovs, l3, dhcp) on all HA hosts."},{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"},{"line_number":144,"context_line":"        db_Agent \u003d orm.aliased(agents_db.Agent, name\u003d\"db_Agent\")"},{"line_number":145,"context_line":"        query \u003d query.join(db_Agent, db_Agent.host \u003d\u003d agents_db.Agent.host)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_e105b314","line":142,"range":{"start_line":142,"start_character":4,"end_line":142,"end_character":45},"updated":"2016-08-24 19:29:36.000000000","message":"I don\u0027t think applying a subtransaction here does anything. We only build the query in this method and exit this context before running the query. It doesn\u0027t make sense to build a query within a transaction, does it?","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":140,"context_line":"    # For this, we join again with \u0027Agent\u0027 table on same host, to get all"},{"line_number":141,"context_line":"    # agent entires(i.e ovs, l3, dhcp) on all HA hosts."},{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"},{"line_number":144,"context_line":"        db_Agent \u003d orm.aliased(agents_db.Agent, name\u003d\"db_Agent\")"},{"line_number":145,"context_line":"        query \u003d query.join(db_Agent, db_Agent.host \u003d\u003d agents_db.Agent.host)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_f82514c2","line":142,"range":{"start_line":142,"start_character":4,"end_line":142,"end_character":45},"in_reply_to":"1ac06dbe_e105b314","updated":"2016-08-25 17:30:25.000000000","message":"done. I removed session","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"},{"line_number":144,"context_line":"        db_Agent \u003d orm.aliased(agents_db.Agent, name\u003d\"db_Agent\")"},{"line_number":145,"context_line":"        query \u003d query.join(db_Agent, db_Agent.host \u003d\u003d agents_db.Agent.host)"},{"line_number":146,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":147,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.l3_agent_id \u003d\u003d db_Agent.id)"},{"line_number":148,"context_line":"        return query"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_b7a9397e","line":145,"range":{"start_line":145,"start_character":8,"end_line":145,"end_character":75},"updated":"2016-08-24 19:29:36.000000000","message":"This query joining the table on an alias of itself is difficult to reason about. I get what you\u0027re after. You want the query to return all of the agents, L3 and otherwise, on the hosts where there is an l3 agent mentioned in the L3HARouterAgentPortBinding table. Is that right? If so, there must be a clearer way to accomplish this.\n\nI think this could be written in a more straight-forward way using a sub-query that selects the hosts from a join between the Agent and L3HARouterAgentPortBinding on the l3_agent_id. Then, the outer query would use this result to select the agents that match the hosts. You might have to pass down the router_id to the sub-query for L155\u0027s filter.\n\nIt might be tricky to handle the filter on L129 where you join the port on device_id \u003d\u003d router_id since you\u0027re joining all agents through the L3 agent and the l3 ha port binding. There, you\u0027re trying to filter out agents that don\u0027t have active ha router ports on the network, right? You might have to reorganize a little bit to pass that information down to the subquery that returns the list of hosts.\n\nThis is a tricky query. Complex queries like this can be very difficult to maintain because they are difficult to reason about. I think breaking it up in to some simpler sub-queries could help.\n\nLet me know if you want me to help out with this.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"},{"line_number":144,"context_line":"        db_Agent \u003d orm.aliased(agents_db.Agent, name\u003d\"db_Agent\")"},{"line_number":145,"context_line":"        query \u003d query.join(db_Agent, db_Agent.host \u003d\u003d agents_db.Agent.host)"},{"line_number":146,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":147,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.l3_agent_id \u003d\u003d db_Agent.id)"},{"line_number":148,"context_line":"        return query"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_c7408238","line":145,"range":{"start_line":145,"start_character":8,"end_line":145,"end_character":75},"in_reply_to":"1ac06dbe_b7a9397e","updated":"2016-08-24 23:37:11.000000000","message":"\u003e I think this could be written in a more straight-forward way using a sub-query that selects the hosts from a join between the Agent and L3HARouterAgentPortBinding on the l3_agent_id. Then, the outer query would use this result to select the agents that match the hosts.\n\nExactly.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":143,"context_line":"        query \u003d session.query(agents_db.Agent)"},{"line_number":144,"context_line":"        db_Agent \u003d orm.aliased(agents_db.Agent, name\u003d\"db_Agent\")"},{"line_number":145,"context_line":"        query \u003d query.join(db_Agent, db_Agent.host \u003d\u003d agents_db.Agent.host)"},{"line_number":146,"context_line":"        query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":147,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.l3_agent_id \u003d\u003d db_Agent.id)"},{"line_number":148,"context_line":"        return query"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_9bcdfab9","line":145,"range":{"start_line":145,"start_character":8,"end_line":145,"end_character":75},"in_reply_to":"1ac06dbe_c7408238","updated":"2016-08-25 17:30:25.000000000","message":"Yes - for \"You want the query to return all of the agents, L3 and otherwise, on the hosts where there is an l3 agent mentioned in the L3HARouterAgentPortBinding table.\"\n\nDOne, I am using subquery in PS46.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        query \u003d get_ha_agents_query(session)"},{"line_number":154,"context_line":"        query \u003d query.filter("},{"line_number":155,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":156,"context_line":"    return [agent for agent in query.all()"},{"line_number":157,"context_line":"            if get_agent_ip(agent)]"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_345c27ed","line":156,"range":{"start_line":156,"start_character":36,"end_line":156,"end_character":42},"updated":"2016-08-24 19:29:36.000000000","message":"ditto L88","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        query \u003d get_ha_agents_query(session)"},{"line_number":154,"context_line":"        query \u003d query.filter("},{"line_number":155,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":156,"context_line":"    return [agent for agent in query.all()"},{"line_number":157,"context_line":"            if get_agent_ip(agent)]"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_7b059ec0","line":156,"range":{"start_line":156,"start_character":36,"end_line":156,"end_character":42},"in_reply_to":"1ac06dbe_345c27ed","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":173,"context_line":"        if ha_iface_ids:"},{"line_number":174,"context_line":"            query1 \u003d query1.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":175,"context_line":"            ha_port_count \u003d get_ha_router_active_port_count("},{"line_number":176,"context_line":"                session, agent_host, network_id)"},{"line_number":177,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_ede002cf","line":174,"range":{"start_line":174,"start_character":35,"end_line":174,"end_character":36},"updated":"2016-08-24 19:29:36.000000000","message":"notin_ is much easier to read. This little ~ is easily missed.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":171,"context_line":""},{"line_number":172,"context_line":"        ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":173,"context_line":"        if ha_iface_ids:"},{"line_number":174,"context_line":"            query1 \u003d query1.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":175,"context_line":"            ha_port_count \u003d get_ha_router_active_port_count("},{"line_number":176,"context_line":"                session, agent_host, network_id)"},{"line_number":177,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_9b121a08","line":174,"range":{"start_line":174,"start_character":35,"end_line":174,"end_character":36},"in_reply_to":"1ac06dbe_ede002cf","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"def get_ha_router_active_port_count(session, agent_host, network_id):"},{"line_number":192,"context_line":"    # Return num of HA router interfaces on the given network and host"},{"line_number":193,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":194,"context_line":"        query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"},{"line_number":195,"context_line":"        query \u003d query.join(agents_db.Agent)"},{"line_number":196,"context_line":"        query \u003d query.filter(agents_db.Agent.host \u003d\u003d agent_host)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_f4e0cfce","line":193,"range":{"start_line":193,"start_character":4,"end_line":193,"end_character":45},"updated":"2016-08-24 19:29:36.000000000","message":"Here you\u0027re at least running the query inside the context of the subtransaction but still, what is the point? You don\u0027t need a transaction to run a simple select query.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":190,"context_line":""},{"line_number":191,"context_line":"def get_ha_router_active_port_count(session, agent_host, network_id):"},{"line_number":192,"context_line":"    # Return num of HA router interfaces on the given network and host"},{"line_number":193,"context_line":"    with session.begin(subtransactions\u003dTrue):"},{"line_number":194,"context_line":"        query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"},{"line_number":195,"context_line":"        query \u003d query.join(agents_db.Agent)"},{"line_number":196,"context_line":"        query \u003d query.filter(agents_db.Agent.host \u003d\u003d agent_host)"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_3b1b261d","line":193,"range":{"start_line":193,"start_character":4,"end_line":193,"end_character":45},"in_reply_to":"1ac06dbe_f4e0cfce","updated":"2016-08-25 17:30:25.000000000","message":"done. Removed session.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":79,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":80,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":81,"context_line":"    l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d"},{"line_number":82,"context_line":"        models_v2.Port.device_id)"},{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_87e9e4b9","line":81,"updated":"2016-09-01 19:16:06.000000000","message":"nit: This should be indented.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":79,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":80,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":81,"context_line":"    l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d"},{"line_number":82,"context_line":"        models_v2.Port.device_id)"},{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_aa0721c8","line":81,"in_reply_to":"fa7ab95a_87e9e4b9","updated":"2016-09-08 22:27:17.000000000","message":"Done","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":79,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":80,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":81,"context_line":"    l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d"},{"line_number":82,"context_line":"        models_v2.Port.device_id)"},{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":85,"context_line":"        models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":86,"context_line":"    return [port.id for port in query]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_bce8bee3","line":85,"range":{"start_line":80,"start_character":0,"end_line":85,"end_character":57},"updated":"2016-09-01 19:16:06.000000000","message":"This is *almost* the same join as L136-L140 and L191-196. Consider a helper method with a descriptive name to reduce duplication.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"def _get_ha_router_interface_ids(session, network_id):"},{"line_number":79,"context_line":"    query \u003d session.query(models_v2.Port)"},{"line_number":80,"context_line":"    query \u003d query.join(l3_hamode_db.L3HARouterAgentPortBinding,"},{"line_number":81,"context_line":"    l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d"},{"line_number":82,"context_line":"        models_v2.Port.device_id)"},{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":85,"context_line":"        models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":86,"context_line":"    return [port.id for port in query]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_f95577fa","line":85,"range":{"start_line":80,"start_character":0,"end_line":85,"end_character":57},"in_reply_to":"fa7ab95a_bce8bee3","updated":"2016-09-08 22:27:17.000000000","message":"I ended up creating a new method to reduce the duplication between this query and the one on L191. See my follow-up comment around L191.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":85,"context_line":"        models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":86,"context_line":"    return [port.id for port in query]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"def get_nondistributed_active_network_ports(session, network_id):"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_07e07495","line":86,"range":{"start_line":86,"start_character":11,"end_line":86,"end_character":38},"updated":"2016-09-01 19:16:06.000000000","message":"I think you\u0027ve got the right idea with this query but in both cases where we use this method, we run this query and then we take the resulting list and put it right back in to a second query. That takes a round trip to the DB when I don\u0027t think we need one.\n\nI think with a little bit of code change, we can return a sub-query (without actually running the query) and use that in the expressions below where this method is called.\n\nIt might be as simple as returning query.from_self(Port.id) [1] here instead of running the query and returning a list. Then, I think you can remove the \"if\" on L95 and just pass the query to notin.\n\n [1] http://docs.sqlalchemy.org/en/latest/orm/query.html#sqlalchemy.orm.query.Query.from_self","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    query \u003d query.filter("},{"line_number":84,"context_line":"        models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":85,"context_line":"        models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":86,"context_line":"    return [port.id for port in query]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"def get_nondistributed_active_network_ports(session, network_id):"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_794a4757","line":86,"range":{"start_line":86,"start_character":11,"end_line":86,"end_character":38},"in_reply_to":"fa7ab95a_07e07495","updated":"2016-09-08 22:27:17.000000000","message":"Done","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":94,"context_line":"    ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":95,"context_line":"    if ha_iface_ids:"},{"line_number":96,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":97,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":98,"context_line":"            if get_agent_ip(agent)]"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_b9e8d0e3","line":95,"range":{"start_line":95,"start_character":4,"end_line":95,"end_character":20},"updated":"2016-09-01 19:16:06.000000000","message":"I think if you follow suggestion on L86, you can just remove this condition and run L96 unconditionally.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    query \u003d query.filter(models_v2.Port.device_owner !\u003d"},{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":94,"context_line":"    ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":95,"context_line":"    if ha_iface_ids:"},{"line_number":96,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":97,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":98,"context_line":"            if get_agent_ip(agent)]"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_5964a3e4","line":95,"range":{"start_line":95,"start_character":4,"end_line":95,"end_character":20},"in_reply_to":"fa7ab95a_b9e8d0e3","updated":"2016-09-08 22:27:17.000000000","message":"Done","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":94,"context_line":"    ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":95,"context_line":"    if ha_iface_ids:"},{"line_number":96,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":97,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":98,"context_line":"            if get_agent_ip(agent)]"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_a7f98882","line":96,"range":{"start_line":96,"start_character":29,"end_line":96,"end_character":30},"updated":"2016-09-01 19:16:06.000000000","message":"nit: I\u0027d prefer using \"notin\"","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                         const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":94,"context_line":"    ha_iface_ids \u003d _get_ha_router_interface_ids(session, network_id)"},{"line_number":95,"context_line":"    if ha_iface_ids:"},{"line_number":96,"context_line":"        query \u003d query.filter(~models_v2.Port.id.in_(ha_iface_ids))"},{"line_number":97,"context_line":"    return [(bind, agent) for bind, agent in query.all()"},{"line_number":98,"context_line":"            if get_agent_ip(agent)]"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_4ae5b506","line":96,"range":{"start_line":96,"start_character":29,"end_line":96,"end_character":30},"in_reply_to":"fa7ab95a_a7f98882","updated":"2016-09-08 22:27:17.000000000","message":"Done","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        query \u003d query.filter("},{"line_number":134,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":135,"context_line":"    elif network_id:"},{"line_number":136,"context_line":"        query \u003d query.join(models_v2.Port, models_v2.Port.device_id \u003d\u003d"},{"line_number":137,"context_line":"                           l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":138,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":139,"context_line":"                             models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":140,"context_line":"                             models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":141,"context_line":"    else:"},{"line_number":142,"context_line":"        return []"},{"line_number":143,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_79e568a2","line":140,"range":{"start_line":136,"start_character":0,"end_line":140,"end_character":78},"updated":"2016-09-01 19:16:06.000000000","message":"nit: This is the same join as L191 - L195. See my comment on L85.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        query \u003d query.filter("},{"line_number":134,"context_line":"            l3_hamode_db.L3HARouterAgentPortBinding.router_id \u003d\u003d router_id)"},{"line_number":135,"context_line":"    elif network_id:"},{"line_number":136,"context_line":"        query \u003d query.join(models_v2.Port, models_v2.Port.device_id \u003d\u003d"},{"line_number":137,"context_line":"                           l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":138,"context_line":"        query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":139,"context_line":"                             models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"},{"line_number":140,"context_line":"                             models_v2.Port.device_owner.in_(HA_ROUTER_PORTS))"},{"line_number":141,"context_line":"    else:"},{"line_number":142,"context_line":"        return []"},{"line_number":143,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_0e71f3e0","line":140,"range":{"start_line":136,"start_character":0,"end_line":140,"end_character":78},"in_reply_to":"fa7ab95a_79e568a2","updated":"2016-09-08 22:27:17.000000000","message":"This query is different enough that I\u0027m okay leaving it with some slight duplication.\n\nRead my follow-up on L191. Those two queries are centered around selecting ha router interface ports on a network. This one is centered around selecting agents (that are hosting active ha router interface ports). The two queries do share this filter but I\u0027m not sure it is worth extracting a method to reduce the duplication.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    else:"},{"line_number":142,"context_line":"        return []"},{"line_number":143,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":144,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":145,"context_line":"    agents_query \u003d session.query(agents_db.Agent)"},{"line_number":146,"context_line":"    agents_query \u003d agents_query.filter(agents_db.Agent.host.in_(query))"},{"line_number":147,"context_line":"    return [agent for agent in agents_query"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_5918a4d3","line":144,"updated":"2016-09-01 19:16:06.000000000","message":"This is a lot easier to wrap my head around. I think this will help anyone who needs to maintain this.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":144,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":145,"context_line":"    agents_query \u003d session.query(agents_db.Agent)"},{"line_number":146,"context_line":"    agents_query \u003d agents_query.filter(agents_db.Agent.host.in_(query))"},{"line_number":147,"context_line":"    return [agent for agent in agents_query"},{"line_number":148,"context_line":"            if get_agent_ip(agent)]"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_5c533217","line":146,"range":{"start_line":146,"start_character":64,"end_line":146,"end_character":69},"updated":"2016-09-01 19:16:06.000000000","message":"This query is the join of several tables. Do the joins add fields from the joined tables to the result? If that is the case I\u0027m trying to figure out what in_ does with that. Does it just take the first column from from the query results and use that?\n\nYou might consider making a little more explicit by taking the \".host\" off of L128 and then making this \"query.from_self(agents_db.Agent.host)\" or something like that.\n\nI haven\u0027t tried this so maybe I\u0027m missing something important.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    # L3HARouterAgentPortBinding will have l3 agent ids of hosting agents."},{"line_number":144,"context_line":"    # But we need l2 agent(for tunneling ip) while creating FDB entries."},{"line_number":145,"context_line":"    agents_query \u003d session.query(agents_db.Agent)"},{"line_number":146,"context_line":"    agents_query \u003d agents_query.filter(agents_db.Agent.host.in_(query))"},{"line_number":147,"context_line":"    return [agent for agent in agents_query"},{"line_number":148,"context_line":"            if get_agent_ip(agent)]"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_b316b056","line":146,"range":{"start_line":146,"start_character":64,"end_line":146,"end_character":69},"in_reply_to":"fa7ab95a_5c533217","updated":"2016-09-08 22:27:17.000000000","message":"I answered my own question running this in the debugger. It is all good. This essentially translates to this:\n\n  ... FROM agents WHERE agents.host IN (SELECT DISTINCT agents.host AS agents_host ...\n\nLooks good to me.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":180,"context_line":"                               const.DEVICE_OWNER_DVR_INTERFACE,"},{"line_number":181,"context_line":"                               ml2_models.DistributedPortBinding.host \u003d\u003d"},{"line_number":182,"context_line":"                               agent_host)"},{"line_number":183,"context_line":"        return (query1.count() + query2.count() + ha_port_count)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"def get_ha_router_active_port_count(session, agent_host, network_id):"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_fc73264c","line":183,"range":{"start_line":183,"start_character":15,"end_line":183,"end_character":64},"updated":"2016-09-01 19:16:06.000000000","message":"I wonder if there is some reasonable way to combine these into a compound select that would only require a single round-trip to the DB. From what I can tell, this could take 3 (including L170). Since this is kind of pre-existing, maybe we could follow up this patch set with something?","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":180,"context_line":"                               const.DEVICE_OWNER_DVR_INTERFACE,"},{"line_number":181,"context_line":"                               ml2_models.DistributedPortBinding.host \u003d\u003d"},{"line_number":182,"context_line":"                               agent_host)"},{"line_number":183,"context_line":"        return (query1.count() + query2.count() + ha_port_count)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"def get_ha_router_active_port_count(session, agent_host, network_id):"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_4ef4db86","line":183,"range":{"start_line":183,"start_character":15,"end_line":183,"end_character":64},"in_reply_to":"fa7ab95a_fc73264c","updated":"2016-09-08 22:27:17.000000000","message":"I think we could defer this.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":188,"context_line":"    query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"},{"line_number":189,"context_line":"    query \u003d query.join(agents_db.Agent)"},{"line_number":190,"context_line":"    query \u003d query.filter(agents_db.Agent.host \u003d\u003d agent_host)"},{"line_number":191,"context_line":"    query \u003d query.join(models_v2.Port, models_v2.Port.device_id \u003d\u003d"},{"line_number":192,"context_line":"                       l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":193,"context_line":"    query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":194,"context_line":"                         models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_bc27de10","line":191,"updated":"2016-09-01 19:16:06.000000000","message":"See comment on L85","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":188,"context_line":"    query \u003d session.query(l3_hamode_db.L3HARouterAgentPortBinding)"},{"line_number":189,"context_line":"    query \u003d query.join(agents_db.Agent)"},{"line_number":190,"context_line":"    query \u003d query.filter(agents_db.Agent.host \u003d\u003d agent_host)"},{"line_number":191,"context_line":"    query \u003d query.join(models_v2.Port, models_v2.Port.device_id \u003d\u003d"},{"line_number":192,"context_line":"                       l3_hamode_db.L3HARouterAgentPortBinding.router_id)"},{"line_number":193,"context_line":"    query \u003d query.filter(models_v2.Port.network_id \u003d\u003d network_id,"},{"line_number":194,"context_line":"                         models_v2.Port.status \u003d\u003d const.PORT_STATUS_ACTIVE,"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_eefccf74","line":191,"in_reply_to":"fa7ab95a_bc27de10","updated":"2016-09-08 22:27:17.000000000","message":"I played with this query a little bit. I think it makes sense to rearrange it so that it begins with a query on port (since the method counts ports). To me, that makes it more obvious that the query is mainly about ports, then joins in other tables to help filter the results.\n\nAfter rearranging, it looks just like the query on L79 with just a couple of extra filter conditions: the host and the active status.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"}],"neutron/plugins/ml2/drivers/l2pop/mech_driver.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ef231b1941f1c54a625865913c985ded237e1b97","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        # When current port is HA network port,"},{"line_number":287,"context_line":"        # notify other agents to add fdb rule for related HA router port"},{"line_number":288,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_ROUTER_HA_INTF:"},{"line_number":289,"context_line":"            for router_port, in self.get_ha_router_ports_from_network_port("},{"line_number":290,"context_line":"                                            session, port[\u0027id\u0027]):"},{"line_number":291,"context_line":"                ha_fdb_entries \u003d self._create_fdb_for_ha_router_port("},{"line_number":292,"context_line":"                    session, router_port)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_9c5731ca","line":289,"range":{"start_line":289,"start_character":27,"end_line":289,"end_character":28},"updated":"2015-12-14 18:54:24.000000000","message":"Why the comma is here?","commit_id":"c323727f1f16b867d95b4bc1ebf394712e24afc5"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9e16ad1cd048d929775ac2faa8340783aa002f89","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_4b7b6d28","line":196,"updated":"2016-03-09 17:49:13.000000000","message":"If get_ha_router_port_bindings is always just passed a list of all ha routers on a network returned by get_ha_router_ports(), it seems like you could do this all in one query instead of querying all of the ha router ports, then doing a separate query with the results of the previous one. Maybe have a _get_ha_router_ports() that returns the generated query and use it in a get_ha_router_port_bindings(session, network_id) function?","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"65cc949dea227d7c69807f96e63998c41c238b0d","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"3afc51ec_ae47034a","line":196,"in_reply_to":"3afc51ec_86906bee","updated":"2016-03-14 07:01:31.000000000","message":"Thanks Terry. With the suggested changes, we may reduce one DB access when first port is \"ACTIVE\" on a agent, but makes code complex. As this code is not hit frequently(only for first port of a agent), we can go with only existing changes.\n\nAnyway, in the dependent child change https://review.openstack.org/#/c/282874/ I will use new flag DEVICE_OWNER_HA_INTERFACE, which makes code simpler.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e5ef61e8bb8e44d5300bfa8a05e9c5728e42f578","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"3afc51ec_63269fcf","line":196,"in_reply_to":"3afc51ec_ae47034a","updated":"2016-03-14 13:56:36.000000000","message":"It thought it made the non-db code a lot less complex and just basically replaced an array with a subquery in the db code. It just feels sloppy to me to do two queries and two full list traversals when you can just do one query. Not (quite) something I would -1 over, since I don\u0027t *think* that the lack of atomicity would cause a problem (an ha router port being deleted between the two queries), but it does bug me a little. :p","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4f0f5a6739ba88119c4dc0020c5d4531cf835f5a","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_b93a2fe5","line":196,"in_reply_to":"5aef4532_4b7b6d28","updated":"2016-03-10 07:02:04.000000000","message":"If _get_ha_router_ports() returns a query object, how can I use that in get_ha_router_port_bindings and get_nondvr_active_network_ports ?","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f1ba076bb6ccff24d49ba7d361c45858f3c8d3c1","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_c3c9d327","line":196,"in_reply_to":"5aef4532_b93a2fe5","updated":"2016-03-10 23:54:41.000000000","message":"anil: Something like: http://paste.openstack.org/show/490080/ is what I was thinking with both of my comments.\n\nI haven\u0027t done any real testing with it (and am not an sqlalchemy expert), but basically just returning the query object and using .with_entities() to limit it to a particular column, then using that as part of a filter() which generates a nested select instead of doing multiple queries/processing the output of queries.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"50da23c243be191fe4e6b2825aa81997144be1b3","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        ha_port_ids \u003d [port.id for port in ha_ports]"},{"line_number":194,"context_line":"        ha_routers \u003d set(port[\u0027device_id\u0027] for port in ha_ports)"},{"line_number":195,"context_line":"        if ha_routers:"},{"line_number":196,"context_line":"            tunnel_network_ports +\u003d l2pop_db.get_ha_router_port_bindings("},{"line_number":197,"context_line":"                session, ha_routers)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("}],"source_content_type":"text/x-python","patch_set":17,"id":"3afc51ec_86906bee","line":196,"in_reply_to":"5aef4532_c3c9d327","updated":"2016-03-11 13:43:14.000000000","message":"Thanks Terry. I will try suggested changes.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"9e16ad1cd048d929775ac2faa8340783aa002f89","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("},{"line_number":200,"context_line":"            l2pop_db.get_nondvr_active_network_ports("},{"line_number":201,"context_line":"                session, network_id, ha_port_ids))"},{"line_number":202,"context_line":"        ports \u003d agent_fdb_entries[network_id][\u0027ports\u0027]"},{"line_number":203,"context_line":"        ports.update(self._get_tunnels("},{"line_number":204,"context_line":"            fdb_network_ports + tunnel_network_ports,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_40da2ce9","line":201,"updated":"2016-03-09 17:49:13.000000000","message":"Is there a reason to pass ha_port_ids here instead of just having get_nondvr_active_network_ports never include ha router ports? This is the only place the function is called that I can find.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4f0f5a6739ba88119c4dc0020c5d4531cf835f5a","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        fdb_network_ports \u003d ("},{"line_number":200,"context_line":"            l2pop_db.get_nondvr_active_network_ports("},{"line_number":201,"context_line":"                session, network_id, ha_port_ids))"},{"line_number":202,"context_line":"        ports \u003d agent_fdb_entries[network_id][\u0027ports\u0027]"},{"line_number":203,"context_line":"        ports.update(self._get_tunnels("},{"line_number":204,"context_line":"            fdb_network_ports + tunnel_network_ports,"}],"source_content_type":"text/x-python","patch_set":17,"id":"5aef4532_f4c79e93","line":201,"in_reply_to":"5aef4532_40da2ce9","updated":"2016-03-10 07:02:04.000000000","message":"I have to create only flood flows and no unicast flows for HA router interface ports, so they have to be get separately and to be added to tunnel_network_ports list.\n\nget_nondvr_active_network_ports should exclude HA router \"interface\" ports as they are taken care through get_ha_router_port_bindings.\n\nWe don\u0027t have separate device owner for HA router interface ports. So Ha router interface ports need to be find out separately.","commit_id":"fd46063dfe243af46385b9fcfd16e16174d81cef"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"97c3834b389d3579a682bc68fc3903eb075557dd","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                                          port, agent_host)"},{"line_number":65,"context_line":"        # If this Ha router port is last port on network on some HA agents,"},{"line_number":66,"context_line":"        # notify all agents to remove flood entry to these HA agents."},{"line_number":67,"context_line":"        session \u003d db_api.get_session()"},{"line_number":68,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":69,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"},{"line_number":70,"context_line":"            other_fdb_ports \u003d self._get_ha_port_agents_fdb("}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_85a5e105","line":67,"range":{"start_line":67,"start_character":8,"end_line":67,"end_character":38},"updated":"2016-03-29 13:34:25.000000000","message":"This session is not needed if port[\u0027device_owner\u0027] !\u003d HA. Please move this line to inside the \u0027if\u0027 block.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"2c1a753f2e53c9bccbcffa257e98b281a583727d","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                                          port, agent_host)"},{"line_number":65,"context_line":"        # If this Ha router port is last port on network on some HA agents,"},{"line_number":66,"context_line":"        # notify all agents to remove flood entry to these HA agents."},{"line_number":67,"context_line":"        session \u003d db_api.get_session()"},{"line_number":68,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":69,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"},{"line_number":70,"context_line":"            other_fdb_ports \u003d self._get_ha_port_agents_fdb("}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_5bdc4112","line":67,"range":{"start_line":67,"start_character":8,"end_line":67,"end_character":38},"in_reply_to":"ba0121b8_85a5e105","updated":"2016-03-29 14:39:17.000000000","message":"sure. thanks John","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"97c3834b389d3579a682bc68fc3903eb075557dd","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def _fixed_ips_changed(self, context, orig, port, diff_ips):"},{"line_number":89,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":90,"context_line":"            return"},{"line_number":91,"context_line":"        orig_ips, port_ips \u003d diff_ips"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        if (port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE):"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_45d0e953","line":90,"range":{"start_line":90,"start_character":12,"end_line":90,"end_character":18},"updated":"2016-03-29 13:34:25.000000000","message":"(No action needed) the function name implies a boolean return value (and the last statement in the function actually returns True), so it\u0027s not clear why the return value is never used. This makes me wonder why the function returns True at all.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"2c1a753f2e53c9bccbcffa257e98b281a583727d","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def _fixed_ips_changed(self, context, orig, port, diff_ips):"},{"line_number":89,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":90,"context_line":"            return"},{"line_number":91,"context_line":"        orig_ips, port_ips \u003d diff_ips"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        if (port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE):"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_7bcca535","line":90,"range":{"start_line":90,"start_character":12,"end_line":90,"end_character":18},"in_reply_to":"ba0121b8_45d0e953","updated":"2016-03-29 14:39:17.000000000","message":"Actually it shoudn\u0027t return any value. May be a separate patch for removing existing return values?","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3b03a2a4ad6a51f6e285fdc5f20e6a9c394d00d5","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def _fixed_ips_changed(self, context, orig, port, diff_ips):"},{"line_number":89,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":90,"context_line":"            return"},{"line_number":91,"context_line":"        orig_ips, port_ips \u003d diff_ips"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        if (port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE):"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba0121b8_cd5898fd","line":90,"range":{"start_line":90,"start_character":12,"end_line":90,"end_character":18},"in_reply_to":"ba0121b8_7bcca535","updated":"2016-03-29 14:43:56.000000000","message":"I\u0027m good with whatever solution you come up for this :)","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"d04304d2e92b511470d0e3d6f2f15cb6b86e54a8","unresolved":false,"context_lines":[{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        session \u003d db_api.get_session()"},{"line_number":309,"context_line":"        agent_active_ports \u003d l2pop_db.get_agent_network_active_port_count("},{"line_number":310,"context_line":"            session, agent_host, network_id)"},{"line_number":311,"context_line":"        agent \u003d l2pop_db.get_agent_by_host(db_api.get_session(), agent_host)"},{"line_number":312,"context_line":"        if not self._validate_segment(segment, port[\u0027id\u0027], agent):"},{"line_number":313,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":19,"id":"da0c15f0_0627dfca","line":310,"updated":"2016-03-29 11:42:28.000000000","message":"nit: removal of newline here is an unrelated change.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"d04304d2e92b511470d0e3d6f2f15cb6b86e54a8","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        agent_ip \u003d l2pop_db.get_agent_ip(agent)"},{"line_number":316,"context_line":"        other_fdb_entries \u003d self._get_fdb_entries_template("},{"line_number":317,"context_line":"            segment, agent_ip, port[\u0027network_id\u0027])"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        if agent_active_ports \u003d\u003d 0:"},{"line_number":320,"context_line":"            # Agent is removing its last activated port in this network,"},{"line_number":321,"context_line":"            # other agents needs to be notified to delete their flooding entry."}],"source_content_type":"text/x-python","patch_set":19,"id":"da0c15f0_4631d704","line":318,"updated":"2016-03-29 11:42:28.000000000","message":"nit: addition of newline above here is an unrelated change.","commit_id":"f5653248deabdf66a50bb3aa08f16665ea0dd457"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        agent_host \u003d context.host"},{"line_number":63,"context_line":"        fdb_entries \u003d self._get_agent_fdb(context.bottom_bound_segment,"},{"line_number":64,"context_line":"                                          port, agent_host)"},{"line_number":65,"context_line":"        # If this Ha router port is last port on network on some HA agents,"},{"line_number":66,"context_line":"        # notify all agents to remove flood entry to these HA agents."},{"line_number":67,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":68,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_9fc9e8ae","line":65,"updated":"2016-05-11 14:46:27.000000000","message":"I don\u0027t see you checking that specific condition \"Is this HA router port last port on a network on some ha agents\", that check is done in L52.\n\nI think you should remove that comment because makes this misleading, and either give a more detailed name to L47 function, or add some docstring to explain the filtering you do based on active ports, etc...","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        agent_host \u003d context.host"},{"line_number":63,"context_line":"        fdb_entries \u003d self._get_agent_fdb(context.bottom_bound_segment,"},{"line_number":64,"context_line":"                                          port, agent_host)"},{"line_number":65,"context_line":"        # If this Ha router port is last port on network on some HA agents,"},{"line_number":66,"context_line":"        # notify all agents to remove flood entry to these HA agents."},{"line_number":67,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":68,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_e7b4f982","line":65,"in_reply_to":"dab17558_9fc9e8ae","updated":"2016-05-17 09:36:09.000000000","message":"done","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _fixed_ips_changed(self, context, orig, port, diff_ips):"},{"line_number":94,"context_line":"        # REVISIT(anilvenkata): This function is not expected to return"},{"line_number":95,"context_line":"        # anything. Fix returning bool values"},{"line_number":96,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":97,"context_line":"            return"},{"line_number":98,"context_line":"        orig_ips, port_ips \u003d diff_ips"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_ffa754a4","line":95,"updated":"2016-05-11 14:46:27.000000000","message":"Ah, you mean the bool values in L187/L189 ? can you move the comment down there may be ?","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _fixed_ips_changed(self, context, orig, port, diff_ips):"},{"line_number":94,"context_line":"        # REVISIT(anilvenkata): This function is not expected to return"},{"line_number":95,"context_line":"        # anything. Fix returning bool values"},{"line_number":96,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_HA_REPLICATED_INT:"},{"line_number":97,"context_line":"            return"},{"line_number":98,"context_line":"        orig_ips, port_ips \u003d diff_ips"}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_2761e105","line":95,"in_reply_to":"dab17558_ffa754a4","updated":"2016-05-17 09:36:09.000000000","message":"I made it not to return any bool and removed the above REVISIT comment","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    def _update_ha_router_port_up("},{"line_number":233,"context_line":"            self, session, network_id, router_id, segment, other_fdb_entries):"},{"line_number":234,"context_line":"        # We notify FDB of all HA agents instead of single HA agent,"},{"line_number":235,"context_line":"        # so that, if flood entries are not created for any previous agents"},{"line_number":236,"context_line":"        # (because that time any of database, messaging server, neutron-server,"},{"line_number":237,"context_line":"        # destination L3 agent are dead), now flood entries can be recreated."}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_9ffd6870","line":234,"updated":"2016-05-11 14:46:27.000000000","message":"Can we create a Docstring  instead? :) Please note that docstring must be imperative (if I didn\u0027t get it wrong).","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    def _update_ha_router_port_up("},{"line_number":233,"context_line":"            self, session, network_id, router_id, segment, other_fdb_entries):"},{"line_number":234,"context_line":"        # We notify FDB of all HA agents instead of single HA agent,"},{"line_number":235,"context_line":"        # so that, if flood entries are not created for any previous agents"},{"line_number":236,"context_line":"        # (because that time any of database, messaging server, neutron-server,"},{"line_number":237,"context_line":"        # destination L3 agent are dead), now flood entries can be recreated."}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_075c9d49","line":234,"in_reply_to":"dab17558_9ffd6870","updated":"2016-05-17 09:36:09.000000000","message":"done","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        fdb_entries \u003d self._get_agent_fdb(context.bottom_bound_segment,"},{"line_number":68,"context_line":"                                          port, agent_host)"},{"line_number":69,"context_line":"        session \u003d db_api.get_session()"},{"line_number":70,"context_line":"        if l3_hamode_db.is_ha_router_port(session, port):"},{"line_number":71,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"},{"line_number":72,"context_line":"            other_fdb_ports \u003d self._get_ha_port_agents_fdb("},{"line_number":73,"context_line":"                session, network_id, port[\u0027device_id\u0027])"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_e84d909a","line":70,"range":{"start_line":70,"start_character":11,"end_line":70,"end_character":57},"updated":"2016-08-24 19:29:36.000000000","message":"Is it necessary to check this? It is an extra round trip to the DB and I wonder if the get_ha_agents_by_router_id query would come up empty anyway (because of the inner join on L3HARouterAgentPortBinding.router_id). If so, we might as well not check this and just run than query. One less round trip to the DB, right?\n\nI haven\u0027t totally checked this out, I could be missing something.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        fdb_entries \u003d self._get_agent_fdb(context.bottom_bound_segment,"},{"line_number":68,"context_line":"                                          port, agent_host)"},{"line_number":69,"context_line":"        session \u003d db_api.get_session()"},{"line_number":70,"context_line":"        if l3_hamode_db.is_ha_router_port(session, port):"},{"line_number":71,"context_line":"            network_id \u003d port[\u0027network_id\u0027]"},{"line_number":72,"context_line":"            other_fdb_ports \u003d self._get_ha_port_agents_fdb("},{"line_number":73,"context_line":"                session, network_id, port[\u0027device_id\u0027])"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_5bbd42d6","line":70,"range":{"start_line":70,"start_character":11,"end_line":70,"end_character":57},"in_reply_to":"1ac06dbe_e84d909a","updated":"2016-08-25 17:30:25.000000000","message":"Thanks Carl. No need for this check. Removed in PS46.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d8f57bdf8716bdbba7f4473e3358ed4eac25abd8","unresolved":false,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        return agents"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def update_port_up(self, context):"},{"line_number":238,"context_line":"        self._update_port_up(context)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def update_port_down(self, context):"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_3c6db598","line":237,"updated":"2016-09-01 09:35:28.000000000","message":"what\u0027s the point of this function and not just naming L256 update_port_up?","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4ed47becdd91aff86754a72ac1d90dae3fa67820","unresolved":false,"context_lines":[{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        return agents"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def update_port_up(self, context):"},{"line_number":238,"context_line":"        self._update_port_up(context)"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def update_port_down(self, context):"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_7f180f85","line":237,"in_reply_to":"fa7ab95a_3c6db598","updated":"2016-09-01 09:50:54.000000000","message":"sure","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d8f57bdf8716bdbba7f4473e3358ed4eac25abd8","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":244,"context_line":"            service_constants.L3_ROUTER_NAT)"},{"line_number":245,"context_line":"        # when agent transitions to backup, don\u0027t remove flood flows"},{"line_number":246,"context_line":"        if l3plugin and agent_host:"},{"line_number":247,"context_line":"            admin_context \u003d n_context.get_admin_context()"},{"line_number":248,"context_line":"            if l3plugin.list_router_ids_on_host("},{"line_number":249,"context_line":"                admin_context, agent_host, [port[\u0027device_id\u0027]]):"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_bcb465f2","line":246,"updated":"2016-09-01 09:35:28.000000000","message":"you\u0027re also going to want to check that the l3 plugin has the attr \u0027list_router_ids_on_host\u0027 before continuing as well. Some L3 plugins are not agent based.","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4ed47becdd91aff86754a72ac1d90dae3fa67820","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":244,"context_line":"            service_constants.L3_ROUTER_NAT)"},{"line_number":245,"context_line":"        # when agent transitions to backup, don\u0027t remove flood flows"},{"line_number":246,"context_line":"        if l3plugin and agent_host:"},{"line_number":247,"context_line":"            admin_context \u003d n_context.get_admin_context()"},{"line_number":248,"context_line":"            if l3plugin.list_router_ids_on_host("},{"line_number":249,"context_line":"                admin_context, agent_host, [port[\u0027device_id\u0027]]):"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_3f07671d","line":246,"in_reply_to":"fa7ab95a_bcb465f2","updated":"2016-09-01 09:50:54.000000000","message":"sure","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d06f546b6738f1d229804b7f0f8bbb770257d29d","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            self, session, network_id, router_id):"},{"line_number":60,"context_line":"        other_fdb_ports \u003d {}"},{"line_number":61,"context_line":"        for agent in l2pop_db.get_ha_agents_by_router_id(session, router_id):"},{"line_number":62,"context_line":"            agent_active_ports \u003d l2pop_db.get_agent_network_active_port_count("},{"line_number":63,"context_line":"                session, agent.host, network_id)"},{"line_number":64,"context_line":"            if agent_active_ports \u003d\u003d 0:"},{"line_number":65,"context_line":"                ip \u003d l2pop_db.get_agent_ip(agent)"}],"source_content_type":"text/x-python","patch_set":54,"id":"fa7ab95a_3c610e3a","line":62,"updated":"2016-09-01 19:16:06.000000000","message":"So, this could now be 3 DB round trips per ha agent. If I want three agents for an HA router, that could be 9. I wonder how much it would take to optimize this. Could it be done reasonably?","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"efa16ce15f3cb70d7729953edb136a19671885d9","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            self, session, network_id, router_id):"},{"line_number":60,"context_line":"        other_fdb_ports \u003d {}"},{"line_number":61,"context_line":"        for agent in l2pop_db.get_ha_agents_by_router_id(session, router_id):"},{"line_number":62,"context_line":"            agent_active_ports \u003d l2pop_db.get_agent_network_active_port_count("},{"line_number":63,"context_line":"                session, agent.host, network_id)"},{"line_number":64,"context_line":"            if agent_active_ports \u003d\u003d 0:"},{"line_number":65,"context_line":"                ip \u003d l2pop_db.get_agent_ip(agent)"}],"source_content_type":"text/x-python","patch_set":54,"id":"9a89bdaa_397b5f2b","line":62,"in_reply_to":"fa7ab95a_3c610e3a","updated":"2016-09-08 22:27:17.000000000","message":"I didn\u0027t get to playing around with this. Maybe we could explore this as a follow-up.","commit_id":"a87edd94f90ad68f6b0e1b42e4e51fc03705ad65"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0e89c67d3927c255f02901256db5ca47f0fe2a15","unresolved":false,"context_lines":[{"line_number":1663,"context_line":"            if l3_hamode_db.ha_router_port(session, port):"},{"line_number":1664,"context_line":"                # Mechanism drivers need port status of calling agents"},{"line_number":1665,"context_line":"                # (i.e active or backup Ha router agents)"},{"line_number":1666,"context_line":"                original_port \u003d self._make_port_dict(port)"},{"line_number":1667,"context_line":"                network \u003d network or self.get_network("},{"line_number":1668,"context_line":"                    context, original_port[\u0027network_id\u0027])"},{"line_number":1669,"context_line":"                updated_port \u003d self._make_port_dict(port)"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_b903fdb7","line":1666,"updated":"2016-08-08 13:47:40.000000000","message":"I see a lot of common bits between this and L1682-L1693\n\nWhat do you think about creating a flag \u0027ha_router_port\u0027 before this big if / elif / elif, and merge this new branch and the existing one?","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"ef7923fe799a05b11ff4c48ef11fae74be2b5538","unresolved":false,"context_lines":[{"line_number":1663,"context_line":"            if l3_hamode_db.ha_router_port(session, port):"},{"line_number":1664,"context_line":"                # Mechanism drivers need port status of calling agents"},{"line_number":1665,"context_line":"                # (i.e active or backup Ha router agents)"},{"line_number":1666,"context_line":"                original_port \u003d self._make_port_dict(port)"},{"line_number":1667,"context_line":"                network \u003d network or self.get_network("},{"line_number":1668,"context_line":"                    context, original_port[\u0027network_id\u0027])"},{"line_number":1669,"context_line":"                updated_port \u003d self._make_port_dict(port)"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_4ea03191","line":1666,"in_reply_to":"9ad45d7e_b903fdb7","updated":"2016-08-08 15:32:57.000000000","message":"Thanks Miguel. HA needs special handling. I added a comment in next PS. Let me know if I need to optimize it further.","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0e89c67d3927c255f02901256db5ca47f0fe2a15","unresolved":false,"context_lines":[{"line_number":1671,"context_line":"                                               port.port_binding.host)"},{"line_number":1672,"context_line":"                port.status \u003d status"},{"line_number":1673,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1674,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1675,"context_line":"                mech_context \u003d (driver_context.PortContext("},{"line_number":1676,"context_line":"                    self, context, updated_port, network,"},{"line_number":1677,"context_line":"                    port.port_binding, levels, original_port\u003doriginal_port))"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_d91cf179","line":1674,"range":{"start_line":1674,"start_character":35,"end_line":1674,"end_character":36},"updated":"2016-08-08 13:47:40.000000000","message":"what\u0027s the difference between this here, and what you do on the lines L1683/L1684 ? \n\nIf my eye isn\u0027t missing anything, you could refactor that whole block into a function, and provide \"set_updated_host_id\" flag or something like that to the function.","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"ef7923fe799a05b11ff4c48ef11fae74be2b5538","unresolved":false,"context_lines":[{"line_number":1671,"context_line":"                                               port.port_binding.host)"},{"line_number":1672,"context_line":"                port.status \u003d status"},{"line_number":1673,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1674,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1675,"context_line":"                mech_context \u003d (driver_context.PortContext("},{"line_number":1676,"context_line":"                    self, context, updated_port, network,"},{"line_number":1677,"context_line":"                    port.port_binding, levels, original_port\u003doriginal_port))"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_6ed1b5fb","line":1674,"range":{"start_line":1674,"start_character":35,"end_line":1674,"end_character":36},"in_reply_to":"9ad45d7e_d91cf179","updated":"2016-08-08 15:32:57.000000000","message":"added comment in PS34.","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0e89c67d3927c255f02901256db5ca47f0fe2a15","unresolved":false,"context_lines":[{"line_number":1753,"context_line":"                    return port"},{"line_number":1754,"context_line":"            LOG.debug(\"No binding found for DVR port %s\", port[\u0027id\u0027])"},{"line_number":1755,"context_line":"            return"},{"line_number":1756,"context_line":"        elif l3_hamode_db.ha_router_port(context.session, port):"},{"line_number":1757,"context_line":"            if l3_hamode_db.ha_router_bound_to_host(context.session,"},{"line_number":1758,"context_line":"                                          port[\u0027device_id\u0027], host):"},{"line_number":1759,"context_line":"                return port"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_1c7317b3","line":1756,"updated":"2016-08-08 13:47:40.000000000","message":"I\u0027d clarify (via comment/note?) here that we consider an ha-router port bound to a host if the router is bound to such host, even if it\u0027s not MASTER in such specific host.","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"ef7923fe799a05b11ff4c48ef11fae74be2b5538","unresolved":false,"context_lines":[{"line_number":1753,"context_line":"                    return port"},{"line_number":1754,"context_line":"            LOG.debug(\"No binding found for DVR port %s\", port[\u0027id\u0027])"},{"line_number":1755,"context_line":"            return"},{"line_number":1756,"context_line":"        elif l3_hamode_db.ha_router_port(context.session, port):"},{"line_number":1757,"context_line":"            if l3_hamode_db.ha_router_bound_to_host(context.session,"},{"line_number":1758,"context_line":"                                          port[\u0027device_id\u0027], host):"},{"line_number":1759,"context_line":"                return port"}],"source_content_type":"text/x-python","patch_set":33,"id":"9ad45d7e_ee03e574","line":1756,"in_reply_to":"9ad45d7e_1c7317b3","updated":"2016-08-08 15:32:57.000000000","message":"done, thanks","commit_id":"8e2787210c42fb2993b542f3adfce8b3c19856ab"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":1675,"context_line":"                                               port.port_binding.host)"},{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if l3_hamode_db.active_ha_router_bound_to_host(context.session,"},{"line_number":1678,"context_line":"                                          original_port[\u0027device_id\u0027], host):"},{"line_number":1679,"context_line":"                    port.status \u003d status"},{"line_number":1680,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1681,"context_line":"                updated_port[\u0027status\u0027] \u003d status"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_83b01bf6","line":1678,"updated":"2016-08-09 09:22:46.000000000","message":"I wonder if ha active router reporting and port reporting will race around this.","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"0bbb9cd2001831dd3abd00f290277fba8c6332e0","unresolved":false,"context_lines":[{"line_number":1675,"context_line":"                                               port.port_binding.host)"},{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if l3_hamode_db.active_ha_router_bound_to_host(context.session,"},{"line_number":1678,"context_line":"                                          original_port[\u0027device_id\u0027], host):"},{"line_number":1679,"context_line":"                    port.status \u003d status"},{"line_number":1680,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1681,"context_line":"                updated_port[\u0027status\u0027] \u003d status"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_1092989f","line":1678,"in_reply_to":"9ad45d7e_1b2da646","updated":"2016-08-09 15:22:18.000000000","message":"Thanks Miguel. In PS 37, I will be using port.port_binding.host for master HA agent, rather than checking in L3HARouterAgentPortBinding table. \n\nAgents will get notification for each port_update(with master_host). Then all HA agents will call udpate_port_status. So I don\u0027t see any races here with new change.","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"490c7a0f9694dfa43ebe58372dd36fb808525e49","unresolved":false,"context_lines":[{"line_number":1675,"context_line":"                                               port.port_binding.host)"},{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if l3_hamode_db.active_ha_router_bound_to_host(context.session,"},{"line_number":1678,"context_line":"                                          original_port[\u0027device_id\u0027], host):"},{"line_number":1679,"context_line":"                    port.status \u003d status"},{"line_number":1680,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1681,"context_line":"                updated_port[\u0027status\u0027] \u003d status"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_1b2da646","line":1678,"in_reply_to":"9ad45d7e_83b01bf6","updated":"2016-08-09 13:29:55.000000000","message":"I will check","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"a10e8ba590ad3fa0d1f4320ffbbb36f92d966351","unresolved":false,"context_lines":[{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if l3_hamode_db.active_ha_router_bound_to_host(context.session,"},{"line_number":1678,"context_line":"                                          original_port[\u0027device_id\u0027], host):"},{"line_number":1679,"context_line":"                    port.status \u003d status"},{"line_number":1680,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1681,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1682,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_4d7e52e5","line":1679,"updated":"2016-08-09 08:43:59.000000000","message":"where is this \"port  / port.status\" used/consumed later? I don\u0027t see where?","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e1511b85311142bde39e824a9645b1aa32de4a98","unresolved":false,"context_lines":[{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if l3_hamode_db.active_ha_router_bound_to_host(context.session,"},{"line_number":1678,"context_line":"                                          original_port[\u0027device_id\u0027], host):"},{"line_number":1679,"context_line":"                    port.status \u003d status"},{"line_number":1680,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1681,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1682,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_08bc4804","line":1679,"in_reply_to":"9ad45d7e_4d7e52e5","updated":"2016-08-09 08:54:31.000000000","message":"yikes, going to the database, this is in a session :]","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"36848e97f50cbf5707fa92e916de1faacadec044","unresolved":false,"context_lines":[{"line_number":1674,"context_line":"                levels \u003d db.get_binding_levels(session, port.id,"},{"line_number":1675,"context_line":"                                               port.port_binding.host)"},{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1678,"context_line":"                    port.status \u003d status"},{"line_number":1679,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1680,"context_line":"                updated_port[\u0027status\u0027] \u003d status"}],"source_content_type":"text/x-python","patch_set":37,"id":"9ad45d7e_00197c22","line":1677,"updated":"2016-08-10 10:39:25.000000000","message":"I\u0027m not fully sure it\u0027s totally race-free (are we completely sure bind will be updated before this status update, I\u0027d guess yes), we shall try it.","commit_id":"1f29aacabf945699b32b5147537223536ecc0b8f"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4d1d40e7e11574dbfd8e0a3bfc489664625eef83","unresolved":false,"context_lines":[{"line_number":1674,"context_line":"                levels \u003d db.get_binding_levels(session, port.id,"},{"line_number":1675,"context_line":"                                               port.port_binding.host)"},{"line_number":1676,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1677,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1678,"context_line":"                    port.status \u003d status"},{"line_number":1679,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1680,"context_line":"                updated_port[\u0027status\u0027] \u003d status"}],"source_content_type":"text/x-python","patch_set":37,"id":"9ad45d7e_d9fbd6bf","line":1677,"in_reply_to":"9ad45d7e_00197c22","updated":"2016-08-10 12:59:40.000000000","message":"Initially it may race. But when the master router is selected, it again calls update_port(host_id). Now the plugin will bind the port and notify agents. Agents will again call update_port_status.\n\nSo after the master is chosen, update_port and then update_pot_status happens.","commit_id":"1f29aacabf945699b32b5147537223536ecc0b8f"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"f9eecaf2db8b8bc2b62dd4a5ebbdcb4e9b476fb7","unresolved":false,"context_lines":[{"line_number":1662,"context_line":"                return None"},{"line_number":1663,"context_line":"            # HA router port is distributed port. All HA agents will wire up"},{"line_number":1664,"context_line":"            # this port and call update_port_status. But Port\u0027s binding host"},{"line_number":1665,"context_line":"            # and status in DB will be only reflecting to master router\u0027s agent"},{"line_number":1666,"context_line":"            # So we have to pass calling HA agent\u0027s host name and port status"},{"line_number":1667,"context_line":"            # to mechanism drivers so that they can complete wiring of the port"},{"line_number":1668,"context_line":"            # in agent(for example, l2pop will create flows)."}],"source_content_type":"text/x-python","patch_set":38,"id":"9ad45d7e_46ba550b","line":1665,"range":{"start_line":1665,"start_character":55,"end_line":1665,"end_character":57},"updated":"2016-08-10 17:36:50.000000000","message":"the","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"adf3472d2f62b82fc8a31801f5f5ff2d2e63aa3d","unresolved":false,"context_lines":[{"line_number":1662,"context_line":"                return None"},{"line_number":1663,"context_line":"            # HA router port is distributed port. All HA agents will wire up"},{"line_number":1664,"context_line":"            # this port and call update_port_status. But Port\u0027s binding host"},{"line_number":1665,"context_line":"            # and status in DB will be only reflecting to master router\u0027s agent"},{"line_number":1666,"context_line":"            # So we have to pass calling HA agent\u0027s host name and port status"},{"line_number":1667,"context_line":"            # to mechanism drivers so that they can complete wiring of the port"},{"line_number":1668,"context_line":"            # in agent(for example, l2pop will create flows)."}],"source_content_type":"text/x-python","patch_set":38,"id":"9ad45d7e_34cdb0b7","line":1665,"range":{"start_line":1665,"start_character":55,"end_line":1665,"end_character":57},"in_reply_to":"9ad45d7e_46ba550b","updated":"2016-08-11 06:40:59.000000000","message":"ok","commit_id":"efedfcb00ed6d1aadf4ea5448202e532f1c30da8"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":1766,"context_line":"            LOG.debug(\"No Port match for: %s\", port_id)"},{"line_number":1767,"context_line":"            return"},{"line_number":1768,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE:"},{"line_number":1769,"context_line":"            bindings \u003d db.get_distributed_port_bindings(context.session,"},{"line_number":1770,"context_line":"                                                        port_id)"},{"line_number":1771,"context_line":"            for b in bindings:"},{"line_number":1772,"context_line":"                if b.host \u003d\u003d host:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_6d136d0e","line":1769,"updated":"2016-08-24 23:37:11.000000000","message":"It seems like there\u0027s a lot of room for deduplication here, by doing something like https://review.openstack.org/#/c/323314/. I\u0027m not convinced our longer term strategy here is optimal, it looks like there\u0027s a lot of refactoring opportunity.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":1766,"context_line":"            LOG.debug(\"No Port match for: %s\", port_id)"},{"line_number":1767,"context_line":"            return"},{"line_number":1768,"context_line":"        if port[\u0027device_owner\u0027] \u003d\u003d const.DEVICE_OWNER_DVR_INTERFACE:"},{"line_number":1769,"context_line":"            bindings \u003d db.get_distributed_port_bindings(context.session,"},{"line_number":1770,"context_line":"                                                        port_id)"},{"line_number":1771,"context_line":"            for b in bindings:"},{"line_number":1772,"context_line":"                if b.host \u003d\u003d host:"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_3ba0469f","line":1769,"in_reply_to":"1ac06dbe_6d136d0e","updated":"2016-08-25 17:30:25.000000000","message":"may be later","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"85280f8f8f8472172cade923aa90ae4d0b3e0156","unresolved":false,"context_lines":[{"line_number":1775,"context_line":"            return"},{"line_number":1776,"context_line":"        elif l3_hamode_db.is_ha_router_port(context.session, port):"},{"line_number":1777,"context_line":"            # HA router port is distributed port and will be hosted by multiple"},{"line_number":1778,"context_line":"            # HA agents. So we consider all HA agents(hosting acitve and backup"},{"line_number":1779,"context_line":"            # routers) here."},{"line_number":1780,"context_line":"            if l3_hamode_db.is_ha_router_bound_to_host(context.session,"},{"line_number":1781,"context_line":"                                          port[\u0027device_id\u0027], host):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_6da9addf","line":1778,"updated":"2016-08-24 23:37:11.000000000","message":"acitve -\u003e active","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":1775,"context_line":"            return"},{"line_number":1776,"context_line":"        elif l3_hamode_db.is_ha_router_port(context.session, port):"},{"line_number":1777,"context_line":"            # HA router port is distributed port and will be hosted by multiple"},{"line_number":1778,"context_line":"            # HA agents. So we consider all HA agents(hosting acitve and backup"},{"line_number":1779,"context_line":"            # routers) here."},{"line_number":1780,"context_line":"            if l3_hamode_db.is_ha_router_bound_to_host(context.session,"},{"line_number":1781,"context_line":"                                          port[\u0027device_id\u0027], host):"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_3bc96679","line":1778,"in_reply_to":"1ac06dbe_6da9addf","updated":"2016-08-25 17:30:25.000000000","message":"done.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"225d336f2f1329a23bc51f272828140c75104aa8","unresolved":false,"context_lines":[{"line_number":1680,"context_line":"            # agent. So we have to pass calling HA agent\u0027s host name and port"},{"line_number":1681,"context_line":"            # status to mechanism drivers so that they can complete wiring of"},{"line_number":1682,"context_line":"            # the port in agent(for example, l2pop will create flows)."},{"line_number":1683,"context_line":"            if l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":1684,"context_line":"                                              port[\u0027device_id\u0027]):"},{"line_number":1685,"context_line":"                original_port \u003d self._make_port_dict(port)"},{"line_number":1686,"context_line":"                network \u003d network or self.get_network("}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_cec76496","line":1683,"updated":"2016-08-29 20:55:37.000000000","message":"I don\u0027t think we actually need a different block for HA and non-HA, it seems like a lot of lines are repeated and it makes this already complex function even more difficult to reason about. As far as I could tell the only difference is lines 1692 to 1695. We could put those in the block that starts in line 1701 and just put them under the conditional you have in line 1683.","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"8212eb96b012205075639b602750a4cb26b888a3","unresolved":false,"context_lines":[{"line_number":1680,"context_line":"            # agent. So we have to pass calling HA agent\u0027s host name and port"},{"line_number":1681,"context_line":"            # status to mechanism drivers so that they can complete wiring of"},{"line_number":1682,"context_line":"            # the port in agent(for example, l2pop will create flows)."},{"line_number":1683,"context_line":"            if l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":1684,"context_line":"                                              port[\u0027device_id\u0027]):"},{"line_number":1685,"context_line":"                original_port \u003d self._make_port_dict(port)"},{"line_number":1686,"context_line":"                network \u003d network or self.get_network("}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_03cc7ab9","line":1683,"in_reply_to":"fa7ab95a_a82f6286","updated":"2016-08-30 19:09:23.000000000","message":"I will submit a separate patch for cleaning up common code in this function.","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"779b47e62b70738d6a192c36879f724d1b08979c","unresolved":false,"context_lines":[{"line_number":1680,"context_line":"            # agent. So we have to pass calling HA agent\u0027s host name and port"},{"line_number":1681,"context_line":"            # status to mechanism drivers so that they can complete wiring of"},{"line_number":1682,"context_line":"            # the port in agent(for example, l2pop will create flows)."},{"line_number":1683,"context_line":"            if l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":1684,"context_line":"                                              port[\u0027device_id\u0027]):"},{"line_number":1685,"context_line":"                original_port \u003d self._make_port_dict(port)"},{"line_number":1686,"context_line":"                network \u003d network or self.get_network("}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_a82f6286","line":1683,"in_reply_to":"fa7ab95a_cec76496","updated":"2016-08-30 06:02:33.000000000","message":"ok","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"225d336f2f1329a23bc51f272828140c75104aa8","unresolved":false,"context_lines":[{"line_number":1781,"context_line":"            # routers) here."},{"line_number":1782,"context_line":"            l3plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":1783,"context_line":"                service_constants.L3_ROUTER_NAT)"},{"line_number":1784,"context_line":"            if l3plugin and l3plugin.list_router_ids_on_host("},{"line_number":1785,"context_line":"                context, host, [port[\u0027device_id\u0027]]):"},{"line_number":1786,"context_line":"                return port"},{"line_number":1787,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_54ae90e6","line":1784,"updated":"2016-08-29 20:55:37.000000000","message":"This will raise an exception if there\u0027s no L3 agent on the host. Can this be an issue with environments that don\u0027t use the L3 agent at all? The user would have to create a port with a device_owner of DEVICE_OWNER_HA_REPLICATED_INT and on a host without a L3 agent, so I don\u0027t think this is a real concern.","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f33ecddd0e3ded5cf2565344111faf311009843","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"    def _setup_dhcp_agent_provisioning_component(self, context, port):"},{"line_number":1190,"context_line":"        # NOTE(kevinbenton): skipping network ports is a workaround for"},{"line_number":1191,"context_line":"        # the fact that we don\u0027t issue dhcp notifications from internal"},{"line_number":1192,"context_line":"        # port creation like router ports and dhcp ports via RPC."},{"line_number":1193,"context_line":"        if utils.is_port_trusted(port):"},{"line_number":1194,"context_line":"            return"},{"line_number":1195,"context_line":"        subnet_ids \u003d [f[\u0027subnet_id\u0027] for f in port[\u0027fixed_ips\u0027]]"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_21f22f70","line":1192,"updated":"2016-08-31 11:10:51.000000000","message":"unrelated change","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"553b1716d87e067462dff2934deefd5d366ac6a3","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"    def _setup_dhcp_agent_provisioning_component(self, context, port):"},{"line_number":1190,"context_line":"        # NOTE(kevinbenton): skipping network ports is a workaround for"},{"line_number":1191,"context_line":"        # the fact that we don\u0027t issue dhcp notifications from internal"},{"line_number":1192,"context_line":"        # port creation like router ports and dhcp ports via RPC."},{"line_number":1193,"context_line":"        if utils.is_port_trusted(port):"},{"line_number":1194,"context_line":"            return"},{"line_number":1195,"context_line":"        subnet_ids \u003d [f[\u0027subnet_id\u0027] for f in port[\u0027fixed_ips\u0027]]"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_b3aafd83","line":1192,"in_reply_to":"fa7ab95a_21f22f70","updated":"2016-08-31 16:53:37.000000000","message":"I will remove","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f33ecddd0e3ded5cf2565344111faf311009843","unresolved":false,"context_lines":[{"line_number":1687,"context_line":"                LOG.debug(\"Port %(port)s update to %(val)s by agent not found\","},{"line_number":1688,"context_line":"                          {\u0027port\u0027: port_id, \u0027val\u0027: status})"},{"line_number":1689,"context_line":"                return None"},{"line_number":1690,"context_line":"            # HA router port is distributed port. All HA agents will wire up"},{"line_number":1691,"context_line":"            # this port and call update_port_status. But Port\u0027s binding host"},{"line_number":1692,"context_line":"            # and status in DB will be only reflecting the master router\u0027s"},{"line_number":1693,"context_line":"            # agent. So we have to pass calling HA agent\u0027s host name and port"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_41e733ac","line":1690,"updated":"2016-08-31 11:10:51.000000000","message":"this is none of the business of this function. the special handling of DVR interfaces is bad and we don\u0027t want to propagate this pattern into a bunch more branching that makes a function called \u0027update_port_status\u0027 silently discard the status update!","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"553b1716d87e067462dff2934deefd5d366ac6a3","unresolved":false,"context_lines":[{"line_number":1687,"context_line":"                LOG.debug(\"Port %(port)s update to %(val)s by agent not found\","},{"line_number":1688,"context_line":"                          {\u0027port\u0027: port_id, \u0027val\u0027: status})"},{"line_number":1689,"context_line":"                return None"},{"line_number":1690,"context_line":"            # HA router port is distributed port. All HA agents will wire up"},{"line_number":1691,"context_line":"            # this port and call update_port_status. But Port\u0027s binding host"},{"line_number":1692,"context_line":"            # and status in DB will be only reflecting the master router\u0027s"},{"line_number":1693,"context_line":"            # agent. So we have to pass calling HA agent\u0027s host name and port"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_d3d5c1b9","line":1690,"in_reply_to":"fa7ab95a_41e733ac","updated":"2016-08-31 16:53:37.000000000","message":"We can cleanup both DVR and HA code here in [1]. Till [1] is mature enough, can we continue with this change? It is needed for l2pop changes in this patch.\n\n[1] https://review.openstack.org/#/c/363777/1/neutron/plugins/ml2/plugin.py@1741","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"70a3adfe06f3ec2d32729dbc8847bbd0201ff6e0","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"                                               port.port_binding.host)"},{"line_number":1704,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1705,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1706,"context_line":"                    port.status \u003d status"},{"line_number":1707,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1708,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1709,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_b7a3acb0","line":1706,"updated":"2016-09-01 04:27:59.000000000","message":"Let me describe what I understand happening here.\n\nIn a function called \u0027update_port_status\u0027, we are silently refusing to update a port\u0027s status if the incorrect host value is set. However, we still lie to all of the mechanism drivers by emitting a precommit event with a port context that implies that the status was indeed changed and additionally that the host binding has changed (L1707-L1708).\n\nFaking changes can easily break other mechanism drivers that record these events into separate database tables because they will now be inconsistent with what really happened to the port object. This approach is unsustainable IMO and can have unintended side effects. \n\nIf we need to take a shortcut until we have an event semantic we agree on for the dataplane event, I would rather see the RPC code call directly into the L2pop driver directly than try to maintain this because that\u0027s much more explicit about the hackiness and is easier to cleanup later.","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"8265e08f7950a122751e841aab9aab32f5b2dcc9","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"                                               port.port_binding.host)"},{"line_number":1704,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1705,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1706,"context_line":"                    port.status \u003d status"},{"line_number":1707,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1708,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1709,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_9db969ef","line":1706,"in_reply_to":"fa7ab95a_5d1d2165","updated":"2016-09-01 05:37:11.000000000","message":"almost, except don\u0027t construct a new driver. Just find the existing instance by getting a reference to the core plugin and then iterating through the mech drivers to find the l2pop one.","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"50d49c7e2b27727705731762334a6d44cdbc0eb1","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"                                               port.port_binding.host)"},{"line_number":1704,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1705,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1706,"context_line":"                    port.status \u003d status"},{"line_number":1707,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1708,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1709,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_5d76013f","line":1706,"in_reply_to":"fa7ab95a_9db969ef","updated":"2016-09-01 05:39:26.000000000","message":"sure, thanks Kevin","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"6155ce47e17ae9a27287759d99f967b0af765104","unresolved":false,"context_lines":[{"line_number":1703,"context_line":"                                               port.port_binding.host)"},{"line_number":1704,"context_line":"                # Only allow master HA agent to update port status in DB"},{"line_number":1705,"context_line":"                if port.port_binding.host \u003d\u003d host:"},{"line_number":1706,"context_line":"                    port.status \u003d status"},{"line_number":1707,"context_line":"                updated_port[portbindings.HOST_ID] \u003d host"},{"line_number":1708,"context_line":"                updated_port[\u0027status\u0027] \u003d status"},{"line_number":1709,"context_line":"                mech_context \u003d (driver_context.PortContext("}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_5d1d2165","line":1706,"in_reply_to":"fa7ab95a_b7a3acb0","updated":"2016-09-01 05:23:57.000000000","message":"When you say - \"RPC code call directly into the L2pop driver directly\" in ml2/rpc.py - do you mean\n\ndef update_device_up(self, rpc_context, **kwargs):\n  provisioning_blocks.provisioning_complete(...)\n  port_context \u003d plugin.get_bound_port_context()\n  l2pop_driver \u003d mech_driver.L2populationMechanismDriver()\n  l2pop_driver._update_port_up(self, context)","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"}],"neutron/plugins/ml2/rpc.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"225d336f2f1329a23bc51f272828140c75104aa8","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                        n_const.DEVICE_OWNER_COMPUTE_PREFIX):"},{"line_number":224,"context_line":"                    plugin.nova_notifier.notify_port_active_direct(port)"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":"        if port and (port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":227,"context_line":"            or l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":228,"context_line":"                                              port[\u0027device_id\u0027])):"},{"line_number":229,"context_line":"            # NOTE(kevinbenton): we have to special case DVR ports because of"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_11a25a4b","line":226,"updated":"2016-08-29 20:55:37.000000000","message":"This means that for HA router ports we won\u0027t execute line 243. I believe for DVR ports it\u0027s OK because we don\u0027t provision the block and so we don\u0027t need to release it. However according to my local test, we do provision for HA router ports, and now we won\u0027t unblock it, meaning that we\u0027ll add more and more records to the provisioning table and never delete them. Can you please look in to this?","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"a0e9762c995a40a18655b61132d82372d5a7951b","unresolved":false,"context_lines":[{"line_number":223,"context_line":"                        n_const.DEVICE_OWNER_COMPUTE_PREFIX):"},{"line_number":224,"context_line":"                    plugin.nova_notifier.notify_port_active_direct(port)"},{"line_number":225,"context_line":"            return"},{"line_number":226,"context_line":"        if port and (port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":227,"context_line":"            or l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":228,"context_line":"                                              port[\u0027device_id\u0027])):"},{"line_number":229,"context_line":"            # NOTE(kevinbenton): we have to special case DVR ports because of"}],"source_content_type":"text/x-python","patch_set":48,"id":"fa7ab95a_6d3e4c29","line":226,"in_reply_to":"fa7ab95a_11a25a4b","updated":"2016-08-30 05:53:15.000000000","message":"This change was needed before this patch [1]. Now with [1], we don\u0027t need this. I will remove it.\n\n[1] https://review.openstack.org/#/c/346323/","commit_id":"425873387b1f3cbf7e88d13909facfe15f712d2b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f33ecddd0e3ded5cf2565344111faf311009843","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            is_ha_port \u003d l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":228,"context_line":"                                                        port[\u0027device_id\u0027])"},{"line_number":229,"context_line":"        if port and (port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":230,"context_line":"             or is_ha_port):"},{"line_number":231,"context_line":"            # For HA router, provision block is added when l3_rpc.py calls"},{"line_number":232,"context_line":"            # update_port with HOST_ID, so remove here as provisioning is"},{"line_number":233,"context_line":"            # completed."}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_61ec3790","line":230,"updated":"2016-08-31 11:10:51.000000000","message":"please try not to modify this function or update_port_status if possible. can you take a look at https://review.openstack.org/363582 and see if L2pop or the ML2 code can subscribe functions to those new events that will mean you don\u0027t have to pollute the update_port_status method?","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"553b1716d87e067462dff2934deefd5d366ac6a3","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            is_ha_port \u003d l3_hamode_db.is_ha_router_port(port[\u0027device_owner\u0027],"},{"line_number":228,"context_line":"                                                        port[\u0027device_id\u0027])"},{"line_number":229,"context_line":"        if port and (port[\u0027device_owner\u0027] \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":230,"context_line":"             or is_ha_port):"},{"line_number":231,"context_line":"            # For HA router, provision block is added when l3_rpc.py calls"},{"line_number":232,"context_line":"            # update_port with HOST_ID, so remove here as provisioning is"},{"line_number":233,"context_line":"            # completed."}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_c1340a4c","line":230,"in_reply_to":"fa7ab95a_61ec3790","updated":"2016-08-31 16:53:37.000000000","message":"For DVR case, it writes every port update to DB because it has a separate DB entry for each agent in DistributedPortBinding. And l2pop has to be notified only after the changes made to DB. So for DVR, notifying l2pop should happen after [1]. And [2] is the correct place to notify l2pop about port wiring. Is it worth to move to DATAPLANE events for just notification [3]. If so, we can cleanup this later when we move to DATAPLANE events. In this change, we are following existing DVR approach i.e call update_port_status with host and then update_port_status notify l2pop. Later we will clean both DVR and HA code we are adding here.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1714\n[2] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1737\n[3] https://review.openstack.org/#/c/363777/1/neutron/plugins/ml2/plugin.py@216","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f33ecddd0e3ded5cf2565344111faf311009843","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            # update_port with HOST_ID, so remove here as provisioning is"},{"line_number":233,"context_line":"            # completed."},{"line_number":234,"context_line":"            if is_ha_port:"},{"line_number":235,"context_line":"                provisioning_blocks.remove_provisioning_component("},{"line_number":236,"context_line":"                    rpc_context, port[\u0027id\u0027], resources.PORT,"},{"line_number":237,"context_line":"                    provisioning_blocks.L2_AGENT_ENTITY)"},{"line_number":238,"context_line":"            # NOTE(kevinbenton): we have to handle special case DVR/HA ports"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_81013b3c","line":235,"updated":"2016-08-31 11:10:51.000000000","message":"This doesn\u0027t serve a purpose if you are setting the status yourself anyway. Provisioning components are just a way to trigger a status change after they are all completed. if you are forcing a status directly, you can just completely ignore them.","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"553b1716d87e067462dff2934deefd5d366ac6a3","unresolved":false,"context_lines":[{"line_number":232,"context_line":"            # update_port with HOST_ID, so remove here as provisioning is"},{"line_number":233,"context_line":"            # completed."},{"line_number":234,"context_line":"            if is_ha_port:"},{"line_number":235,"context_line":"                provisioning_blocks.remove_provisioning_component("},{"line_number":236,"context_line":"                    rpc_context, port[\u0027id\u0027], resources.PORT,"},{"line_number":237,"context_line":"                    provisioning_blocks.L2_AGENT_ENTITY)"},{"line_number":238,"context_line":"            # NOTE(kevinbenton): we have to handle special case DVR/HA ports"}],"source_content_type":"text/x-python","patch_set":51,"id":"fa7ab95a_131d690f","line":235,"in_reply_to":"fa7ab95a_81013b3c","updated":"2016-08-31 16:53:37.000000000","message":"As explained in the comment at line 231, update_port(with HOST_ID) is called for only HA router port, and a provision block is added for this. We have to remove that here otherwise this provision block remain in DB as stale till port is removed. DVR calls update_distributed_port_binding [1], which doesn\u0027t use update_port and directly modifies DB. So provision block is not added for it.\nAlso see my comment [2] i.e if provisioning_complete at line 252 fails, should we still notify DATAPLANE_ACTIVE event.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/api/rpc/handlers/l3_rpc.py#L197\n[2] https://review.openstack.org/#/c/363777/1/neutron/plugins/ml2/rpc.py@261","commit_id":"3d4b51084dfae9c18ef30dd029c2e31762c77d43"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d8f57bdf8716bdbba7f4473e3358ed4eac25abd8","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                rpc_context, port[\u0027id\u0027], resources.PORT,"},{"line_number":253,"context_line":"                provisioning_blocks.L2_AGENT_ENTITY)"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def notify_ha_port_status(self, port_id, rpc_context, status, host):"},{"line_number":256,"context_line":"        port \u003d ml2_db.get_port(rpc_context.session, port_id)"},{"line_number":257,"context_line":"        if not port:"},{"line_number":258,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_fc026d3d","line":255,"updated":"2016-09-01 09:35:28.000000000","message":"can you add port\u003dNone to the signature and then have L256 only do a port lookup if it\u0027s None? That avoids the double-lookup in the update_device_up case.","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4ed47becdd91aff86754a72ac1d90dae3fa67820","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                rpc_context, port[\u0027id\u0027], resources.PORT,"},{"line_number":253,"context_line":"                provisioning_blocks.L2_AGENT_ENTITY)"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def notify_ha_port_status(self, port_id, rpc_context, status, host):"},{"line_number":256,"context_line":"        port \u003d ml2_db.get_port(rpc_context.session, port_id)"},{"line_number":257,"context_line":"        if not port:"},{"line_number":258,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_9f4dd39b","line":255,"in_reply_to":"fa7ab95a_fc026d3d","updated":"2016-09-01 09:50:54.000000000","message":"sure","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d8f57bdf8716bdbba7f4473e3358ed4eac25abd8","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                    rpc_context, port_id)"},{"line_number":265,"context_line":"            port_context.current[\u0027status\u0027] \u003d status"},{"line_number":266,"context_line":"            port_context.current[portbindings.HOST_ID] \u003d host"},{"line_number":267,"context_line":"            l2pop_driver \u003d plugin.mechanism_manager.mech_drivers["},{"line_number":268,"context_line":"                \u0027l2population\u0027].obj"},{"line_number":269,"context_line":"            if status \u003d\u003d n_const.PORT_STATUS_ACTIVE:"},{"line_number":270,"context_line":"                l2pop_driver.update_port_up(port_context)"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_bc6d4506","line":267,"updated":"2016-09-01 09:35:28.000000000","message":"you need to check that \u0027l2population\u0027 is a loaded driver. Preferably do this right at the start of the function before any DB lookups so it\u0027s not impacting performance if it\u0027s not loaded.","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4ed47becdd91aff86754a72ac1d90dae3fa67820","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                    rpc_context, port_id)"},{"line_number":265,"context_line":"            port_context.current[\u0027status\u0027] \u003d status"},{"line_number":266,"context_line":"            port_context.current[portbindings.HOST_ID] \u003d host"},{"line_number":267,"context_line":"            l2pop_driver \u003d plugin.mechanism_manager.mech_drivers["},{"line_number":268,"context_line":"                \u0027l2population\u0027].obj"},{"line_number":269,"context_line":"            if status \u003d\u003d n_const.PORT_STATUS_ACTIVE:"},{"line_number":270,"context_line":"                l2pop_driver.update_port_up(port_context)"}],"source_content_type":"text/x-python","patch_set":53,"id":"fa7ab95a_9f34f3fb","line":267,"in_reply_to":"fa7ab95a_bc6d4506","updated":"2016-09-01 09:50:54.000000000","message":"sure","commit_id":"2e00eb89b7364377580bcc7bac2dd3df3ed38e48"}],"neutron/tests/unit/plugins/ml2/drivers/l2pop/test_db.py":[{"author":{"_account_id":17958,"name":"NGUYEN TUONG THANH","email":"ngtthanh1010@gmail.com","username":"thanhnt"},"change_message_id":"f016ef522b826f173cb82745cfafff98994f41bb","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HOST \u003d helpers.HOST"},{"line_number":31,"context_line":"HOST_2 \u003d \u0027HOST\u0027 + \u0027_2\u0027"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"MAC \u003d \u002700:11:22:33:44:55\u0027"},{"line_number":34,"context_line":"MAC_1 \u003d \u002711:22:33:44:55:66\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_68b914cb","line":31,"updated":"2016-01-08 08:39:42.000000000","message":"Why don\u0027t you use \u0027HOST_2\" here? could you please help me to explain? thanks","commit_id":"63c6f54a9e9d51f58e0c5285af5c05d49e8d5172"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"d5389ed9f5764be7752774ed4c1e0aa2c1fab0b8","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"HOST \u003d helpers.HOST"},{"line_number":31,"context_line":"HOST_2 \u003d \u0027HOST\u0027 + \u0027_2\u0027"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"MAC \u003d \u002700:11:22:33:44:55\u0027"},{"line_number":34,"context_line":"MAC_1 \u003d \u002711:22:33:44:55:66\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa69d971_9e59d437","line":31,"in_reply_to":"fa69d971_68b914cb","updated":"2016-01-11 09:42:31.000000000","message":"sure thanks, I will do that in my next patchset.","commit_id":"63c6f54a9e9d51f58e0c5285af5c05d49e8d5172"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"cac87ec42e81816f37db5d58eb85146005c8dd5e","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                self.ctx.session.add(models_v2.Network(id\u003dnetwork_id))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_router_port(self):"},{"line_number":98,"context_line":"        # Create a HA router, add two l3 agents and l2 agents to HOST"},{"line_number":99,"context_line":"        # and HOST_2. Creat HA network and add HA network ports for"},{"line_number":100,"context_line":"        # the router on HOST and HOST_2. Also add router interface port"},{"line_number":101,"context_line":"        # on HOST."}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_fff6345b","line":98,"updated":"2016-05-11 14:46:27.000000000","message":"Docstring? :)","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"3476464770cca53a7dfeea1727ce964391b80674","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                self.ctx.session.add(models_v2.Network(id\u003dnetwork_id))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_router_port(self):"},{"line_number":98,"context_line":"        # Create a HA router, add two l3 agents and l2 agents to HOST"},{"line_number":99,"context_line":"        # and HOST_2. Creat HA network and add HA network ports for"},{"line_number":100,"context_line":"        # the router on HOST and HOST_2. Also add router interface port"},{"line_number":101,"context_line":"        # on HOST."}],"source_content_type":"text/x-python","patch_set":26,"id":"dab17558_c73c55e3","line":98,"in_reply_to":"dab17558_fff6345b","updated":"2016-05-17 09:36:09.000000000","message":"done","commit_id":"87b1a48dadcee7df07adca79f3acbfd57f52b9bf"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"5dfaada7e17cb9cb6e9523dce504e4165f06c624","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            except exc.NoResultFound:"},{"line_number":95,"context_line":"                self.ctx.session.add(models_v2.Network(id\u003dnetwork_id))"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def _setup_router_port(self):"},{"line_number":98,"context_line":"        \"\"\"Create HA router"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        Create a HA router, add two l3 agents and l2 agents to HOST"}],"source_content_type":"text/x-python","patch_set":27,"id":"bab6814e_41ac95ed","line":97,"updated":"2016-05-18 09:00:55.000000000","message":"if you need to respin:\n\n_setup_router_port means that you create a router port, not a router, and a port.\n\n_setup_router_and_port would be more accurate.","commit_id":"92e86e1a3a95939d83b9feb22642ea97ce6a360a"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":29,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"HOST \u003d helpers.HOST"},{"line_number":32,"context_line":"HOST_2 \u003d \u0027HOST_2\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_a3839fcb","line":32,"updated":"2016-08-09 09:22:46.000000000","message":"just a big nit:\n\nCreate constants for all the string literals that you repeat around the tests (as values). They\u0027re easier to change if needed just in one place later, and also, python will catch any typos when using the constants.","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"490c7a0f9694dfa43ebe58372dd36fb808525e49","unresolved":false,"context_lines":[{"line_number":29,"context_line":"from neutron.tests.unit import testlib_api"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"HOST \u003d helpers.HOST"},{"line_number":32,"context_line":"HOST_2 \u003d \u0027HOST_2\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_dbc94e7a","line":32,"in_reply_to":"9ad45d7e_a3839fcb","updated":"2016-08-09 13:29:55.000000000","message":"done at all paces in PS36","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _create_router(self, distributed\u003dTrue, ha\u003dFalse):"},{"line_number":47,"context_line":"        with self.ctx.session.begin(subtransactions\u003dTrue):"},{"line_number":48,"context_line":"            self.ctx.session.add(l3_db.Router(id\u003d\u0027router_id\u0027))"},{"line_number":49,"context_line":"            self.ctx.session.add(l3_attrs_db.RouterExtraAttributes("},{"line_number":50,"context_line":"                router_id\u003d\u0027router_id\u0027, distributed\u003ddistributed, ha\u003dha))"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_6304077e","line":48,"updated":"2016-08-09 09:22:46.000000000","message":"please use a constant at the top of the module\nTEST_ROUTER_ID","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        helpers.register_l3_agent(HOST_2)"},{"line_number":54,"context_line":"        helpers.register_ovs_agent(HOST_2, tunneling_ip\u003d\u002720.0.0.2\u0027)"},{"line_number":55,"context_line":"        with self.ctx.session.begin(subtransactions\u003dTrue):"},{"line_number":56,"context_line":"            self.ctx.session.add(models_v2.Network(id\u003d\u0027ha_network_id\u0027))"},{"line_number":57,"context_line":"            self._create_router(distributed\u003ddistributed, ha\u003dTrue)"},{"line_number":58,"context_line":"            for state, host in [(\u0027active\u0027, HOST), (\u0027standby\u0027, HOST_2)]:"},{"line_number":59,"context_line":"                self._setup_port_binding("}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_a3feff68","line":56,"updated":"2016-08-09 09:22:46.000000000","message":"TEST_HA_NETWORK_ID","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        with self.ctx.session.begin(subtransactions\u003dTrue):"},{"line_number":88,"context_line":"            mac \u003d utils.get_random_mac(\u0027fa:16:3e:00:00:00\u0027.split(\u0027:\u0027))"},{"line_number":89,"context_line":"            port_id \u003d uuidutils.generate_uuid()"},{"line_number":90,"context_line":"            network_id \u003d kwargs.get(\u0027network_id\u0027, \u0027network_id\u0027)"},{"line_number":91,"context_line":"            device_owner \u003d kwargs.get(\u0027device_owner\u0027, \u0027\u0027)"},{"line_number":92,"context_line":"            device_id \u003d kwargs.get(\u0027device_id\u0027, \u0027\u0027)"},{"line_number":93,"context_line":"            host \u003d kwargs.get(\u0027host\u0027, helpers.HOST)"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_632b67ec","line":90,"range":{"start_line":90,"start_character":51,"end_line":90,"end_character":60},"updated":"2016-08-09 09:22:46.000000000","message":"TEST_NETWORK_ID","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":110,"context_line":""},{"line_number":111,"context_line":"            self.ctx.session.add(port_binding_cls(**binding_kwarg))"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            if network_id \u003d\u003d \u0027ha_network_id\u0027:"},{"line_number":114,"context_line":"                agent \u003d self.get_l3_agent_by_host(host)"},{"line_number":115,"context_line":"                haport_bindings_cls \u003d l3_hamode_db.L3HARouterAgentPortBinding"},{"line_number":116,"context_line":"                habinding_kwarg \u003d {\u0027port_id\u0027: port_id,"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_e33f5723","line":113,"updated":"2016-08-09 09:22:46.000000000","message":"constant","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self._create_router(ha\u003dTrue)"},{"line_number":168,"context_line":"        self._setup_port_binding("},{"line_number":169,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_SNAT,"},{"line_number":170,"context_line":"            device_id\u003d\u0027router_id\u0027)"},{"line_number":171,"context_line":"        ha_iface_ids \u003d l2pop_db._get_ha_router_interface_ids("},{"line_number":172,"context_line":"            self.ctx.session, \u0027network_id\u0027)"},{"line_number":173,"context_line":"        self.assertEqual(1, len(ha_iface_ids))"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_234a0fc6","line":170,"updated":"2016-08-09 09:22:46.000000000","message":"constant","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f52df25e712b9b437e4cc60937829d75ac34c8de","unresolved":false,"context_lines":[{"line_number":169,"context_line":"            device_owner\u003dconstants.DEVICE_OWNER_ROUTER_SNAT,"},{"line_number":170,"context_line":"            device_id\u003d\u0027router_id\u0027)"},{"line_number":171,"context_line":"        ha_iface_ids \u003d l2pop_db._get_ha_router_interface_ids("},{"line_number":172,"context_line":"            self.ctx.session, \u0027network_id\u0027)"},{"line_number":173,"context_line":"        self.assertEqual(1, len(ha_iface_ids))"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def test__get_ha_router_interface_ids_with_ha_replicated_port(self):"}],"source_content_type":"text/x-python","patch_set":34,"id":"9ad45d7e_03470bbc","line":172,"updated":"2016-08-09 09:22:46.000000000","message":"constant","commit_id":"f4d627b79ff05b80f087546ca8b1c89fcf28c870"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"36848e97f50cbf5707fa92e916de1faacadec044","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        with self.ctx.session.begin(subtransactions\u003dTrue):"},{"line_number":94,"context_line":"            mac \u003d utils.get_random_mac(\u0027fa:16:3e:00:00:00\u0027.split(\u0027:\u0027))"},{"line_number":95,"context_line":"            port_id \u003d uuidutils.generate_uuid()"},{"line_number":96,"context_line":"            network_id \u003d kwargs.get(TEST_NETWORK_ID, TEST_NETWORK_ID)"},{"line_number":97,"context_line":"            device_owner \u003d kwargs.get(\u0027device_owner\u0027, \u0027\u0027)"},{"line_number":98,"context_line":"            device_id \u003d kwargs.get(\u0027device_id\u0027, \u0027\u0027)"},{"line_number":99,"context_line":"            host \u003d kwargs.get(\u0027host\u0027, helpers.HOST)"}],"source_content_type":"text/x-python","patch_set":37,"id":"9ad45d7e_402ed443","line":96,"updated":"2016-08-10 10:39:25.000000000","message":"ok, left side of the get parameters is not exactly the same, ask yourself this:\n\nCan you change TEST_NETWORK_ID to something else, and does it all keep working? \n\nleft side is the attribute of the port, right side is the value you\u0027re using.\n\nPlease set the left sides (here (L96), L114,  L123,  back to the literal)\n\nThere could be others, I read it horizontally.","commit_id":"1f29aacabf945699b32b5147537223536ecc0b8f"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"4d1d40e7e11574dbfd8e0a3bfc489664625eef83","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        with self.ctx.session.begin(subtransactions\u003dTrue):"},{"line_number":94,"context_line":"            mac \u003d utils.get_random_mac(\u0027fa:16:3e:00:00:00\u0027.split(\u0027:\u0027))"},{"line_number":95,"context_line":"            port_id \u003d uuidutils.generate_uuid()"},{"line_number":96,"context_line":"            network_id \u003d kwargs.get(TEST_NETWORK_ID, TEST_NETWORK_ID)"},{"line_number":97,"context_line":"            device_owner \u003d kwargs.get(\u0027device_owner\u0027, \u0027\u0027)"},{"line_number":98,"context_line":"            device_id \u003d kwargs.get(\u0027device_id\u0027, \u0027\u0027)"},{"line_number":99,"context_line":"            host \u003d kwargs.get(\u0027host\u0027, helpers.HOST)"}],"source_content_type":"text/x-python","patch_set":37,"id":"9ad45d7e_7923caa7","line":96,"in_reply_to":"9ad45d7e_402ed443","updated":"2016-08-10 12:59:40.000000000","message":"sure :)","commit_id":"1f29aacabf945699b32b5147537223536ecc0b8f"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ab1ed0f4024406ffb49632a3387047cb90fc4a1","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def setUp(self):"},{"line_number":45,"context_line":"        super(TestL2PopulationDBTestCase, self).setUp()"},{"line_number":46,"context_line":"        self.ctx \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_a3ba9b08","line":43,"updated":"2016-08-24 19:29:36.000000000","message":"nit: Don\u0027t change whitespace willy nilly.","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"},{"author":{"_account_id":10267,"name":"venkata anil","email":"anilvenkata@redhat.com","username":"anil"},"change_message_id":"374534945576317a3f0041bc3dc3eda498377abf","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"class TestL2PopulationDBTestCase(testlib_api.SqlTestCase):"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def setUp(self):"},{"line_number":45,"context_line":"        super(TestL2PopulationDBTestCase, self).setUp()"},{"line_number":46,"context_line":"        self.ctx \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":45,"id":"1ac06dbe_1bd74aee","line":43,"in_reply_to":"1ac06dbe_a3ba9b08","updated":"2016-08-25 17:30:25.000000000","message":"done","commit_id":"9fcec5a9dffbabf2ec631d1590ec986755e0b5ab"}]}