)]}'
{"neutron/db/ipam_backend_mixin.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"110150231a939a88d2c5cf269fc0c8d758fdb6dc","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                 enable_eagerloads(False).filter_by(id\u003dport_id))"},{"line_number":415,"context_line":"        if not context.is_admin:"},{"line_number":416,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":417,"context_line":"        context.session.delete(query.first())"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _save_subnet(self, context,"},{"line_number":420,"context_line":"                     network,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_d596593e","side":"PARENT","line":417,"updated":"2015-09-16 20:44:00.000000000","message":"looks like we regressed somehow from the way this used to be:\n\nhttps://review.openstack.org/#/c/81196/1/neutron/db/db_base_plugin_v2.py","commit_id":"2195501e9706de7a9127771187da4981d692eb7d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"fadaf0c82d1583974583593da2fb6b1253fa2f4d","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                 enable_eagerloads(False).filter_by(id\u003dport_id))"},{"line_number":415,"context_line":"        if not context.is_admin:"},{"line_number":416,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":417,"context_line":"        context.session.delete(query.first())"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _save_subnet(self, context,"},{"line_number":420,"context_line":"                     network,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_ab62683c","side":"PARENT","line":417,"in_reply_to":"ba15a1d1_4bddcc6d","updated":"2015-09-16 21:48:54.000000000","message":"/me throws up a little.","commit_id":"2195501e9706de7a9127771187da4981d692eb7d"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"9624fee1da38a1653462c58c3cb37e5f9ca09fa1","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                 enable_eagerloads(False).filter_by(id\u003dport_id))"},{"line_number":415,"context_line":"        if not context.is_admin:"},{"line_number":416,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":417,"context_line":"        context.session.delete(query.first())"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _save_subnet(self, context,"},{"line_number":420,"context_line":"                     network,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_4bddcc6d","side":"PARENT","line":417,"in_reply_to":"ba15a1d1_d596593e","updated":"2015-09-16 21:26:03.000000000","message":"Correct, previous it used one: \n\n port \u003d query.with_lockmode(\u0027update\u0027).one()","commit_id":"2195501e9706de7a9127771187da4981d692eb7d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"19690fb951166dc4a703aeb802daa61e694c5770","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_5a0cb840","line":419,"updated":"2015-09-16 20:34:07.000000000","message":"are we ok with raising an exception here? We didn\u0027t before.\n\nWouldn\u0027t a safer fix issuing a delete on first() being not none? Having said that we should just issue a delete like [1], because it makes no sense to select and delete, it\u0027s racy, and we want to kill the race entirely:\n\nhttp://docs.sqlalchemy.org/en/rel_1_0/orm/query.html#sqlalchemy.orm.query.Query.delete","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f2659c3b2e6966bde66ddc5a3888fd6fad8a098c","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_4613815b","line":419,"in_reply_to":"ba15a1d1_0ba6f452","updated":"2015-09-16 21:53:06.000000000","message":"the reason why I say that is because the existing code may call delete_port and not except the NotFound exception","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"9624fee1da38a1653462c58c3cb37e5f9ca09fa1","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_8b5be4f5","line":419,"in_reply_to":"ba15a1d1_30756363","updated":"2015-09-16 21:26:03.000000000","message":"sounds good to me I\u0027ll switch it to this.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"fc1dc05711bdf6b8d76622538e7f0a33394d8613","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_e69df566","line":419,"in_reply_to":"ba15a1d1_4613815b","updated":"2015-09-16 22:11:15.000000000","message":"The existing api is raising UnmappedInstanceError when a non-existing port is deleted, the previous behavior raised a NoResultFound.  If we want to maintain that the previous behavior we should just delete the exception handing i added below.  I think it\u0027s important for the caller of this method to know that the port was already removed from the database.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"3919ae5005501c7163c34f7adf427728a50cb889","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_30756363","line":419,"in_reply_to":"ba15a1d1_5a0cb840","updated":"2015-09-16 21:00:55.000000000","message":"+1, query.delete(synchronize_session\u003dFalse) is safer and faster","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"f311b075d441ab52ba5e020b95807ab2051627a9","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_8b852408","line":419,"in_reply_to":"ba15a1d1_6bb21008","updated":"2015-09-16 21:33:46.000000000","message":"@Armando, also it looks we were raising an exception before as:    port \u003d query.with_lockmode(\u0027update\u0027).one() would have caused an exc.NoResultFound to be raised this code changes it to raise a neutron PortNotFound exception instead which i think is better anyways.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"fadaf0c82d1583974583593da2fb6b1253fa2f4d","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_0ba6f452","line":419,"in_reply_to":"ba15a1d1_6bb21008","updated":"2015-09-16 21:48:54.000000000","message":"but then you\u0027re altering the behavior of the API. The existing API is not complaining on a non-existent port. I think this should stay this way, at least in the context of this bug fix. If you want to change the semantic, then we can yak-shave on another patch.\n\nI personally think that the delete semantic can allow for both \u0027error on not-found\u0027 or \u0027silently-fail\u0027...this can take a while to yak-shave","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"8649d6f92ddb922c96fb6fc327062fa20fce6890","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_6bb21008","line":419,"in_reply_to":"ba15a1d1_8b5be4f5","updated":"2015-09-16 21:30:23.000000000","message":"Actually on second thought i think it makes sense to actually raise an exception here this way the caller knows that a race occurred and the port has already been deleted from the db.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"fadaf0c82d1583974583593da2fb6b1253fa2f4d","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_eb562029","line":419,"in_reply_to":"ba15a1d1_8b852408","updated":"2015-09-16 21:48:54.000000000","message":"that was changed a while back; again I ultimately I think these are two separate changes.\n\nhaving delete_port raise on not found is a different issue from fixing the delete on none.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"784e4200a4a6cec4d83eb359fccd6c59d3395006","unresolved":false,"context_lines":[{"line_number":416,"context_line":"        if not context.is_admin:"},{"line_number":417,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            context.session.delete(query.one())"},{"line_number":420,"context_line":"        except exc.NoResultFound:"},{"line_number":421,"context_line":"            raise n_exc.PortNotFound(port_id\u003dport_id)"},{"line_number":422,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_6681e5c2","line":419,"in_reply_to":"ba15a1d1_e69df566","updated":"2015-09-16 22:19:40.000000000","message":"the UnmappedInstanceError was the effect of a bug, first() doesn\u0027t raise.\n\nAssuming that race conditions are exceptions rather than the norm we should assume that the intention of this code path is not to raise a not found when the port is missing. The caller shouldn\u0027t care, the objective of his request has already been fulfilled.","commit_id":"5af49162fc03e5c20fdbc706ba99b9e0113bc8c2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"2d6f7af7d4d15a86c300f5375ab1a2925bdf6752","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                 enable_eagerloads(False).filter_by(id\u003dport_id))"},{"line_number":415,"context_line":"        if not context.is_admin:"},{"line_number":416,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":417,"context_line":"        query.delete(synchronize_session\u003dFalse)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _save_subnet(self, context,"},{"line_number":420,"context_line":"                     network,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_a7413865","line":417,"updated":"2015-09-16 23:44:25.000000000","message":"just for reference (something we gotta document for ourselves), when using bulky deletes there are some caveats:\n\nhttp://docs.sqlalchemy.org/en/rel_1_0/orm/query.html#sqlalchemy.orm.query.Query.delete\n\nMore importantly is that MapperEvents.before_delete() and MapperEvents.after_delete() are not invoked with this delete mechanism. Now we don\u0027t currently rely on these, hence this patch is fine, but we have examples in the code where we rely on sqlalchemy\u0027s triggers (for port updates to be sent to nova). Should we add listeners to delete events, then we\u0027d be in trouble.","commit_id":"77de4a3b54110c2d5b8b38efdfbeb5c35e2cb16d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a3b4cd9f5888cd26b537b6d90aa36df9b17ffb9e","unresolved":false,"context_lines":[{"line_number":414,"context_line":"                 enable_eagerloads(False).filter_by(id\u003dport_id))"},{"line_number":415,"context_line":"        if not context.is_admin:"},{"line_number":416,"context_line":"            query \u003d query.filter_by(tenant_id\u003dcontext.tenant_id)"},{"line_number":417,"context_line":"        query.delete(synchronize_session\u003dFalse)"},{"line_number":418,"context_line":""},{"line_number":419,"context_line":"    def _save_subnet(self, context,"},{"line_number":420,"context_line":"                     network,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_92216cb7","line":417,"in_reply_to":"ba15a1d1_a7413865","updated":"2015-09-19 00:45:14.000000000","message":"I should eat my shoe:\n\nhttps://review.openstack.org/#/c/225349/","commit_id":"77de4a3b54110c2d5b8b38efdfbeb5c35e2cb16d"}]}