)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"afe272832efc27cd6644cacef730eb248f685156","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is a heavy work in progress, but it seems to be the"},{"line_number":13,"context_line":"bare minimum to try out the feature:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* get client extensions from I6fe1dbd81813fae234801a61c0e3d89f9e7c791e"},{"line_number":16,"context_line":"* enable the \u0027trunk\u0027 service plugin"},{"line_number":17,"context_line":"* make a port, turn it into a trunk, attach it to a VM (or boot a VM with it)"},{"line_number":18,"context_line":"* add subports and configure your guest on the corresponding VLAN"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9ad45d7e_5e517f35","line":15,"updated":"2016-08-10 16:06:49.000000000","message":"this has merged.","commit_id":"87fa1ec6d319dd59a324e3cc5202a195dd41bb0d"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is a heavy work in progress, but it seems to be the"},{"line_number":13,"context_line":"bare minimum to try out the feature:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* get client extensions from I6fe1dbd81813fae234801a61c0e3d89f9e7c791e"},{"line_number":16,"context_line":"* enable the \u0027trunk\u0027 service plugin"},{"line_number":17,"context_line":"* make a port, turn it into a trunk, attach it to a VM (or boot a VM with it)"},{"line_number":18,"context_line":"* add subports and configure your guest on the corresponding VLAN"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"fa7ab95a_a2706c73","line":15,"updated":"2016-09-02 13:26:29.000000000","message":"Done","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":12,"context_line":"This is a heavy work in progress, but it seems to be the"},{"line_number":13,"context_line":"bare minimum to try out the feature:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* get client extensions from I6fe1dbd81813fae234801a61c0e3d89f9e7c791e"},{"line_number":16,"context_line":"* enable the \u0027trunk\u0027 service plugin"},{"line_number":17,"context_line":"* make a port, turn it into a trunk, attach it to a VM (or boot a VM with it)"},{"line_number":18,"context_line":"* add subports and configure your guest on the corresponding VLAN"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"1ac06dbe_f44d6f9a","line":15,"updated":"2016-08-24 18:50:47.000000000","message":"this has merged.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"}],"neutron/plugins/ml2/drivers/agent/_common_agent.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"4384f53900511abf9d6bd550bc384db36ea174ba","unresolved":false,"context_lines":[{"line_number":40,"context_line":"from neutron.common import topics"},{"line_number":41,"context_line":"from neutron import context"},{"line_number":42,"context_line":"from neutron.plugins.ml2.drivers.agent import _agent_manager_base as amb"},{"line_number":43,"context_line":"from neutron.plugins.ml2.drivers.agent import capabilities"},{"line_number":44,"context_line":"from neutron.plugins.ml2.drivers.agent import config as cagt_config  # noqa"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9ad45d7e_323ddfc4","line":43,"updated":"2016-08-10 16:08:19.000000000","message":"this would need to depend on https://review.openstack.org/#/c/350784/, no?","commit_id":"87fa1ec6d319dd59a324e3cc5202a195dd41bb0d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":106,"context_line":"                self._report_state)"},{"line_number":107,"context_line":"            heartbeat.start(interval\u003dreport_interval)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        capabilities.notify_init_event(self.agent_type, self)"},{"line_number":110,"context_line":"        # The initialization is complete; we can start receiving messages"},{"line_number":111,"context_line":"        self.connection.consume_in_threads()"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_f7857159","line":109,"updated":"2016-08-24 18:50:47.000000000","message":"I assume you considered having this here rather than in [1] to allow other agents to pick up the event notification?\n\n[1] https://review.openstack.org/#/c/346377/8/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py@946","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":106,"context_line":"                self._report_state)"},{"line_number":107,"context_line":"            heartbeat.start(interval\u003dreport_interval)"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        capabilities.notify_init_event(self.agent_type, self)"},{"line_number":110,"context_line":"        # The initialization is complete; we can start receiving messages"},{"line_number":111,"context_line":"        self.connection.consume_in_threads()"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_e2fd14b2","line":109,"updated":"2016-09-02 13:26:29.000000000","message":"Yes since it\u0027s supposed to be agent-agnostic.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"}],"neutron/plugins/ml2/drivers/linuxbridge/agent/common/utils.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"312c52368b6536e62aff8df6ab0b7c9e1860b719","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":3,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":4,"context_line":"#    a copy of the License at"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_6b7791d4","line":1,"updated":"2016-09-09 22:13:03.000000000","message":"Ack","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"120ad0afb4224625157d4b1f1a9e29465643b155","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":3,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":4,"context_line":"#    a copy of the License at"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_9ddb3b1b","line":1,"updated":"2016-09-09 21:04:54.000000000","message":"I think there should be a:\n\nCopyright 2012 Cisco Systems, Inc.\n\nsince they added the original code for get_tap_device_name() to linuxbridge_neutron_agent.py","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"}],"neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_capabilities.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"312c52368b6536e62aff8df6ab0b7c9e1860b719","unresolved":false,"context_lines":[{"line_number":7,"context_line":"# distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"},{"line_number":8,"context_line":"# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":9,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":10,"context_line":"# under the License."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"from neutron_lib import constants"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_8b7215e2","line":10,"updated":"2016-09-09 22:13:03.000000000","message":"Done","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"120ad0afb4224625157d4b1f1a9e29465643b155","unresolved":false,"context_lines":[{"line_number":7,"context_line":"# distributed under the License is distributed on an \"AS IS\" BASIS, WITHOUT"},{"line_number":8,"context_line":"# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":9,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":10,"context_line":"# under the License."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"from neutron_lib import constants"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_bda8ff78","line":10,"updated":"2016-09-09 21:04:54.000000000","message":"nit: this copyright statement is a little wonky compared to others.","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"}],"neutron/services/trunk/drivers/linuxbridge/agent/driver.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from oslo_utils import excutils"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"class LinuxBridgeTrunkDriver(trunk_rpc.TrunkSkeleton):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_b2a66751","line":33,"updated":"2016-08-24 18:50:47.000000000","message":"Right now you slice this into two: TrunkDriver and _TrunkAPI (aside from _InterfaceInfo). I feel there\u0027s probably one or two more classes that can be carved out of these for better separation of responsibilities. I can clearly see a class that encapsulates the local cache alone (which in turns uses the stub when dealing with cache miss), and a class that encapsulates the linuxbridge underlying plumbing details. A class that deals with local events may even be in there (which use the \u0027plumber\u0027).\n\nThe driver\u0027s responsibility is to initialize the objects and use the one that does the plumbing himself when dealing with remote events.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def __init__(self, agent):"},{"line_number":39,"context_line":"        self._agent \u003d agent"},{"line_number":40,"context_line":"        self._tapi \u003d _TrunkAPI(trunk_rpc.TrunkStub())"},{"line_number":41,"context_line":"        registry.subscribe(self.agent_port_change,"},{"line_number":42,"context_line":"                           local_resources.PORT_DEVICE,"},{"line_number":43,"context_line":"                           local_events.AFTER_UPDATE)"},{"line_number":44,"context_line":"        registry.subscribe(self.agent_port_delete,"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_c269f08d","line":41,"updated":"2016-09-02 13:26:29.000000000","message":"These methods also have to call methods in this class so it\u0027s hard to move them out without just passing a reference back to this one...","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    def __init__(self, agent):"},{"line_number":39,"context_line":"        self._agent \u003d agent"},{"line_number":40,"context_line":"        self._tapi \u003d _TrunkAPI(trunk_rpc.TrunkStub())"},{"line_number":41,"context_line":"        registry.subscribe(self.agent_port_change,"},{"line_number":42,"context_line":"                           local_resources.PORT_DEVICE,"},{"line_number":43,"context_line":"                           local_events.AFTER_UPDATE)"},{"line_number":44,"context_line":"        registry.subscribe(self.agent_port_delete,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_f7ca9119","line":41,"updated":"2016-08-24 18:50:47.000000000","message":"why can\u0027t you make this the business of another class and have the LinuxBridgeTrunkDriver simply load it? This may lead to cleaner separation.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        registry.subscribe(self.agent_port_delete,"},{"line_number":45,"context_line":"                           local_resources.PORT_DEVICE,"},{"line_number":46,"context_line":"                           local_events.AFTER_DELETE)"},{"line_number":47,"context_line":"        super(LinuxBridgeTrunkDriver, self).__init__(agent.connection)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @lockutils.synchronized(\u0027das-trunk\u0027)"},{"line_number":50,"context_line":"    def handle_trunks(self, trunks, event_type):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_e266f4b9","line":47,"updated":"2016-09-02 13:26:29.000000000","message":"Done","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        registry.subscribe(self.agent_port_delete,"},{"line_number":45,"context_line":"                           local_resources.PORT_DEVICE,"},{"line_number":46,"context_line":"                           local_events.AFTER_DELETE)"},{"line_number":47,"context_line":"        super(LinuxBridgeTrunkDriver, self).__init__(agent.connection)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @lockutils.synchronized(\u0027das-trunk\u0027)"},{"line_number":50,"context_line":"    def handle_trunks(self, trunks, event_type):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_17dc8d17","line":47,"updated":"2016-08-24 18:50:47.000000000","message":"the skeleton no longer takes a connection, but it creates a new one. Initially I thought that a new connection is the one that creates new RPC bus resources, but that\u0027s not the case.\n\nIt\u0027s my understanding that each agent-side component can safely hold its own copy of the connection and call create_consumer/consume_in_threads on it.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":46,"context_line":"                           local_events.AFTER_DELETE)"},{"line_number":47,"context_line":"        super(LinuxBridgeTrunkDriver, self).__init__(agent.connection)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @lockutils.synchronized(\u0027das-trunk\u0027)"},{"line_number":50,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":51,"context_line":"        for trunk in trunks:"},{"line_number":52,"context_line":"            context \u003d n_ctx.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_57e605e9","line":49,"updated":"2016-08-24 18:50:47.000000000","message":"booo","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if not ip_lib.device_exists(trunk_dev):"},{"line_number":103,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk_dev)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self._tapi.set_trunk_status(context, trunk, \u0027BUILD\u0027)"},{"line_number":106,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            self._try_ensure_trunk_subports(trunk)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_020678cd","line":105,"updated":"2016-09-02 13:26:29.000000000","message":"sure","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if not ip_lib.device_exists(trunk_dev):"},{"line_number":103,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk_dev)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self._tapi.set_trunk_status(context, trunk, \u0027BUILD\u0027)"},{"line_number":106,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            self._try_ensure_trunk_subports(trunk)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_576c4508","line":105,"updated":"2016-08-24 18:50:47.000000000","message":"this should be done by update_subport_bindings seamlessly, do you think we can avoid the call here?","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk_dev)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self._tapi.set_trunk_status(context, trunk, \u0027BUILD\u0027)"},{"line_number":106,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            self._try_ensure_trunk_subports(trunk)"},{"line_number":109,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_3748a987","line":106,"updated":"2016-08-24 18:50:47.000000000","message":"this is update_subport_bindings if I understand this correctly.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk_dev)"},{"line_number":104,"context_line":"            return"},{"line_number":105,"context_line":"        self._tapi.set_trunk_status(context, trunk, \u0027BUILD\u0027)"},{"line_number":106,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            self._try_ensure_trunk_subports(trunk)"},{"line_number":109,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_220b7cd7","line":106,"updated":"2016-09-02 13:26:29.000000000","message":"yep","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                # something broke"},{"line_number":116,"context_line":"                LOG.exception(_LE(\"Failure setting up subports for %s\"),"},{"line_number":117,"context_line":"                              trunk_dev)"},{"line_number":118,"context_line":"                self._tapi.set_trunk_status(context, trunk, \u0027ERROR\u0027)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def _try_ensure_trunk_subports(self, trunk):"},{"line_number":121,"context_line":"        trunk_dev \u003d self._trunk_device_name(trunk)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_17300d07","line":118,"updated":"2016-08-24 18:50:47.000000000","message":"should this be \u0027DEGRADED\u0027, indicating that the error occurred only for a subset of subports?","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                # something broke"},{"line_number":116,"context_line":"                LOG.exception(_LE(\"Failure setting up subports for %s\"),"},{"line_number":117,"context_line":"                              trunk_dev)"},{"line_number":118,"context_line":"                self._tapi.set_trunk_status(context, trunk, \u0027ERROR\u0027)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def _try_ensure_trunk_subports(self, trunk):"},{"line_number":121,"context_line":"        trunk_dev \u003d self._trunk_device_name(trunk)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_420c00ec","line":118,"updated":"2016-09-02 13:26:29.000000000","message":"yeah","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                      dict(name\u003ddevname, tag\u003dvlan_id))"},{"line_number":143,"context_line":"            self._safe_delete_device(devname)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def handle_subport_agent_change(self, trunk, port):"},{"line_number":146,"context_line":"        # we need to correct the mac address of the subport that we created"},{"line_number":147,"context_line":"        # now that we have the port data to compare to"},{"line_number":148,"context_line":"        dev_name \u003d self._agent.mgr.get_tap_device_name(port[\u0027port_id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_f2d61f26","line":145,"updated":"2016-08-24 18:50:47.000000000","message":"I think these can be responsibility of a different class.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                      dict(name\u003ddevname, tag\u003dvlan_id))"},{"line_number":143,"context_line":"            self._safe_delete_device(devname)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def handle_subport_agent_change(self, trunk, port):"},{"line_number":146,"context_line":"        # we need to correct the mac address of the subport that we created"},{"line_number":147,"context_line":"        # now that we have the port data to compare to"},{"line_number":148,"context_line":"        dev_name \u003d self._agent.mgr.get_tap_device_name(port[\u0027port_id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_62110404","line":145,"updated":"2016-09-02 13:26:29.000000000","message":"done","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":159,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice(dev_name).exists()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def _create_vlan_subint(self, trunk_name, devname, vlan_id):"},{"line_number":162,"context_line":"        ip_wrap \u003d ip_lib.IPWrapper(namespace\u003dNone)"},{"line_number":163,"context_line":"        try:"},{"line_number":164,"context_line":"            ip_wrap.add_vlan(devname, trunk_name, vlan_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_52dc5346","line":161,"updated":"2016-08-24 18:50:47.000000000","message":"ditto","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":159,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice(dev_name).exists()"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def _create_vlan_subint(self, trunk_name, devname, vlan_id):"},{"line_number":162,"context_line":"        ip_wrap \u003d ip_lib.IPWrapper(namespace\u003dNone)"},{"line_number":163,"context_line":"        try:"},{"line_number":164,"context_line":"            ip_wrap.add_vlan(devname, trunk_name, vlan_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_82fa88bb","line":161,"updated":"2016-09-02 13:26:29.000000000","message":"done","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":167,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice(trunk_name).exists()"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _safe_delete_device(self, devname):"},{"line_number":170,"context_line":"        dev \u003d ip_lib.IPDevice(devname)"},{"line_number":171,"context_line":"        try:"},{"line_number":172,"context_line":"            dev.link.set_down()"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_a2f78cd1","line":169,"updated":"2016-09-02 13:26:29.000000000","message":"done","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":166,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":167,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice(trunk_name).exists()"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _safe_delete_device(self, devname):"},{"line_number":170,"context_line":"        dev \u003d ip_lib.IPDevice(devname)"},{"line_number":171,"context_line":"        try:"},{"line_number":172,"context_line":"            dev.link.set_down()"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_92cd2b89","line":169,"updated":"2016-08-24 18:50:47.000000000","message":"same here","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"class _TrunkAPI(object):"},{"line_number":194,"context_line":"    \"\"\"Our secret stash of trunks stored by port ID. Tell no one.\"\"\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def __init__(self, trunk_stub):"},{"line_number":197,"context_line":"        self.server_api \u003d trunk_stub"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_7247cf1d","line":194,"updated":"2016-08-24 18:50:47.000000000","message":"I\u0027d personally split the responsibility of this class into two: one that implements the local cache, and one that is used by the local cache to deal with a cache miss. Only the latter needs a reference to the stub.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed27f062fb874ab693928669c58c7d15fc8e5639","unresolved":false,"context_lines":[{"line_number":191,"context_line":""},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"class _TrunkAPI(object):"},{"line_number":194,"context_line":"    \"\"\"Our secret stash of trunks stored by port ID. Tell no one.\"\"\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def __init__(self, trunk_stub):"},{"line_number":197,"context_line":"        self.server_api \u003d trunk_stub"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa7ab95a_c20010ae","line":194,"updated":"2016-09-02 13:26:29.000000000","message":"The stub is the thing that deals with the cache miss. I\u0027m having trouble seeing how separating it further works. Also, if I add another class between this and the manager to access the stub, that makes it difficult for the manager to call update_trunk_status, no?","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            return None"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def set_trunk_status(self, context, trunk, status):"},{"line_number":213,"context_line":"        # TODO(kevinbenton): retries"},{"line_number":214,"context_line":"        self.server_api.update_trunk_status(context, trunk.id, status)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def bind_subports_to_host(self, context, trunk):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_521f335b","line":213,"updated":"2016-08-24 18:50:47.000000000","message":"wouldn\u0027t it make sense to add the retries to the underlying stub implementation? This way other stub users can benefit of the more resilient logic. What retries are you thinking of though? I thought the underlying RPC client took care of retrying failed RPC messages.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"446daf24dbcf916431e0dcb1cf89953d70d9455d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        self.server_api.update_trunk_status(context, trunk.id, status)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def bind_subports_to_host(self, context, trunk):"},{"line_number":217,"context_line":"        # TODO(kevinbenton): retries"},{"line_number":218,"context_line":"        self.server_api.update_subport_bindings(context, trunk.sub_ports)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def put_trunk_subport(self, trunk_id, subport):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ac06dbe_b23027ea","line":217,"updated":"2016-08-24 18:50:47.000000000","message":"Same here.","commit_id":"cf33506d2aa5c0c5124de54a4dfe27701d1fa192"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        LinuxBridgeTrunkDriver(agent)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class LinuxBridgeTrunkDriver(trunk_rpc.TrunkSkeleton):"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, agent):"},{"line_number":46,"context_line":"        self._plumber \u003d _Plumber(agent)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_966bc8f0","line":43,"updated":"2016-09-07 09:19:17.000000000","message":"ok","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        LinuxBridgeTrunkDriver(agent)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class LinuxBridgeTrunkDriver(trunk_rpc.TrunkSkeleton):"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def __init__(self, agent):"},{"line_number":46,"context_line":"        self._plumber \u003d _Plumber(agent)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_188a991b","line":43,"updated":"2016-09-06 23:11:35.000000000","message":"overall docstrings would improve readability quite a bit IMO","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":57,"context_line":"        for trunk in trunks:"},{"line_number":58,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":59,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":60,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":61,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_b670ccdd","line":58,"updated":"2016-09-07 09:19:17.000000000","message":"This constructs a context for the single handle trunks event, so thats what I get by doing this. Or am I misunderstanding what you are saying?\n\n(even better would be if RPC push notifications sent the context so we could track API operations all of the way through to the agent, but that\u0027s a different story)","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":57,"context_line":"        for trunk in trunks:"},{"line_number":58,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":59,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":60,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":61,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_18d5391b","line":58,"updated":"2016-09-06 23:11:35.000000000","message":"weren\u0027t you the one complaining about wanting a context with different request IDs?","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3d92f0580d9b9364f726cdc2c45d66370f3d7022","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":57,"context_line":"        for trunk in trunks:"},{"line_number":58,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":59,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":60,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":61,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_36d64ef1","line":58,"in_reply_to":"9a89bdaa_867619d5","updated":"2016-09-07 22:36:14.000000000","message":"I did it differently because it was invasive to change all of the references to self.adminCtx or whatever it was called. I would rather have it be explicit a new one was being built like this if we can\u0027t maintain the one from the server.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"11b68f5daf331b71adc4d8cc0537f49f0a8b8739","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":57,"context_line":"        for trunk in trunks:"},{"line_number":58,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":59,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":60,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":61,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_867619d5","line":58,"in_reply_to":"9a89bdaa_b670ccdd","updated":"2016-09-07 17:42:02.000000000","message":"No, I think this is fine, however in the DHCP and L3 agent you did it slightly differently and I wondered why","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                self.delete_trunk_subports(context, trunk)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_d679d0c9","line":67,"updated":"2016-09-07 09:19:17.000000000","message":"?","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":64,"context_line":"                self.delete_trunk_subports(context, trunk)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_f8ec15f5","line":67,"updated":"2016-09-06 23:11:35.000000000","message":"ditto","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"},{"line_number":71,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_f676d4d5","line":68,"updated":"2016-09-07 09:19:17.000000000","message":"The way the function is written, it handles multiple trunks. I wasn\u0027t sure if that was a thing or not.\n\nAre you saying the complexity of this method is too high?","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"},{"line_number":71,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_58b26107","line":68,"updated":"2016-09-06 23:11:35.000000000","message":"you could but that would make the complexity of this method not worth your while.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3d92f0580d9b9364f726cdc2c45d66370f3d7022","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"},{"line_number":71,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_96347a4a","line":68,"in_reply_to":"9a89bdaa_66603502","updated":"2016-09-07 22:36:14.000000000","message":"ack. then i can simplify this","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"11b68f5daf331b71adc4d8cc0537f49f0a8b8739","unresolved":false,"context_lines":[{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":67,"context_line":"        context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"        affected_trunks \u003d set()  # can we receive for multiple trunks?"},{"line_number":69,"context_line":"        if event_type \u003d\u003d events.DELETED:"},{"line_number":70,"context_line":"            method \u003d self._tapi.delete_trunk_subport"},{"line_number":71,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_66603502","line":68,"in_reply_to":"9a89bdaa_f676d4d5","updated":"2016-09-07 17:42:02.000000000","message":"what I am saying is that the RPC upcall gives you subports for a single trunk.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        elif self._tapi.get_trunk_for_subport(context,"},{"line_number":99,"context_line":"                                              device_details[\u0027port_id\u0027]):"},{"line_number":100,"context_line":"            # This is a subport. We need to ensure the correct mac address is"},{"line_number":101,"context_line":"            # set now that we have the port data to see the data model mac."},{"line_number":102,"context_line":"            self._plumber.set_port_mac(device_details[\u0027port_id\u0027],"},{"line_number":103,"context_line":"                                       device_details[\u0027mac_address\u0027])"},{"line_number":104,"context_line":"        # TODO(kevinbenton): wipe out all vlans on interface if not a trunk"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_f8bfb5a7","line":101,"updated":"2016-09-06 23:11:35.000000000","message":"MAC?","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        elif self._tapi.get_trunk_for_subport(context,"},{"line_number":99,"context_line":"                                              device_details[\u0027port_id\u0027]):"},{"line_number":100,"context_line":"            # This is a subport. We need to ensure the correct mac address is"},{"line_number":101,"context_line":"            # set now that we have the port data to see the data model mac."},{"line_number":102,"context_line":"            self._plumber.set_port_mac(device_details[\u0027port_id\u0027],"},{"line_number":103,"context_line":"                                       device_details[\u0027mac_address\u0027])"},{"line_number":104,"context_line":"        # TODO(kevinbenton): wipe out all vlans on interface if not a trunk"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_1680988a","line":101,"updated":"2016-09-07 09:19:17.000000000","message":"https://en.wikipedia.org/wiki/MAC_address","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"11b68f5daf331b71adc4d8cc0537f49f0a8b8739","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        elif self._tapi.get_trunk_for_subport(context,"},{"line_number":99,"context_line":"                                              device_details[\u0027port_id\u0027]):"},{"line_number":100,"context_line":"            # This is a subport. We need to ensure the correct mac address is"},{"line_number":101,"context_line":"            # set now that we have the port data to see the data model mac."},{"line_number":102,"context_line":"            self._plumber.set_port_mac(device_details[\u0027port_id\u0027],"},{"line_number":103,"context_line":"                                       device_details[\u0027mac_address\u0027])"},{"line_number":104,"context_line":"        # TODO(kevinbenton): wipe out all vlans on interface if not a trunk"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_463ed1da","line":101,"in_reply_to":"9a89bdaa_1680988a","updated":"2016-09-07 17:42:02.000000000","message":"right, capitalize MAC","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_367d9cb1","line":110,"updated":"2016-09-07 09:19:17.000000000","message":"@Rossella,\n\nIf we create the subports before they are bound to this host, we end up with a small race condition where this agent can start wiring the ports before the host_id is correctly set. It could then call update_device_up on the subport and have its status change ignored because the host_id is incorrect.\n\nEven if we solve that, I prefer the method of binding first to set the trunk to BUILD before we start building, not after it\u0027s done. Sort of like the agent does with ports.\n\n\n@Armando,\n\nI would rather keep consistency between the created subports and the subports in the data model. If one has failed to bind, that will be reflected in the port status and will prevent the subport from being wired correctly by the main L2 agent loop (due to missing segment info). Otherwise we risk binding failure recovery fixing the port but not the trunk.\n\n\nThink about something like the DHCP agent. It creates its interface and plugs it in whether or not the port binding failed. It\u0027s then up to the main L2 agent process to decide what to do with it.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"668417aff5acfd849cf3029ecebe8f65f548d1dd","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_801759a2","line":110,"updated":"2016-09-05 15:50:37.000000000","message":"wouldn\u0027t it be safer to move this line after line 112, so we update the binding when we are sure that ensure_trunk_subports didn\u0027t fail?","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"403371f8b1274ef943539a1387d4ba2ae86c536c","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_56bf5260","line":110,"in_reply_to":"9a89bdaa_16206af6","updated":"2016-09-07 22:42:04.000000000","message":"Let\u0027s brainstorm a little on this, because I can see your side of the argument only partially. If the logical model cannot be honored there\u0027s no point in stubbornly getting into a broken physical state IMO","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"11b68f5daf331b71adc4d8cc0537f49f0a8b8739","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_e9d17658","line":110,"in_reply_to":"9a89bdaa_367d9cb1","updated":"2016-09-07 17:42:02.000000000","message":"Um, I need to think more about this as I am not convinced that proceeding with creating physical resources for a broken subport makes sense, when the server has determined that the port cannot be bound. For that matter, we cannot even assume the host binding has been done correctly.\n\nPerhaps we just need to learn in practice about the failure mode.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_785d6578","line":110,"in_reply_to":"9a89bdaa_801759a2","updated":"2016-09-06 23:11:35.000000000","message":"It is worth considering that the number of ports being bound may be less than the number of subports being currently associated to the trunk, that\u0027s because binding failures may prevent some subports from being bound. The underlying RPC calls returns the list of ports that were successfully bound. Therefore it\u0027s probably safer to make sure you ensure trunk_subports only for those, otherwise things would be out of sync between agent and server","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3d92f0580d9b9364f726cdc2c45d66370f3d7022","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        if not self._plumber.trunk_on_host(trunk):"},{"line_number":108,"context_line":"            LOG.debug(\"Trunk %s not present on this host\", trunk.port_id)"},{"line_number":109,"context_line":"            return"},{"line_number":110,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":111,"context_line":"        try:"},{"line_number":112,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":113,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_16206af6","line":110,"in_reply_to":"9a89bdaa_e9d17658","updated":"2016-09-07 22:36:14.000000000","message":"i\u0027m thinking about it from a separation of concerns perspective.\n\nThe existence of the vlan sub-interfaces should reflect the data model of the trunk IMO. The status of the port resources associated to the subports is something for the main agent loop to worry about.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice("},{"line_number":186,"context_line":"                    dev_name, namespace\u003dself.namespace).exists()"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _trunk_lock(self, trunk_dev):"},{"line_number":189,"context_line":"        lock_name \u003d \u0027trunk-%s\u0027 % trunk_dev"},{"line_number":190,"context_line":"        return lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX)"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_564ce09d","line":188,"updated":"2016-09-07 09:19:17.000000000","message":"Maybe, but that\u0027s a decorator and not a context manager so I would have to split out the functions that use this for only part of their calls.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice("},{"line_number":186,"context_line":"                    dev_name, namespace\u003dself.namespace).exists()"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _trunk_lock(self, trunk_dev):"},{"line_number":189,"context_line":"        lock_name \u003d \u0027trunk-%s\u0027 % trunk_dev"},{"line_number":190,"context_line":"        return lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX)"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_d8faf142","line":188,"updated":"2016-09-06 23:11:35.000000000","message":"We don\u0027t have to deal with this right now, but it\u0027s worth considering using the same decorator [1] later on.\n\n[1] https://review.openstack.org/#/c/347466/16/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py@44","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1cb48a49e154fc6cef05e0b379e68246d552ece9","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            return None"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def set_trunk_status(self, context, trunk, status):"},{"line_number":250,"context_line":"        # TODO(kevinbenton): retries"},{"line_number":251,"context_line":"        self.server_api.update_trunk_status(context, trunk.id, status)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def bind_subports_to_host(self, context, trunk):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_98606903","line":250,"updated":"2016-09-06 23:11:35.000000000","message":"Perhaps it\u0027s worth dealing with this directly in the stub, so that any user benefits from it...but what\u0027s with you and retries! gee","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f40531a812b098559c57887ebfc28b94ca611ae5","unresolved":false,"context_lines":[{"line_number":247,"context_line":"            return None"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def set_trunk_status(self, context, trunk, status):"},{"line_number":250,"context_line":"        # TODO(kevinbenton): retries"},{"line_number":251,"context_line":"        self.server_api.update_trunk_status(context, trunk.id, status)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def bind_subports_to_host(self, context, trunk):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a89bdaa_7651e435","line":250,"updated":"2016-09-07 09:19:17.000000000","message":"Whoops. Meant to remove the TODO. We can figure out what to do with that later.","commit_id":"da036fd4486e227cfd0dfeef4e901fe080d0b57c"},{"author":{"_account_id":10558,"name":"Dustin Lundquist","email":"dustin@null-ptr.net","username":"dlundquist"},"change_message_id":"07e6938af763d72f23caf36f844bb6282c4e1705","unresolved":false,"context_lines":[{"line_number":216,"context_line":"    def _create_vlan_subint(self, trunk_name, devname, vlan_id):"},{"line_number":217,"context_line":"        ip_wrap \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"},{"line_number":218,"context_line":"        try:"},{"line_number":219,"context_line":"            ip_wrap.add_vlan(devname, trunk_name, vlan_id)"},{"line_number":220,"context_line":"        except Exception:"},{"line_number":221,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":222,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a89bdaa_47e086cd","line":219,"updated":"2016-09-07 20:45:37.000000000","message":"We should ensure IPv6 is disabled on the new VLAN interface before it is transitioned to the link up state to avoid exposing the host machine via IPv6 link-local addresses see https://review.openstack.org/#/c/296659","commit_id":"e95dd8ddd74aee74a24437622b7c06b219b72fa9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f587de17269b09263cfdea3c756dafdb4f29c704","unresolved":false,"context_lines":[{"line_number":216,"context_line":"    def _create_vlan_subint(self, trunk_name, devname, vlan_id):"},{"line_number":217,"context_line":"        ip_wrap \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"},{"line_number":218,"context_line":"        try:"},{"line_number":219,"context_line":"            ip_wrap.add_vlan(devname, trunk_name, vlan_id)"},{"line_number":220,"context_line":"        except Exception:"},{"line_number":221,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":222,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a89bdaa_f6a14695","line":219,"in_reply_to":"9a89bdaa_47e086cd","updated":"2016-09-07 22:31:07.000000000","message":"good point. I think that might be worth putting right in add_vlan.","commit_id":"e95dd8ddd74aee74a24437622b7c06b219b72fa9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7c41d3e1539389526b0f635efc93c4d52595c794","unresolved":false,"context_lines":[{"line_number":216,"context_line":"    def _create_vlan_subint(self, trunk_name, devname, vlan_id):"},{"line_number":217,"context_line":"        ip_wrap \u003d ip_lib.IPWrapper(namespace\u003dself.namespace)"},{"line_number":218,"context_line":"        try:"},{"line_number":219,"context_line":"            ip_wrap.add_vlan(devname, trunk_name, vlan_id)"},{"line_number":220,"context_line":"        except Exception:"},{"line_number":221,"context_line":"            with excutils.save_and_reraise_exception() as ectx:"},{"line_number":222,"context_line":"                ectx.reraise \u003d ip_lib.IPDevice("}],"source_content_type":"text/x-python","patch_set":10,"id":"9a89bdaa_fa576055","line":219,"in_reply_to":"9a89bdaa_f6a14695","updated":"2016-09-08 05:53:07.000000000","message":"latest patch just explicitly disables it here. we can look later at making that part of add_vlan.","commit_id":"e95dd8ddd74aee74a24437622b7c06b219b72fa9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                self.wire_trunk(context, trunk)"},{"line_number":70,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":71,"context_line":"                self._tapi.put_trunk(trunk.port_id, None)"},{"line_number":72,"context_line":"                self.delete_trunk_subports(context, trunk)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":75,"context_line":"        \"\"\"Subport data model change from the server.\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_94727248","line":72,"updated":"2016-09-08 23:07:57.000000000","message":"isn\u0027t this supposed to be self.plumber.delete_trunk_subports? I am seeing double. But why deleting just the subports?","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                self.wire_trunk(context, trunk)"},{"line_number":70,"context_line":"            elif event_type \u003d\u003d events.DELETED:"},{"line_number":71,"context_line":"                self._tapi.put_trunk(trunk.port_id, None)"},{"line_number":72,"context_line":"                self.delete_trunk_subports(context, trunk)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def handle_subports(self, subports, event_type):"},{"line_number":75,"context_line":"        \"\"\"Subport data model change from the server.\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_45166c56","line":72,"updated":"2016-09-09 07:12:36.000000000","message":"it is. looks like i need some test coverage here.\n\nIt\u0027s just deleting the subports because that\u0027s all there is to delete. The trunk itself has no special physical resource.","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    def agent_port_delete(self, resource, event, trigger, context, port_id,"},{"line_number":92,"context_line":"                          **kwargs):"},{"line_number":93,"context_line":"        \"\"\"Agent informed us a VIF was removed.\"\"\""},{"line_number":94,"context_line":"        # NOTE(kevinbenton): we don\u0027t need to do anything to cleanup VLAN"},{"line_number":95,"context_line":"        # interfaces if a trunk was removed because the kernel will do that"},{"line_number":96,"context_line":"        # for us. We also don\u0027t update the trunk status to DOWN because we"},{"line_number":97,"context_line":"        # don\u0027t want to race with another agent that the trunk may have been"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_d4923a4b","line":94,"updated":"2016-09-08 23:07:57.000000000","message":"So is this here for documentation purposes only?","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    def agent_port_delete(self, resource, event, trigger, context, port_id,"},{"line_number":92,"context_line":"                          **kwargs):"},{"line_number":93,"context_line":"        \"\"\"Agent informed us a VIF was removed.\"\"\""},{"line_number":94,"context_line":"        # NOTE(kevinbenton): we don\u0027t need to do anything to cleanup VLAN"},{"line_number":95,"context_line":"        # interfaces if a trunk was removed because the kernel will do that"},{"line_number":96,"context_line":"        # for us. We also don\u0027t update the trunk status to DOWN because we"},{"line_number":97,"context_line":"        # don\u0027t want to race with another agent that the trunk may have been"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_650b702c","line":94,"updated":"2016-09-09 07:12:36.000000000","message":"yep. also a placeholder in case we need to tell the server something in the future.","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            return"},{"line_number":108,"context_line":"        # clear any VLANs in case this was a trunk that changed status while"},{"line_number":109,"context_line":"        # agent was offline."},{"line_number":110,"context_line":"        self._plumber.delete_subports_by_port_id(device_details[\u0027port_id\u0027])"},{"line_number":111,"context_line":"        if self._tapi.get_trunk_for_subport(context,"},{"line_number":112,"context_line":"                                            device_details[\u0027port_id\u0027]):"},{"line_number":113,"context_line":"            # This is a subport. We need to ensure the correct mac address is"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_b4b4d6d4","line":110,"updated":"2016-09-08 23:07:57.000000000","message":"isn\u0027t this delete_trunk_subports?","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":107,"context_line":"            return"},{"line_number":108,"context_line":"        # clear any VLANs in case this was a trunk that changed status while"},{"line_number":109,"context_line":"        # agent was offline."},{"line_number":110,"context_line":"        self._plumber.delete_subports_by_port_id(device_details[\u0027port_id\u0027])"},{"line_number":111,"context_line":"        if self._tapi.get_trunk_for_subport(context,"},{"line_number":112,"context_line":"                                            device_details[\u0027port_id\u0027]):"},{"line_number":113,"context_line":"            # This is a subport. We need to ensure the correct mac address is"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_8508742f","line":110,"updated":"2016-09-09 07:12:36.000000000","message":"we don\u0027t have a trunk object so we just clear everything based port_id","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":124,"context_line":"        try:"},{"line_number":125,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":126,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"},{"line_number":127,"context_line":"        except Exception:"},{"line_number":128,"context_line":"            if not self._plumber.trunk_on_host(trunk):"},{"line_number":129,"context_line":"                LOG.debug(\"Trunk %s removed during wiring\", trunk.port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_94c0b22c","line":126,"updated":"2016-09-08 23:07:57.000000000","message":"booo, use defined string literals","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        self._tapi.bind_subports_to_host(context, trunk)"},{"line_number":124,"context_line":"        try:"},{"line_number":125,"context_line":"            self._plumber.ensure_trunk_subports(trunk)"},{"line_number":126,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027ACTIVE\u0027)"},{"line_number":127,"context_line":"        except Exception:"},{"line_number":128,"context_line":"            if not self._plumber.trunk_on_host(trunk):"},{"line_number":129,"context_line":"                LOG.debug(\"Trunk %s removed during wiring\", trunk.port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_a5057815","line":126,"updated":"2016-09-09 07:12:36.000000000","message":"ok","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            # something broke"},{"line_number":132,"context_line":"            LOG.exception(_LE(\"Failure setting up subports for %s\"),"},{"line_number":133,"context_line":"                          trunk.port_id)"},{"line_number":134,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027DEGRADED\u0027)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class _Plumber(object):"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_b4dbf620","line":134,"updated":"2016-09-08 23:07:57.000000000","message":"here too","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            # something broke"},{"line_number":132,"context_line":"            LOG.exception(_LE(\"Failure setting up subports for %s\"),"},{"line_number":133,"context_line":"                          trunk.port_id)"},{"line_number":134,"context_line":"            self._tapi.set_trunk_status(context, trunk, \u0027DEGRADED\u0027)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"class _Plumber(object):"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_c5027c0b","line":134,"updated":"2016-09-09 07:12:36.000000000","message":"ok","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            to_delete \u003d existing - expected"},{"line_number":169,"context_line":"            to_create \u003d expected - existing"},{"line_number":170,"context_line":"            for devname, vlan_id in to_delete:"},{"line_number":171,"context_line":"                LOG.debug(\"Deleting subport %(name)s with vlan tag %(tag)s\","},{"line_number":172,"context_line":"                          dict(name\u003ddevname, tag\u003dvlan_id))"},{"line_number":173,"context_line":"                self._safe_delete_device(devname)"},{"line_number":174,"context_line":"            for devname, vlan_id in to_create:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_d4079a7f","line":171,"updated":"2016-09-08 23:07:57.000000000","message":"for debug traces you don\u0027t need named params but meh","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8f3452cc0d3445a4e0972633b96ba2de04415d10","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                          dict(name\u003ddevname, tag\u003dvlan_id))"},{"line_number":177,"context_line":"                self._create_vlan_subint(trunk_dev, devname, vlan_id)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def delete_trunk_subports(self, trunk):"},{"line_number":180,"context_line":"        return self.delete_subports_by_port_id(trunk.port_id)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def delete_subports_by_port_id(self, port_id):"},{"line_number":183,"context_line":"        device \u003d self._get_tap_device_name(port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_100c8441","line":180,"range":{"start_line":179,"start_character":4,"end_line":180,"end_character":61},"updated":"2016-09-09 15:06:05.000000000","message":"I meant that you have this method and don\u0027t use it at line 110","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b5affdc88283a408c62c61b21fa70ce122c937f2","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                          dict(name\u003ddevname, tag\u003dvlan_id))"},{"line_number":177,"context_line":"                self._create_vlan_subint(trunk_dev, devname, vlan_id)"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def delete_trunk_subports(self, trunk):"},{"line_number":180,"context_line":"        return self.delete_subports_by_port_id(trunk.port_id)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def delete_subports_by_port_id(self, port_id):"},{"line_number":183,"context_line":"        device \u003d self._get_tap_device_name(port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_d3a4e609","line":180,"range":{"start_line":179,"start_character":4,"end_line":180,"end_character":61},"in_reply_to":"9a89bdaa_100c8441","updated":"2016-09-09 15:08:33.000000000","message":"oh, I see.","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"52d523bd023174bb18feee34dff1381ee3f329b7","unresolved":false,"context_lines":[{"line_number":320,"context_line":"        \"\"\"Gets trunk object for port_id. None if not trunk.\"\"\""},{"line_number":321,"context_line":"        if port_id not in self._trunk_by_port_id:"},{"line_number":322,"context_line":"            # TODO(kevinbenton): ask the server for *all* trunk port IDs on"},{"line_number":323,"context_line":"            # start and eliminate asking the server if every port is a trunk"},{"line_number":324,"context_line":"            LOG.debug(\"Cache miss for port %s, fetching from server\", port_id)"},{"line_number":325,"context_line":"            self.put_trunk(port_id, self._fetch_trunk(context, port_id))"},{"line_number":326,"context_line":"            return self.get_trunk(context, port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_346c4636","line":323,"updated":"2016-09-08 23:07:57.000000000","message":"another TODO is cache eviction in case the agent becomes disconnected for a while.","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e90040b77cf48db443d5f36d01694e5f3b2a04e4","unresolved":false,"context_lines":[{"line_number":320,"context_line":"        \"\"\"Gets trunk object for port_id. None if not trunk.\"\"\""},{"line_number":321,"context_line":"        if port_id not in self._trunk_by_port_id:"},{"line_number":322,"context_line":"            # TODO(kevinbenton): ask the server for *all* trunk port IDs on"},{"line_number":323,"context_line":"            # start and eliminate asking the server if every port is a trunk"},{"line_number":324,"context_line":"            LOG.debug(\"Cache miss for port %s, fetching from server\", port_id)"},{"line_number":325,"context_line":"            self.put_trunk(port_id, self._fetch_trunk(context, port_id))"},{"line_number":326,"context_line":"            return self.get_trunk(context, port_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a89bdaa_e0fdae09","line":323,"updated":"2016-09-09 07:12:36.000000000","message":"ok","commit_id":"96240681e4ab591d85cd87a76884c13264cd6039"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"120ad0afb4224625157d4b1f1a9e29465643b155","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from neutron.services.trunk import constants as t_const"},{"line_number":31,"context_line":"from neutron.services.trunk.rpc import agent as trunk_rpc"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"from oslo_utils import excutils"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_dd1143b1","line":33,"updated":"2016-09-09 21:04:54.000000000","message":"Should be on L18","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"312c52368b6536e62aff8df6ab0b7c9e1860b719","unresolved":false,"context_lines":[{"line_number":30,"context_line":"from neutron.services.trunk import constants as t_const"},{"line_number":31,"context_line":"from neutron.services.trunk.rpc import agent as trunk_rpc"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"from oslo_utils import excutils"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_0b660529","line":33,"updated":"2016-09-09 22:13:03.000000000","message":"whoops. good catch","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"312c52368b6536e62aff8df6ab0b7c9e1860b719","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":65,"context_line":"        \"\"\"Trunk data model change from the server.\"\"\""},{"line_number":66,"context_line":"        for trunk in trunks:"},{"line_number":67,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":69,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":70,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_2b6d8904","line":67,"updated":"2016-09-09 22:13:03.000000000","message":"Done","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"120ad0afb4224625157d4b1f1a9e29465643b155","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def handle_trunks(self, trunks, event_type):"},{"line_number":65,"context_line":"        \"\"\"Trunk data model change from the server.\"\"\""},{"line_number":66,"context_line":"        for trunk in trunks:"},{"line_number":67,"context_line":"            context \u003d n_ctx.get_admin_context()"},{"line_number":68,"context_line":"            if event_type in (events.UPDATED, events.CREATED):"},{"line_number":69,"context_line":"                self._tapi.put_trunk(trunk.port_id, trunk)"},{"line_number":70,"context_line":"                self.wire_trunk(context, trunk)"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_bdf71f58","line":67,"updated":"2016-09-09 21:04:54.000000000","message":"nit: could be outside loop to not re-initialize","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"312c52368b6536e62aff8df6ab0b7c9e1860b719","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # interfaces if a trunk was removed because the kernel will do that"},{"line_number":97,"context_line":"        # for us. We also don\u0027t update the trunk status to DOWN because we"},{"line_number":98,"context_line":"        # don\u0027t want to race with another agent that the trunk may have been"},{"line_number":99,"context_line":"        # moved to."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def agent_port_change(self, resource, event, trigger, context,"},{"line_number":102,"context_line":"                          device_details, **kwargs):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_4b700ddd","line":99,"updated":"2016-09-09 22:13:03.000000000","message":"No, I\u0027m starting an anti-pass movement. :)","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"120ad0afb4224625157d4b1f1a9e29465643b155","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # interfaces if a trunk was removed because the kernel will do that"},{"line_number":97,"context_line":"        # for us. We also don\u0027t update the trunk status to DOWN because we"},{"line_number":98,"context_line":"        # don\u0027t want to race with another agent that the trunk may have been"},{"line_number":99,"context_line":"        # moved to."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def agent_port_change(self, resource, event, trigger, context,"},{"line_number":102,"context_line":"                          device_details, **kwargs):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a89bdaa_dd622316","line":99,"updated":"2016-09-09 21:04:54.000000000","message":"not even a pass? :)","commit_id":"2a3b5c05d5aa7ebe1fc849598420e32f843e822a"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"beae40ed50670e0328de0edbabc70937be1eb4af","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                s: p for s, p in self._sub_port_id_to_trunk_port_id.items()"},{"line_number":186,"context_line":"                if p !\u003d port_id}"},{"line_number":187,"context_line":"        self._trunk_by_port_id[port_id] \u003d trunk"},{"line_number":188,"context_line":"        if not trunk:"},{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        self._trunk_by_id[trunk.id] \u003d trunk"},{"line_number":191,"context_line":"        for sub in trunk.sub_ports:"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a89bdaa_567672a4","line":188,"updated":"2016-09-13 14:36:47.000000000","message":"if trunk is None do we need to update trunk_by_port_id and _trunk_by_id? If so, why do you skip the latter?","commit_id":"2e882a94960f5af7f5720eeb7f5154840aec723c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"4d2f56c6599a1b2c73b0bee689998c49e9a9803c","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                s: p for s, p in self._sub_port_id_to_trunk_port_id.items()"},{"line_number":186,"context_line":"                if p !\u003d port_id}"},{"line_number":187,"context_line":"        self._trunk_by_port_id[port_id] \u003d trunk"},{"line_number":188,"context_line":"        if not trunk:"},{"line_number":189,"context_line":"            return"},{"line_number":190,"context_line":"        self._trunk_by_id[trunk.id] \u003d trunk"},{"line_number":191,"context_line":"        for sub in trunk.sub_ports:"}],"source_content_type":"text/x-python","patch_set":14,"id":"7a8ec9b2_4fce9284","line":188,"in_reply_to":"9a89bdaa_567672a4","updated":"2016-09-13 23:04:36.000000000","message":"This is the association between a port and trunk. If it\u0027s None, that means that it isn\u0027t a trunk so we do want to explicitly store that.\n\nWe can\u0027t update _trunk_by_id because there is no trunk.","commit_id":"2e882a94960f5af7f5720eeb7f5154840aec723c"}],"neutron/services/trunk/rules.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"63d00553e96444811203ee19d7c255b82c1cf24e","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        if trunks:"},{"line_number":75,"context_line":"            raise trunk_exc.ParentPortInUse(port_id\u003dself.port_id)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        if self.is_bound(context) and False:  # \u003c--- nu-uh!!!"},{"line_number":78,"context_line":"            raise trunk_exc.ParentPortInUse(port_id\u003dself.port_id)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return self.port_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_9e57e9c9","line":77,"updated":"2016-08-03 01:16:36.000000000","message":"I\u0027ll fix it for ya.","commit_id":"27280f63d7530d594612b6bd141c05b8be8b1d9f"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9ea0843fda8bb1ea97c6753ac30e712712c284bd","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        if trunks:"},{"line_number":75,"context_line":"            raise trunk_exc.ParentPortInUse(port_id\u003dself.port_id)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"        if self.is_bound(context) and False:  # \u003c--- nu-uh!!!"},{"line_number":78,"context_line":"            raise trunk_exc.ParentPortInUse(port_id\u003dself.port_id)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        return self.port_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_1e3444f9","line":77,"in_reply_to":"bacf61ea_9e57e9c9","updated":"2016-08-04 11:23:27.000000000","message":"you better!","commit_id":"27280f63d7530d594612b6bd141c05b8be8b1d9f"}]}