)]}'
{"neutron/plugins/ml2/managers.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"82d60cf58b88d211028b56e260896cfcf6ab386a","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"network_type value \u0027%s\u0027 not supported\"),"},{"line_number":105,"context_line":"                      network_type)"},{"line_number":106,"context_line":"            return"},{"line_number":107,"context_line":"        driver.obj.release_segment(session, segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F4l8%3D","line":104,"updated":"2014-04-09 17:42:40.000000000","message":"I think the error message needs a bit more context, and should begin with a capital letter. How about the following: \"Failed to release segment %(segment_id)s because network type \u0027%(type)s\u0027 is not supported\"?","commit_id":"8ccc26b2ca62e169987ab0784e349d0b55b77022"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"6c1b2f7d4d12be134f934f3c30068b65d45c4632","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"network_type value \u0027%s\u0027 not supported\"),"},{"line_number":105,"context_line":"                      network_type)"},{"line_number":106,"context_line":"            return"},{"line_number":107,"context_line":"        driver.obj.release_segment(session, segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2F4bQ%3D","line":104,"in_reply_to":"AAAAW3%2F%2F4l8%3D","updated":"2014-04-09 17:57:06.000000000","message":"Done","commit_id":"8ccc26b2ca62e169987ab0784e349d0b55b77022"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"0b159d012b97929b8da85cc9001894c69678c571","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment %(segment_id)s because \""},{"line_number":105,"context_line":"                        \"network type \u0027%(type)s\u0027 is not supported.\"),"},{"line_number":106,"context_line":"                      {\u0027segment_id\u0027: segment.get(api.SEGMENTATION_ID),"},{"line_number":107,"context_line":"                       \u0027type\u0027: network_type})"},{"line_number":108,"context_line":"            return"},{"line_number":109,"context_line":"        driver.obj.release_segment(session, segment)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2F4WA%3D","line":106,"updated":"2014-04-09 18:05:56.000000000","message":"This should be api.ID, not api.SEGMENTATION_ID. Not all types have a segmentation ID, and its probably not that useful by itself. If you want to be thorough, also include the network ID.","commit_id":"ad268ede399a513b9f811ffabc94fc165c66e726"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7aeb5ff7ea9b25058e0fbfe5e4e06162fafcb4f0","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment %(segment_id)s because \""},{"line_number":105,"context_line":"                        \"network type \u0027%(type)s\u0027 is not supported.\"),"},{"line_number":106,"context_line":"                      {\u0027segment_id\u0027: segment.get(api.SEGMENTATION_ID),"},{"line_number":107,"context_line":"                       \u0027type\u0027: network_type})"},{"line_number":108,"context_line":"            return"},{"line_number":109,"context_line":"        driver.obj.release_segment(session, segment)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2F4Os%3D","line":106,"in_reply_to":"AAAAW3%2F%2F4WA%3D","updated":"2014-04-09 18:15:42.000000000","message":"Actually, don\u0027t bother including the network ID since its not available in this function.","commit_id":"ad268ede399a513b9f811ffabc94fc165c66e726"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"456beda15ee2f327458501b87509e4f3d4da365f","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":100,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":105,"context_line":"                        \"network type is not supported.\"), segment)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAW3%2F%2Fzak%3D","line":102,"updated":"2014-04-10 10:00:44.000000000","message":"and does this mean will be left with a db entry for a segment that we can never remove?\nI think if the driver was removed from the configuration this would imply that anything that was backing it has been removed as well, so it should be safe to remove the db entry?","commit_id":"38aaff7057a3d062ac21167693e52624b6158553"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"208766b025d56d0356295c4f30e408cfa1843b83","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":100,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":105,"context_line":"                        \"network type is not supported.\"), segment)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAW3%2F%2FwRQ%3D","line":102,"in_reply_to":"AAAAW3%2F%2Fwjw%3D","updated":"2014-04-10 15:59:03.000000000","message":"Agreed. This is worth noted.","commit_id":"38aaff7057a3d062ac21167693e52624b6158553"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"914bfc19665dc341acfb07577d213881397a0426","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":100,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":105,"context_line":"                        \"network type is not supported.\"), segment)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAW3%2F%2Fwjw%3D","line":102,"in_reply_to":"AAAAW3%2F%2FxJE%3D","updated":"2014-04-10 15:28:40.000000000","message":"Nice explanation Bob!","commit_id":"38aaff7057a3d062ac21167693e52624b6158553"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7a3544fb3f02395f4d8e71bdaf8c27cd7415b0a4","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":100,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":101,"context_line":"        # ML2 may have been reconfigured since the segment was created,"},{"line_number":102,"context_line":"        # so a driver may no longer exist for this network_type."},{"line_number":103,"context_line":"        if not driver:"},{"line_number":104,"context_line":"            LOG.error(_(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":105,"context_line":"                        \"network type is not supported.\"), segment)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAW3%2F%2FxJE%3D","line":102,"in_reply_to":"AAAAW3%2F%2Fzak%3D","updated":"2014-04-10 14:33:57.000000000","message":"Good question Salvatore!\n\nThe segment table entries still get deleted via cascaded delete when the network is deleted, so the segment records themselves are not orphaned. Not calling the driver here just prevents the segment\u0027s resource (VLAN tag, tunnel ID, whatever) from being returned to a pool for reallocation. Without the type driver enabled, this reallocation is not possible, so it doesn\u0027t really matter until the driver is later re-enabled.\n\nThe existing type drivers that maintain these pools sync the allocation table with config at startup (in case ranges have changed), but don\u0027t sync with the segment tables at startup. So removing a type driver, deleting a network with a segment of that type, and later restoring that type driver would result in an orphaned record in the type driver\u0027s allocation table that would prevent the same segment from ever being used again. \n\nThese orphaned allocation records might be addressed by having each type driver issue a query at startup looking for segments in their allocation tables that are marked as in-use but have no corresponding record in the plugin\u0027s segment table, and deleting them from the allocation table. This would be easier if the allocation tables used a non-null segment_id instead of a boolean to indicate they are allocated, but it still may be possible.\n\nAlternatively, it would seem reasonable, when a type driver is removed from the plugin configuration (or just before its re-enabled), for the DB admin to manually clear the tables owned by that driver. Maybe neutron-db-manage or some similar tool could automate this, based on the current ML2 configuration.\n\nFinally, refactoring of type drivers is planned for the juno release that will move the type-specific data (such as segmentation_id and physical_network) from the ML2 segment table into driver-specific tables. This orphaned-allocation issue might be best addressed as part of this refactoring, by making the type-specific allocation tables use foreign keys and possibly cascaded deletes to ensure they clean up at the DB level when the network segment is deleted.\n\nFor now, I think this patch is sufficient and necessary, since it allows networks that no longer work after the type driver is disabled to be deleted successfully, and it is possible to clean things up manually before re-enabling the driver. It does seem that ML2 admin documentation should address modifying driver configuration in deployed systems.","commit_id":"38aaff7057a3d062ac21167693e52624b6158553"}],"neutron/tests/unit/ml2/test_ml2_plugin.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"0b159d012b97929b8da85cc9001894c69678c571","unresolved":false,"context_lines":[{"line_number":291,"context_line":"    def test_release_segment_no_type_driver(self):"},{"line_number":292,"context_line":"        segment \u003d {driver_api.NETWORK_TYPE: \u0027faketype\u0027,"},{"line_number":293,"context_line":"                   driver_api.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":294,"context_line":"                   driver_api.SEGMENTATION_ID: 1}"},{"line_number":295,"context_line":"        # this should not raise any exceptions"},{"line_number":296,"context_line":"        self.driver.type_manager.release_segment(session\u003dNone,"},{"line_number":297,"context_line":"                                                 segment\u003dsegment)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAW3%2F%2F4Vg%3D","line":294,"updated":"2014-04-09 18:05:56.000000000","message":"This should also be driver_api.ID.","commit_id":"ad268ede399a513b9f811ffabc94fc165c66e726"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"73b968b98ffd7415cac54978641f67b498faf40a","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                   driver_api.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":296,"context_line":"                   driver_api.ID: 1}"},{"line_number":297,"context_line":"        # this should not raise any exceptions"},{"line_number":298,"context_line":"        self.driver.type_manager.release_segment(session\u003dNone,"},{"line_number":299,"context_line":"                                                 segment\u003dsegment)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def test_create_provider_fail(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FonA%3D","line":298,"updated":"2014-04-11 14:36:09.000000000","message":"You could wrap this in\n\n    with mock.patch.object(ml2_plugin, \"LOG\") as log:\n\nThen afterwards check:\n\n    log.error.assert_called_once_with(\"Failed ...\" % segment)","commit_id":"d9a61b2a48a171ef31defc654a06d7beeff42686"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9a10d90745eb319e6277a40fb9005c9540c43fd6","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                   driver_api.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":296,"context_line":"                   driver_api.ID: 1}"},{"line_number":297,"context_line":"        # this should not raise any exceptions"},{"line_number":298,"context_line":"        self.driver.type_manager.release_segment(session\u003dNone,"},{"line_number":299,"context_line":"                                                 segment\u003dsegment)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def test_create_provider_fail(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2FnZs%3D","line":298,"in_reply_to":"AAAAW3%2F%2Fnxo%3D","updated":"2014-04-11 17:58:49.000000000","message":"Done","commit_id":"d9a61b2a48a171ef31defc654a06d7beeff42686"},{"author":{"_account_id":9412,"name":"Andres Buraschi","email":"andres.buraschi@intel.com","username":"aburaschi"},"change_message_id":"5a6a0f2a0493cd0d15926f5e9573a212a2dec506","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                   driver_api.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":296,"context_line":"                   driver_api.ID: 1}"},{"line_number":297,"context_line":"        # this should not raise any exceptions"},{"line_number":298,"context_line":"        self.driver.type_manager.release_segment(session\u003dNone,"},{"line_number":299,"context_line":"                                                 segment\u003dsegment)"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def test_create_provider_fail(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAW3%2F%2Fnxo%3D","line":298,"in_reply_to":"AAAAW3%2F%2FonA%3D","updated":"2014-04-11 16:40:23.000000000","message":"I agree. Test should have an assertion that evidences passing through the conditional block. (It improves readability too!)","commit_id":"d9a61b2a48a171ef31defc654a06d7beeff42686"}]}
