)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":333,"name":"Isaku Yamahata","email":"isaku.yamahata@gmail.com","username":"yamahata"},"change_message_id":"79d9798ba8db254f17afca86617c420341edbe4a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"If the network does not exist, the query that inserts the port will"},{"line_number":10,"context_line":"fail with a foreign key constraint failure.  This code is an attempt"},{"line_number":11,"context_line":"to detect that and throw the NetworkNotFound exception when it is"},{"line_number":12,"context_line":"caught."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I8c8bd63b8c755265f4ef75eb2d3e75b9638d52c8"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"baa201ad_1d91a252","line":12,"updated":"2014-10-10 09:20:26.000000000","message":"Can you please describe the motivation of the patch?\ndb performance gain?","commit_id":"ab7bb1c3dab6095f4a5a7f409c38d469374608ce"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3c27e525e632890840270880ef72d39f0e90c763","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        def wrap(self, context, port):"},{"line_number":1275,"context_line":"            try:"},{"line_number":1276,"context_line":"                return create_port(self, context, port)"},{"line_number":1277,"context_line":"            except db_exc.DBError as e:"},{"line_number":1278,"context_line":"                if e.inner_exception and ("},{"line_number":1279,"context_line":"                    \u0027foreign key constraint\u0027 in str(e.inner_exception)):"},{"line_number":1280,"context_line":"                    network_id\u003dport[\u0027port\u0027][\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_da02d9b3","line":1277,"updated":"2014-09-20 00:03:10.000000000","message":"Why not catch IntegrityError?","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a1fe1be599f9b7a0c1d2866ab4d9920d3d4d1b0c","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        def wrap(self, context, port):"},{"line_number":1275,"context_line":"            try:"},{"line_number":1276,"context_line":"                return create_port(self, context, port)"},{"line_number":1277,"context_line":"            except db_exc.DBError as e:"},{"line_number":1278,"context_line":"                if e.inner_exception and ("},{"line_number":1279,"context_line":"                    \u0027foreign key constraint\u0027 in str(e.inner_exception)):"},{"line_number":1280,"context_line":"                    network_id\u003dport[\u0027port\u0027][\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_56b5d2ce","line":1277,"in_reply_to":"baa201ad_6da4e407","updated":"2014-10-09 23:38:30.000000000","message":"The reason is that it doesn\u0027t actually throw IntegrityError directly.  It is the innerexception.  I did change this up a bit.  Let me know what you think.","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18fe9a15e3f05d9058e001b2a0dc8e400f53af2c","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        def wrap(self, context, port):"},{"line_number":1275,"context_line":"            try:"},{"line_number":1276,"context_line":"                return create_port(self, context, port)"},{"line_number":1277,"context_line":"            except db_exc.DBError as e:"},{"line_number":1278,"context_line":"                if e.inner_exception and ("},{"line_number":1279,"context_line":"                    \u0027foreign key constraint\u0027 in str(e.inner_exception)):"},{"line_number":1280,"context_line":"                    network_id\u003dport[\u0027port\u0027][\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_6da4e407","line":1277,"in_reply_to":"da9df570_da02d9b3","updated":"2014-10-09 03:17:08.000000000","message":"I\u0027ll look in to that.","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3c27e525e632890840270880ef72d39f0e90c763","unresolved":false,"context_lines":[{"line_number":1282,"context_line":"                raise"},{"line_number":1283,"context_line":"        return wrap"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"    @_detect_network_not_found"},{"line_number":1286,"context_line":"    def create_port(self, context, port):"},{"line_number":1287,"context_line":"        p \u003d port[\u0027port\u0027]"},{"line_number":1288,"context_line":"        port_id \u003d p.get(\u0027id\u0027) or uuidutils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_1a2c614a","line":1285,"updated":"2014-09-20 00:03:10.000000000","message":"Not sure I like this decorator business.","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18fe9a15e3f05d9058e001b2a0dc8e400f53af2c","unresolved":false,"context_lines":[{"line_number":1282,"context_line":"                raise"},{"line_number":1283,"context_line":"        return wrap"},{"line_number":1284,"context_line":""},{"line_number":1285,"context_line":"    @_detect_network_not_found"},{"line_number":1286,"context_line":"    def create_port(self, context, port):"},{"line_number":1287,"context_line":"        p \u003d port[\u0027port\u0027]"},{"line_number":1288,"context_line":"        port_id \u003d p.get(\u0027id\u0027) or uuidutils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_cd65d0e1","line":1285,"in_reply_to":"da9df570_1a2c614a","updated":"2014-10-09 03:17:08.000000000","message":"You\u0027re no fun.  :P","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"4be3cd6cf2d5a82c1748b760c2041d2604ee129d","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                                  status\u003dstatus,"},{"line_number":1329,"context_line":"                                  device_id\u003dp[\u0027device_id\u0027],"},{"line_number":1330,"context_line":"                                  device_owner\u003dp[\u0027device_owner\u0027])"},{"line_number":1331,"context_line":"            context.session.add(port)"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"            # Update the allocated IP\u0027s"},{"line_number":1334,"context_line":"            if ips:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da9df570_33186b84","line":1331,"updated":"2014-09-20 23:08:55.000000000","message":"Seems like it would be easier to catch the error and translate it here if this is the only place that the exception is thrown. This is all happening in a transaction anyway so any database work will be reverted by the exception.","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a1fe1be599f9b7a0c1d2866ab4d9920d3d4d1b0c","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                                  status\u003dstatus,"},{"line_number":1329,"context_line":"                                  device_id\u003dp[\u0027device_id\u0027],"},{"line_number":1330,"context_line":"                                  device_owner\u003dp[\u0027device_owner\u0027])"},{"line_number":1331,"context_line":"            context.session.add(port)"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"            # Update the allocated IP\u0027s"},{"line_number":1334,"context_line":"            if ips:"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_d6c8e258","line":1331,"in_reply_to":"baa201ad_0d5c3818","updated":"2014-10-09 23:38:30.000000000","message":"In the tests, this isn\u0027t precisely where the exception is thrown by sqlalchemy.  It is thrown by __exit__ on the session.begin(...).","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"18fe9a15e3f05d9058e001b2a0dc8e400f53af2c","unresolved":false,"context_lines":[{"line_number":1328,"context_line":"                                  status\u003dstatus,"},{"line_number":1329,"context_line":"                                  device_id\u003dp[\u0027device_id\u0027],"},{"line_number":1330,"context_line":"                                  device_owner\u003dp[\u0027device_owner\u0027])"},{"line_number":1331,"context_line":"            context.session.add(port)"},{"line_number":1332,"context_line":""},{"line_number":1333,"context_line":"            # Update the allocated IP\u0027s"},{"line_number":1334,"context_line":"            if ips:"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_0d5c3818","line":1331,"in_reply_to":"da9df570_33186b84","updated":"2014-10-09 03:17:08.000000000","message":"You\u0027re no fun either.  :P","commit_id":"1d4f72d1c6fed71aff80973a995d0f73fdcafbf1"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6c05b92208645c3d1a75ae9bf39b51929c34d5f4","unresolved":false,"context_lines":[{"line_number":1303,"context_line":"            self._enforce_device_owner_not_router_intf_or_device_id(context, p,"},{"line_number":1304,"context_line":"                                                                    tenant_id)"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        try:"},{"line_number":1307,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":1308,"context_line":"                # Ensure that a MAC address is defined and it is unique on the"},{"line_number":1309,"context_line":"                # network"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_824a7faa","line":1306,"updated":"2014-10-10 19:23:27.000000000","message":"Catching the error this way has the drawback of going through the hassle of doing the IP allocations only to roll that back if the integrity constraints occurs. I think it makes sense to optimize the code for the most common case, ie. where the network does exist: this saves us a DB call, and even though I wonder how much we really gain here...the performance-orientated patch is so trivial that I am okay punting on my request.\n\nI would have personally refactored the code to have a lot smaller try-catch, for instance, I would have broken out the mac handling and ip handling in separate units: better isolation, better testing. But Salvatore will tell you I am a refactoring freak so don\u0027t mind me.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7d50ee531b45590cf0a69b4572cb37a97ee9adf8","unresolved":false,"context_lines":[{"line_number":1303,"context_line":"            self._enforce_device_owner_not_router_intf_or_device_id(context, p,"},{"line_number":1304,"context_line":"                                                                    tenant_id)"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        try:"},{"line_number":1307,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":1308,"context_line":"                # Ensure that a MAC address is defined and it is unique on the"},{"line_number":1309,"context_line":"                # network"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_1c309900","line":1306,"in_reply_to":"9aa7fdbe_66d04a51","updated":"2014-10-21 17:33:25.000000000","message":"Thanks, I\u0027ll give it a try.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ae24dec0577817d6510e1e925a3f6c86ecc9978e","unresolved":false,"context_lines":[{"line_number":1303,"context_line":"            self._enforce_device_owner_not_router_intf_or_device_id(context, p,"},{"line_number":1304,"context_line":"                                                                    tenant_id)"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        try:"},{"line_number":1307,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":1308,"context_line":"                # Ensure that a MAC address is defined and it is unique on the"},{"line_number":1309,"context_line":"                # network"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_bdbec4d2","line":1306,"in_reply_to":"baa201ad_824a7faa","updated":"2014-10-10 19:48:14.000000000","message":"All good stuff.  Unlike you, I sleep!  One step at a time.  :P\n\nSeriously, though. I don\u0027t feel totally comfortable with the size of this try/catch either[*].  The problem was that a foreign key constraint error -- it seems -- is not raised until the transaction is committed which translates to the __exit__ call in the below with statement.  I guess the database doesn\u0027t really care about integrity in the middle of a transaction.\n\nIf you think we need more time on this, we can set it aside.  I don\u0027t mind.  I don\u0027t want to be responsible for adding bad code to the project when I can help it.\n\n[*] That was actually the motivation for that decorator attempt in previous patch sets.  Not that that attempt really made the situation better.  It just made me feel better at the time having a visibly smaller try/catch.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"c4fe46c0ffe952f6e9848cbbed496c566177f16c","unresolved":false,"context_lines":[{"line_number":1303,"context_line":"            self._enforce_device_owner_not_router_intf_or_device_id(context, p,"},{"line_number":1304,"context_line":"                                                                    tenant_id)"},{"line_number":1305,"context_line":""},{"line_number":1306,"context_line":"        try:"},{"line_number":1307,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":1308,"context_line":"                # Ensure that a MAC address is defined and it is unique on the"},{"line_number":1309,"context_line":"                # network"}],"source_content_type":"text/x-python","patch_set":4,"id":"9aa7fdbe_66d04a51","line":1306,"in_reply_to":"baa201ad_bdbec4d2","updated":"2014-10-21 04:38:30.000000000","message":"i prefer a smaller try-except block.\n\nisn\u0027t sqlalchemy \"flush\" operation is enough for your purpose?\n\nthe \"defer integration error until commit\" behavior is expensive for db\nto perform.  iirc it isn\u0027t the default behavior for postgresql at least.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6c05b92208645c3d1a75ae9bf39b51929c34d5f4","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"                        raise n_exc.MacAddressInUse(net_id\u003dnetwork_id,"},{"line_number":1321,"context_line":"                                                    mac\u003dp[\u0027mac_address\u0027])"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"                if \u0027status\u0027 not in p:"},{"line_number":1324,"context_line":"                    status \u003d constants.PORT_STATUS_ACTIVE"},{"line_number":1325,"context_line":"                else:"},{"line_number":1326,"context_line":"                    status \u003d p[\u0027status\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_a2edc390","line":1323,"updated":"2014-10-10 19:23:27.000000000","message":"(puke): this can be:\n\nstatus \u003d p.get(\u0027status\u0027, constants.PORT_STATUS_ACTIVE)\n\nno action required, but if you did it, you gets kudos from me.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ae24dec0577817d6510e1e925a3f6c86ecc9978e","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"                        raise n_exc.MacAddressInUse(net_id\u003dnetwork_id,"},{"line_number":1321,"context_line":"                                                    mac\u003dp[\u0027mac_address\u0027])"},{"line_number":1322,"context_line":""},{"line_number":1323,"context_line":"                if \u0027status\u0027 not in p:"},{"line_number":1324,"context_line":"                    status \u003d constants.PORT_STATUS_ACTIVE"},{"line_number":1325,"context_line":"                else:"},{"line_number":1326,"context_line":"                    status \u003d p[\u0027status\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_bd3b249b","line":1323,"in_reply_to":"baa201ad_a2edc390","updated":"2014-10-10 19:48:14.000000000","message":"I know that would be better but it shouldn\u0027t be done in this patch.  I\u0027l have to find another way to gets me some kudos.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"6c05b92208645c3d1a75ae9bf39b51929c34d5f4","unresolved":false,"context_lines":[{"line_number":1347,"context_line":"                                                               network_id,"},{"line_number":1348,"context_line":"                                                               subnet_id,"},{"line_number":1349,"context_line":"                                                               port_id)"},{"line_number":1350,"context_line":"        except db_exc.DBError as e:"},{"line_number":1351,"context_line":"            if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":1352,"context_line":"                raise n_exc.NetworkNotFound(net_id\u003dp[\u0027network_id\u0027])"},{"line_number":1353,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_62803b8c","line":1350,"updated":"2014-10-10 19:23:27.000000000","message":"cough cough: can we check whether this is covered? The only place I could find was:\n\n./unit/test_dhcp_rpc.py\n\nWhich might be enough...however, when it comes to Neutron we SHALL never assume that we have existing coverage!!","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ae24dec0577817d6510e1e925a3f6c86ecc9978e","unresolved":false,"context_lines":[{"line_number":1347,"context_line":"                                                               network_id,"},{"line_number":1348,"context_line":"                                                               subnet_id,"},{"line_number":1349,"context_line":"                                                               port_id)"},{"line_number":1350,"context_line":"        except db_exc.DBError as e:"},{"line_number":1351,"context_line":"            if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":1352,"context_line":"                raise n_exc.NetworkNotFound(net_id\u003dp[\u0027network_id\u0027])"},{"line_number":1353,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"baa201ad_fd5cccef","line":1350,"in_reply_to":"baa201ad_62803b8c","updated":"2014-10-10 19:48:14.000000000","message":"I did ensure that a test failed with the removal of L1306 expected the NetworkNotFound exception.  If you remove this try/catch or even just L1351/2 that test fails in the same way.\n\nneutron.tests.unit.vmware.extensions.test_networkgw.TestNetworkGateway.test_connect_invalid_network_returns_400 covers this case.  It fails expecting a 400 due to the NetworkNotFound exception being raised.\n\nIt isn\u0027t exactly in the main test suite, I know.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"726ea34f48473f7d006d3b1f70c335c21386274a","unresolved":false,"context_lines":[{"line_number":1347,"context_line":"                                                               network_id,"},{"line_number":1348,"context_line":"                                                               subnet_id,"},{"line_number":1349,"context_line":"                                                               port_id)"},{"line_number":1350,"context_line":"        except db_exc.DBError as e:"},{"line_number":1351,"context_line":"            if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":1352,"context_line":"                raise n_exc.NetworkNotFound(net_id\u003dp[\u0027network_id\u0027])"},{"line_number":1353,"context_line":"            raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_280df237","line":1350,"in_reply_to":"baa201ad_fd5cccef","updated":"2014-11-21 02:51:15.000000000","message":"given that we plan to move vendor UTs to separate repos,\nit\u0027s better to have a non-vendor UT case for this.","commit_id":"541acea9b0975449d3e39349605c452f014d3616"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"89fb0585d5df09e9a0d3c54cf252f2eb5a3e3818","unresolved":false,"context_lines":[{"line_number":1337,"context_line":"            try:"},{"line_number":1338,"context_line":"                context.session.add(db_port)"},{"line_number":1339,"context_line":"                context.session.flush()"},{"line_number":1340,"context_line":"            except db_exc.DBError as e:"},{"line_number":1341,"context_line":"                if isinstance(e.inner_exception, sql_exc.IntegrityError):"},{"line_number":1342,"context_line":"                    raise n_exc.NetworkNotFound(net_id\u003dp[\u0027network_id\u0027])"},{"line_number":1343,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_c8746e9e","line":1340,"updated":"2014-11-21 02:56:57.000000000","message":"probably worth a comment like:\n\"if the network doesn\u0027t exist, the above flush() triggers FK constraint failure.\"","commit_id":"80c93b684430e886453e1ee03d51f9d8f8475e48"}]}