)]}'
{"neutron/common/exceptions.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"2b1209b38f8c31e1cf64d75b7ba101a7b58cf600","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        super(NetworkVlanRangeError, self).__init__(**kwargs)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"class InternalError(NeutronException):"},{"line_number":286,"context_line":"    message \u003d _(\"Internal error\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F9Ec%3D","line":285,"updated":"2013-07-18 15:29:01.000000000","message":"Hmmm. This seems to stick out here. Either put it up at around line 48, or (my preference) keep it here at the end but call it TunnelSegmentationInternalError().","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7359176b4eb186090cb940b67ed55f02fd742b82","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        super(NetworkVlanRangeError, self).__init__(**kwargs)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"class InternalError(NeutronException):"},{"line_number":286,"context_line":"    message \u003d _(\"Internal error\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F8cY%3D","line":285,"in_reply_to":"AAAAQn%2F%2F9Aw%3D","updated":"2013-07-18 18:46:26.000000000","message":"I agree with Henry that this could go earlier in the module, since its a generic exception that can apply in lots of situations where a more-specific exception is not desired. But I\u0027m OK with it as is, but I\u0027d move it up if updating the patch for other reasons.\n\nIf we wanted a more-specific exception for this patch, TunnelSegmentationInternalError would be too specific - it could happen when any TypeDriver returns None from validate_provider_segment(), not just tunnel drivers. But the exceptions returned to clients should not unnecessarily expose deployment details such as which plugin and/or drivers are being used. Also, creating separate exception types for every possible case where an ml2 driver does not obey the rules documented in driver_api.py could result in lots of exception types that should never occur. Being more specific would provide no value to the client, so logging the details and raising a generic InternalError should be sufficient in my opinion.","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"},{"author":{"_account_id":2888,"name":"Mathieu Rohon","email":"mathieu.rohon@gmail.com","username":"mathieu-rohon"},"change_message_id":"ec07de857149ef44b02db17c6f5816fc810352cf","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        super(NetworkVlanRangeError, self).__init__(**kwargs)"},{"line_number":283,"context_line":""},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"class InternalError(NeutronException):"},{"line_number":286,"context_line":"    message \u003d _(\"Internal error\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F9Aw%3D","line":285,"in_reply_to":"AAAAQn%2F%2F9Ec%3D","updated":"2013-07-18 15:39:21.000000000","message":"I have followed the comment of bob on patch set 4, who wanted to create a general exception InternalError.","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"}],"neutron/plugins/ml2/drivers/type_gre.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"921677d7d5785e324928b9a25727b3b57f5c1ff1","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":77,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":78,"context_line":"        if physical_network:"},{"line_number":79,"context_line":"            msg \u003d _(\"provider:physical_network specified for GRE \""},{"line_number":80,"context_line":"                    \"network\")"},{"line_number":81,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fezw%3D","side":"PARENT","line":78,"updated":"2013-07-17 16:23:24.000000000","message":"This code raises an exception if a physical_network is specified, unlike the VlanTypeDriver that raises an exception if it is missing. I think we want to keep this check.","commit_id":"7efd7957a2125c4b079149cb5ba086f8b16e93de"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"c055bab3b97cd60a850bf5e623d3fd983dd5fcc4","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":"        self._sync_gre_allocations()"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":77,"context_line":"        return self._validate_provider_tunnel_segment(segment)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def reserve_provider_segment(self, session, segment):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F9ug%3D","line":76,"updated":"2013-07-18 12:02:16.000000000","message":"Remove completely, allow the base class to handle it.","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"}],"neutron/plugins/ml2/drivers/type_tunnel.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"c055bab3b97cd60a850bf5e623d3fd983dd5fcc4","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(_(\"%(type)s ID ranges: %(range)s\"),"},{"line_number":65,"context_line":"                 {\u0027type\u0027: tunnel_type, \u0027range\u0027: current_range})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _validate_provider_tunnel_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F9uI%3D","line":67,"updated":"2013-07-18 12:02:16.000000000","message":"Remove the leading underscore. Let this be the public method for tunnel types.","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6515f526a050f19bed18fbb0c3113aa3285d7a70","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(_(\"%(type)s ID ranges: %(range)s\"),"},{"line_number":65,"context_line":"                 {\u0027type\u0027: tunnel_type, \u0027range\u0027: current_range})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def _validate_provider_tunnel_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F9ew%3D","line":67,"in_reply_to":"AAAAQn%2F%2F9uI%3D","updated":"2013-07-18 13:33:11.000000000","message":"My comment is not valid, since this is an ABC.","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"},{"author":{"_account_id":7170,"name":"Zang MingJie","email":"zealot0630@gmail.com","username":"zealot"},"change_message_id":"8c3227a7977470bf93af051d7a3cc7cc574db8eb","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        segmentation_id \u003d segment.get(api.SEGMENTATION_ID)"},{"line_number":75,"context_line":"        if segmentation_id is None:"},{"line_number":76,"context_line":"            msg \u003d _(\"segmentation_id required for tunnel provider network\")"},{"line_number":77,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":78,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F%2BBY%3D","line":75,"updated":"2013-07-18 11:13:18.000000000","message":"if not segmentation_id","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"84c28d5eb995e263337c6afb9f444a34afe614ce","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(_(\"%(type)s ID ranges: %(range)s\"),"},{"line_number":65,"context_line":"                 {\u0027type\u0027: tunnel_type, \u0027range\u0027: current_range})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def validate_provider_tunnel_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9T0%3D","line":67,"updated":"2013-07-18 14:23:44.000000000","message":"Could this be used directly (rather than called by the derived class) if it were named validate_provider_segment()?","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"36cda4f028a7194df8af24fa207351cbb4b64b50","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(_(\"%(type)s ID ranges: %(range)s\"),"},{"line_number":65,"context_line":"                 {\u0027type\u0027: tunnel_type, \u0027range\u0027: current_range})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def validate_provider_tunnel_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F8Zs%3D","line":67,"in_reply_to":"AAAAQn%2F%2F9QE%3D","updated":"2013-07-18 18:37:54.000000000","message":"I didn\u0027t notice that TunnelTypeDriver was not derived from TypeDriver. I\u0027m OK with the current approach, but adding this inheritance (and making TunnelTypeDriver abstract) might be worth a try.","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":2888,"name":"Mathieu Rohon","email":"mathieu.rohon@gmail.com","username":"mathieu-rohon"},"change_message_id":"c696a42342f78fa9df9f27da8bc2734507cde269","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        LOG.info(_(\"%(type)s ID ranges: %(range)s\"),"},{"line_number":65,"context_line":"                 {\u0027type\u0027: tunnel_type, \u0027range\u0027: current_range})"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def validate_provider_tunnel_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9QE%3D","line":67,"in_reply_to":"AAAAQn%2F%2F9T0%3D","updated":"2013-07-18 14:40:00.000000000","message":"I\u0027ve tried to implement validate_provider_network in TunnelTypeDriver directly, but the result is an error because the abstract method validate_provider_network must be implemented by the class that implement TypeDriver.","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7359176b4eb186090cb940b67ed55f02fd742b82","unresolved":false,"context_lines":[{"line_number":24,"context_line":"TUNNEL \u003d \u0027tunnel\u0027"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class TunnelTypeDriver(object):"},{"line_number":28,"context_line":"    \"\"\"Define stable abstract interface for ML2 type drivers."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    tunnel type networks rely on tunnel endpoints. This class defines abstract"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F8Rk%3D","line":27,"updated":"2013-07-18 18:46:26.000000000","message":"I think a validate_provider_segment() function could be implemented directly in TunnelTypeDriver if it inherited from TypeDriver.","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"d67f6832ce40be03eba8630a4077c8683335e3be","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""},{"line_number":71,"context_line":"                    \"network\")"},{"line_number":72,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2F5io%3D","line":70,"updated":"2013-07-19 15:51:36.000000000","message":"It seems that this message is shown for both vxlan and gre drivers after they inherit from TunnelTypeDriver.\nCould you clarify?","commit_id":"8e4b78cd8488755b00d229d9b888cbc7777e9bd3"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"fb26a941549abbce450f7138b0587bf06ddf4902","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""},{"line_number":71,"context_line":"                    \"network\")"},{"line_number":72,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2F5gE%3D","line":70,"in_reply_to":"AAAAQn%2F%2F5io%3D","updated":"2013-07-19 16:29:31.000000000","message":"Good catch! Could just replace \"VXLAN\" in msg with \"tunnel\", or substitute in segment[api.NETWORK_TYPE]. If the latter, please do the same for the other exception messages below as well.","commit_id":"8e4b78cd8488755b00d229d9b888cbc7777e9bd3"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"86b3f7207aab8fdf59c92439b7d40c2612fa5963","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":68,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":69,"context_line":"        if physical_network:"},{"line_number":70,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""},{"line_number":71,"context_line":"                    \"network\")"},{"line_number":72,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2F5fk%3D","line":70,"in_reply_to":"AAAAQn%2F%2F5io%3D","updated":"2013-07-19 16:09:52.000000000","message":"Good catch. This should say \u0027tunnel\u0027 instead of \u0027VXLAN\u0027.","commit_id":"8e4b78cd8488755b00d229d9b888cbc7777e9bd3"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"d67f6832ce40be03eba8630a4077c8683335e3be","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            msg \u003d _(\"segmentation_id required for tunnel provider network\")"},{"line_number":77,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        for key, value in segment.iteritems():"},{"line_number":80,"context_line":"            if value and key not in [api.NETWORK_TYPE,"},{"line_number":81,"context_line":"                                     api.SEGMENTATION_ID]:"},{"line_number":82,"context_line":"                msg \u003d _(\"%s prohibited for tunnel provider network\") % key"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAQn%2F%2F5i8%3D","line":79,"updated":"2013-07-19 15:51:36.000000000","message":"suggestion: items() instead of iteritems() for py3 compatibility.","commit_id":"8e4b78cd8488755b00d229d9b888cbc7777e9bd3"}],"neutron/plugins/ml2/drivers/type_vxlan.py":[{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"921677d7d5785e324928b9a25727b3b57f5c1ff1","unresolved":false,"context_lines":[{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":84,"context_line":"        physical_network \u003d segment.get(api.PHYSICAL_NETWORK)"},{"line_number":85,"context_line":"        if physical_network:"},{"line_number":86,"context_line":"            msg \u003d _(\"provider:physical_network specified for VXLAN \""},{"line_number":87,"context_line":"                    \"network\")"},{"line_number":88,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fey4%3D","side":"PARENT","line":85,"updated":"2013-07-17 16:23:24.000000000","message":"As in type_gre.py, I think we want  to keep this code.","commit_id":"7efd7957a2125c4b079149cb5ba086f8b16e93de"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"c055bab3b97cd60a850bf5e623d3fd983dd5fcc4","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        )"},{"line_number":81,"context_line":"        self._sync_vxlan_allocations()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    def validate_provider_segment(self, segment):"},{"line_number":84,"context_line":"        return self._validate_provider_tunnel_segment(segment)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def reserve_provider_segment(self, session, segment):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F9uE%3D","line":83,"updated":"2013-07-18 12:02:16.000000000","message":"Remove completely, allow the base class to handle it.","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"c055bab3b97cd60a850bf5e623d3fd983dd5fcc4","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            segment \u003d self.type_manager.validate_provider_segment(segment)"},{"line_number":131,"context_line":"            if not segment:"},{"line_number":132,"context_line":"                msg \u003d _(\"specified provider network attributes are invalid\")"},{"line_number":133,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"            return segment"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAQn%2F%2F9vE%3D","line":132,"updated":"2013-07-18 12:02:16.000000000","message":"Can we ever reach here? Not with the current code AFAICT but I am OK with keeping it as a belt\u0026suspenders approach in case the underlying code changes in the future.\n\nIf so, please print segment as part of the message.","commit_id":"b8f25d769a656f6690abd675b1d246248887fc55"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"041048a09653ee1c8e623b9c8daea88e3a300a69","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                         \"%(phy); segmentation_id : %(seg)\"),"},{"line_number":135,"context_line":"                       {\u0027network\u0027: network_type,"},{"line_number":136,"context_line":"                        \u0027phy\u0027: physical_network,"},{"line_number":137,"context_line":"                        \u0027seg\u0027: segmentation_id})"},{"line_number":138,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            return segment"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAQn%2F%2F9do%3D","line":137,"updated":"2013-07-18 13:44:11.000000000","message":"I think it would simpler to just print the segment dict directly, so there is no risk of the keys being translated.\n\n    msg(_(\"... are invalid: %s\"), original_segment)","commit_id":"2f61a4490f20b7f9c99c4874f19026c45cf28441"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"023ad720a48233002d6754114c99bdd26bd136bd","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                       api.PHYSICAL_NETWORK: physical_network,"},{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            valid_seg \u003d self.type_manager.validate_provider_segment(segment)"},{"line_number":131,"context_line":"            if not valid_seg:"},{"line_number":132,"context_line":"                msg \u003d (_(\"specified provider network attributes are invalid. \""},{"line_number":133,"context_line":"                         \"segment: %s\"), segment)"},{"line_number":134,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9Vw%3D","line":131,"updated":"2013-07-18 14:15:48.000000000","message":"You could do\n\n  if not self.type_manager.validate_provider_segment(segment):","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"36cda4f028a7194df8af24fa207351cbb4b64b50","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                       api.PHYSICAL_NETWORK: physical_network,"},{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            valid_seg \u003d self.type_manager.validate_provider_segment(segment)"},{"line_number":131,"context_line":"            if not valid_seg:"},{"line_number":132,"context_line":"                msg \u003d (_(\"specified provider network attributes are invalid. \""},{"line_number":133,"context_line":"                         \"segment: %s\"), segment)"},{"line_number":134,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F8TE%3D","line":131,"in_reply_to":"AAAAQn%2F%2F9QA%3D","updated":"2013-07-18 18:37:54.000000000","message":"Note that validate_provider_segment() returns the segment to use, which might be the same one passed in, the one passed in with modifications, or a replacement of some sort.\n\nNow that I think about it, having the TypeDriver return a different segment isn\u0027t all that useful since it can just modify the one passed in if needed. Maybe we should change driver_api.py so that TypeDriver.validate_provider_segment() does not return anything, and then we wouldn\u0027t need to check what it returned. We\u0027d need to modify the documentation in driver_api.py to indicate this, and change TypeManager.validate_provider_segment() as well.","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":2888,"name":"Mathieu Rohon","email":"mathieu.rohon@gmail.com","username":"mathieu-rohon"},"change_message_id":"c696a42342f78fa9df9f27da8bc2734507cde269","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                       api.PHYSICAL_NETWORK: physical_network,"},{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            valid_seg \u003d self.type_manager.validate_provider_segment(segment)"},{"line_number":131,"context_line":"            if not valid_seg:"},{"line_number":132,"context_line":"                msg \u003d (_(\"specified provider network attributes are invalid. \""},{"line_number":133,"context_line":"                         \"segment: %s\"), segment)"},{"line_number":134,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9QA%3D","line":131,"in_reply_to":"AAAAQn%2F%2F9Vw%3D","updated":"2013-07-18 14:40:00.000000000","message":"Done","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"f527186dd27eb95fe9375b7528ed8f5d44b6de86","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            if not valid_seg:"},{"line_number":132,"context_line":"                msg \u003d (_(\"specified provider network attributes are invalid. \""},{"line_number":133,"context_line":"                         \"segment: %s\"), segment)"},{"line_number":134,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"            return segment"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9WY%3D","line":134,"updated":"2013-07-18 14:19:30.000000000","message":"The TypeDriver API documentation specifies that validate_provider_segment() must raise InvalidInput itself if the set of attributes is invalid for any reason - it should never return None. Therefore, we only hit this case if there is a bug in a TypeDriver. If this does happen, I think we should log the error (including the segment details), and raise an InternalError exception (without the segment details). This is not a case where the user provided invalid input, so returning InvalidInput would be misleading. We don\u0027t generally want to return details to clients exposing what plugin/driver is being used, and no existing exception seems to fit, so a new generic InternalError exception derived from NeutronException with \"internal error\" as the description seems appropriate.\n\nIn general, I\u0027m not sure how defensive we need the ml2 plugin to be against driver implementations that don\u0027t correctly implement the specification. In simple cases like this, logging an error and raising InternalError seems helpful to driver developers. But I don\u0027t think we need to go to extremes. Thoughts?","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":2888,"name":"Mathieu Rohon","email":"mathieu.rohon@gmail.com","username":"mathieu-rohon"},"change_message_id":"c696a42342f78fa9df9f27da8bc2734507cde269","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            if not valid_seg:"},{"line_number":132,"context_line":"                msg \u003d (_(\"specified provider network attributes are invalid. \""},{"line_number":133,"context_line":"                         \"segment: %s\"), segment)"},{"line_number":134,"context_line":"                raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"            return segment"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAQn%2F%2F9Po%3D","line":134,"in_reply_to":"AAAAQn%2F%2F9WY%3D","updated":"2013-07-18 14:40:00.000000000","message":"i\u0027m agree with you, this simple check is not very costly.","commit_id":"7872332a15500d41dc5cb012c67e4c07c005094c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7359176b4eb186090cb940b67ed55f02fd742b82","unresolved":false,"context_lines":[{"line_number":127,"context_line":"            segment \u003d {api.NETWORK_TYPE: network_type,"},{"line_number":128,"context_line":"                       api.PHYSICAL_NETWORK: physical_network,"},{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            if not self.type_manager.validate_provider_segment(segment):"},{"line_number":131,"context_line":"                LOG.error(_(\"specified provider network attributes are \""},{"line_number":132,"context_line":"                            \"invalid. segment: %s\"), segment)"},{"line_number":133,"context_line":"                raise exc.InternalError()"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F8Sk%3D","line":130,"updated":"2013-07-18 18:46:26.000000000","message":"As currently defined in driver_api.py, the segment to be used is returned from the function. This might be different than the one passed in. The result needs to be assigned into segment, which gets returned below.\n\nSee my new comment on the previous patch set. Maybe we should change driver_api.py and the existing TypeDriver implementations so that they no longer return anything, and just modify the passed-in segment if necessary.","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"},{"author":{"_account_id":1689,"name":"Robert Kukura","email":"rk@theep.net","username":"rkukura"},"change_message_id":"7359176b4eb186090cb940b67ed55f02fd742b82","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                       api.SEGMENTATION_ID: segmentation_id}"},{"line_number":130,"context_line":"            if not self.type_manager.validate_provider_segment(segment):"},{"line_number":131,"context_line":"                LOG.error(_(\"specified provider network attributes are \""},{"line_number":132,"context_line":"                            \"invalid. segment: %s\"), segment)"},{"line_number":133,"context_line":"                raise exc.InternalError()"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"            return segment"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAQn%2F%2F8bI%3D","line":132,"updated":"2013-07-18 18:46:26.000000000","message":"This error message is misleading. If the client actually supplied invalid attributes, the driver should have raised an InvalidInput exception describing the issue. How about something like \"TypeDriver.validate_provider_segment() returned None for segment: %s\"?","commit_id":"f29c5d459e2dd6076000edb5583fee165ed1238c"}]}