)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"d14fa927842627177192bba0a0c1a49ce5c1ac7a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ann Kamyshnikova \u003cakamyshnikova@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-05 13:02:26 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Avoid rollbacked transactions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For places like this:"},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a1a9d01_4708952d","line":7,"updated":"2015-10-05 10:29:18.000000000","message":"nit: Avoid working with the transaction that has been rolled back","commit_id":"32ffb21b32831450463bcc199ec812d911a5adfc"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"6e10d5fcd3fffc16463b2b663fb281be3be3f8b5","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ann Kamyshnikova \u003cakamyshnikova@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-10-05 13:02:26 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Avoid rollbacked transactions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For places like this:"},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a2fa921_7cacb1ed","line":7,"in_reply_to":"9a1a9d01_4708952d","updated":"2015-10-05 14:40:37.000000000","message":"Done","commit_id":"32ffb21b32831450463bcc199ec812d911a5adfc"},{"author":{"_account_id":11682,"name":"Ryan Moats","email":"rmoats@us.ibm.com","username":"regXboi"},"change_message_id":"a47aec6d47ea41b61710f0bdcd7e746505fbd730","unresolved":false,"context_lines":[{"line_number":24,"context_line":"is caught in except block, but the object cannot be deleted in"},{"line_number":25,"context_line":"except section because internal transaction has been rolled back."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The possible solution is to use nested transaction."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I952f6f7f8684743aa7f829bd92b1dc41b2c6aecf"},{"line_number":30,"context_line":"Closes-bug: #1501686"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a1a9d01_b0a440b9","line":27,"updated":"2015-10-05 13:27:12.000000000","message":"nit: the proposed solution","commit_id":"32ffb21b32831450463bcc199ec812d911a5adfc"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"6e10d5fcd3fffc16463b2b663fb281be3be3f8b5","unresolved":false,"context_lines":[{"line_number":24,"context_line":"is caught in except block, but the object cannot be deleted in"},{"line_number":25,"context_line":"except section because internal transaction has been rolled back."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"The possible solution is to use nested transaction."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: I952f6f7f8684743aa7f829bd92b1dc41b2c6aecf"},{"line_number":30,"context_line":"Closes-bug: #1501686"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a2fa921_7c9351a4","line":27,"in_reply_to":"9a1a9d01_b0a440b9","updated":"2015-10-05 14:40:37.000000000","message":"Done","commit_id":"32ffb21b32831450463bcc199ec812d911a5adfc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ab229a15c540c2cc05f5730913a8eaa4036073e2","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Methods _create_ha_network, add_ha_port, _create_ha_interfaces"},{"line_number":10,"context_line":"don\u0027t have wrapping transaction in them, so they are prone"},{"line_number":11,"context_line":"to race conditions. This commit adds a transaction to them."},{"line_number":12,"context_line":"To avoid problem with rolling back outmost transaction during"},{"line_number":13,"context_line":"exception handling, internal methods have been wrapped"},{"line_number":14,"context_line":"in nested transaction."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"da6ed579_9274fdcf","line":11,"range":{"start_line":11,"start_character":8,"end_line":11,"end_character":18},"updated":"2016-01-12 03:09:56.000000000","message":"After all, behind so many patchs, you are going to deal with the race condition too since patch set 17. So I don\u0027t think this patch is still needed after we can figure out what caused the race and which exceptions the race raised.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3654e32df421db91dafa9eb4b74c6ac68f774323","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Methods _create_ha_network, add_ha_port, _create_ha_interfaces"},{"line_number":10,"context_line":"don\u0027t have wrapping transaction in them, so they are prone"},{"line_number":11,"context_line":"to race conditions. This commit adds a transaction to them."},{"line_number":12,"context_line":"To avoid problem with rolling back outmost transaction during"},{"line_number":13,"context_line":"exception handling, internal methods have been wrapped"},{"line_number":14,"context_line":"in nested transaction."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":22,"id":"da6ed579_9713865c","line":11,"range":{"start_line":11,"start_character":8,"end_line":11,"end_character":18},"in_reply_to":"da6ed579_9274fdcf","updated":"2016-01-12 07:45:08.000000000","message":"If all our actions are atomic (or transactional) in nature we can\u0027t have any race conditions. We need to be cautious not only about races that we have identified but also about those that can happen in future or are yet to be seen. This approach guarantees that our state is consistent in any case.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"}],"doc/source/devref/effective_neutron.rst":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"9a063ead858878bb2f9e5d176e6b12e5a4028340","unresolved":false,"context_lines":[{"line_number":100,"context_line":"               func.count(Object.number)).group_by(Object.id, Object.name)"},{"line_number":101,"context_line":"* Beware of the `InvalidRequestError \u003chttp://docs.sqlalchemy.org/en/rel_0_8/faq.html#this-session-s-transaction-has-been-rolled-back-due-to-a-previous-exception-during-flush-or-similar\u003e`_ exception."},{"line_number":102,"context_line":"  There is even a `Neutron bug \u003chttps://bugs.launchpad.net/neutron/+bug/1409774\u003e`_"},{"line_number":103,"context_line":"  registered for it. Bear in mind that this error may also occur when nesting"},{"line_number":104,"context_line":"  transaction blocks, and the innermost block raises an error without proper"},{"line_number":105,"context_line":"  rollback. Consider if `savepoints \u003chttp://docs.sqlalchemy.org/en/rel_1_0/orm/session_transaction.html#using-savepoint\u003e`_"},{"line_number":106,"context_line":"  can fit your use case."},{"line_number":107,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"da85f559_2553c13a","line":104,"range":{"start_line":103,"start_character":70,"end_line":104,"end_character":21},"updated":"2015-11-12 01:38:44.000000000","message":"\"nesting transcation block\" made me think nested transactions. (\u003d\u003d savepoints)\n\ni guess it\u0027s better to avoid the word \"nest\", or have an explicit note to say this doesn\u0027t mean nested transactions.","commit_id":"2ea39515524bbbf41036156152e1a935b907ad94"},{"author":{"_account_id":11816,"name":"mike_mp@zzzcomputing.com","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"7b03aabe4a1f603f4bfdbf6cdb37cb828bb4f511","unresolved":false,"context_lines":[{"line_number":129,"context_line":"  .. code:: python"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"     def create():"},{"line_number":132,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":133,"context_line":"          create_something()"},{"line_number":134,"context_line":"          try:"},{"line_number":135,"context_line":"              _do_other_thing()"}],"source_content_type":"text/x-rst","patch_set":10,"id":"9a8ffd7b_eaf686d6","line":132,"updated":"2015-11-24 23:16:51.000000000","message":"ok this is not actually what SQLA refers to as a \"nested\" transaction.  \"nested\" means we\u0027re emitting a SAVEPOINT.  \"subtransactions\" only means there\u0027s a logical nesting of begin/commit, such that if we called a function, it could also say begin()/commit() internally and not care that it was part of a larger block.\n\n**BUT**.   With subtransactions, if *any* part of the code emits rollback(), *the actual DB transaction is rolled back*.  So the work you do in delete_something() would proceed within a new transaction, assuming it does something with the database.     But then when you re-raise, it would rollback() again, so you\u0027d want delete_something() to also *commit()* what it\u0027s doing internally.\n\nUsing the new enginefacade, which is where neutron and all openstack should go, i think delete_something() would simply be inside of @enginefacade.writer.independent - so it\u0027s totally independent of the enclosing transaction.\n\nThat or, we\u0027re using savepoints.  The issue we have with MySQL failing on savepoint rollback and then not allowing us to show the original error has a fix in SQLA 1.0.10 where we will at least show a warning of the original error in the logs.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ab3fc542b615a01d118db4eada8a897adff31b38","unresolved":false,"context_lines":[{"line_number":129,"context_line":"  .. code:: python"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"     def create():"},{"line_number":132,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":133,"context_line":"          create_something()"},{"line_number":134,"context_line":"          try:"},{"line_number":135,"context_line":"              _do_other_thing()"}],"source_content_type":"text/x-rst","patch_set":10,"id":"9a8ffd7b_a6042d7e","line":132,"in_reply_to":"9a8ffd7b_eaf686d6","updated":"2015-11-25 08:22:08.000000000","message":"This is not what I refer to as a \"nested\" transaction either. The transaction in _do_other_thing is. :)","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"950e702ebacb1f6c33804d8200036d5c2856dbf2","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":133,"context_line":"          create_something()"},{"line_number":134,"context_line":"          try:"},{"line_number":135,"context_line":"              _do_other_thing()"},{"line_number":136,"context_line":"          except Exception:"},{"line_number":137,"context_line":"              with excutils.save_and_reraise_exception():"},{"line_number":138,"context_line":"                  delete_something()"}],"source_content_type":"text/x-rst","patch_set":10,"id":"ba8a016a_9d49f6d2","line":135,"updated":"2015-11-24 02:45:10.000000000","message":"I don\u0027t think the db exception in \u0027_do_other_thing()\u0027 will be raised  here if it is in nested transaction. right?","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bfb99bdbc7283e0d3bbc3e80782739dd84cc15d0","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":133,"context_line":"          create_something()"},{"line_number":134,"context_line":"          try:"},{"line_number":135,"context_line":"              _do_other_thing()"},{"line_number":136,"context_line":"          except Exception:"},{"line_number":137,"context_line":"              with excutils.save_and_reraise_exception():"},{"line_number":138,"context_line":"                  delete_something()"}],"source_content_type":"text/x-rst","patch_set":10,"id":"ba8a016a_20a8da8b","line":135,"in_reply_to":"ba8a016a_9d49f6d2","updated":"2015-11-24 08:18:51.000000000","message":"Why it should not? Please, check up tests test_create_ha_network_binding_failure_rolls_back_network, test_add_ha_port_binding_failure_rolls_back_port","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"834ecb70083ae7d59756f81047fff3fda4ecf86b","unresolved":false,"context_lines":[{"line_number":105,"context_line":"  rollback. Consider if `savepoints \u003chttp://docs.sqlalchemy.org/en/rel_1_0/orm/session_transaction.html#using-savepoint\u003e`_"},{"line_number":106,"context_line":"  can fit your use case."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"* Sometimes in code we use the following structures:"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"  .. code:: python"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"7a740942_6b9c446a","line":108,"updated":"2015-12-13 11:42:40.000000000","message":"structures \u003d\u003e flows","commit_id":"3eaeb1e5e407e076e951633a77b68baaf8b46bab"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"0f12281c7e999959e0868067c3e1a04739a1ac8f","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":122,"context_line":"            ...."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"  The problem is that when exception is raised in ``_do_other_thing`` it is caught"},{"line_number":125,"context_line":"  in except block, but the object cannot be deleted in except section because"},{"line_number":126,"context_line":"  internal transaction has been rolled back. To avoid this nested transactions"},{"line_number":127,"context_line":"  should be used. For such cases help function ``_safe_creation`` has been created"}],"source_content_type":"text/x-rst","patch_set":16,"id":"5a710552_bd2534c3","line":124,"range":{"start_line":124,"start_character":6,"end_line":124,"end_character":13},"updated":"2015-12-15 04:41:31.000000000","message":"this is a problem only when the entire create() is surrounded by a transaction?","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"194d2acee44f0b906e7dc3c7d325353290780da6","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":122,"context_line":"            ...."},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"  The problem is that when exception is raised in ``_do_other_thing`` it is caught"},{"line_number":125,"context_line":"  in except block, but the object cannot be deleted in except section because"},{"line_number":126,"context_line":"  internal transaction has been rolled back. To avoid this nested transactions"},{"line_number":127,"context_line":"  should be used. For such cases help function ``_safe_creation`` has been created"}],"source_content_type":"text/x-rst","patch_set":16,"id":"5a710552_9e482589","line":124,"range":{"start_line":124,"start_character":6,"end_line":124,"end_character":13},"in_reply_to":"5a710552_bd2534c3","updated":"2015-12-16 11:35:59.000000000","message":"yes, I\u0027ll update description.","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"0f12281c7e999959e0868067c3e1a04739a1ac8f","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"  The problem is that when exception is raised in ``_do_other_thing`` it is caught"},{"line_number":125,"context_line":"  in except block, but the object cannot be deleted in except section because"},{"line_number":126,"context_line":"  internal transaction has been rolled back. To avoid this nested transactions"},{"line_number":127,"context_line":"  should be used. For such cases help function ``_safe_creation`` has been created"},{"line_number":128,"context_line":"  in neutron/db/common_db_mixin.py So, the example above should be replaced with:"},{"line_number":129,"context_line":""}],"source_content_type":"text/x-rst","patch_set":16,"id":"5a710552_7d810c8f","line":126,"range":{"start_line":126,"start_character":2,"end_line":126,"end_character":22},"updated":"2015-12-15 04:41:31.000000000","message":"what\u0027s \"internal transaction\"?","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"194d2acee44f0b906e7dc3c7d325353290780da6","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"  The problem is that when exception is raised in ``_do_other_thing`` it is caught"},{"line_number":125,"context_line":"  in except block, but the object cannot be deleted in except section because"},{"line_number":126,"context_line":"  internal transaction has been rolled back. To avoid this nested transactions"},{"line_number":127,"context_line":"  should be used. For such cases help function ``_safe_creation`` has been created"},{"line_number":128,"context_line":"  in neutron/db/common_db_mixin.py So, the example above should be replaced with:"},{"line_number":129,"context_line":""}],"source_content_type":"text/x-rst","patch_set":16,"id":"5a710552_9e7105e3","line":126,"range":{"start_line":126,"start_character":2,"end_line":126,"end_character":22},"in_reply_to":"5a710552_7d810c8f","updated":"2015-12-16 11:35:59.000000000","message":"transaction that begins in _do_other_thing()","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"56f505190c60512eb173e10ba8d59a0bf10c1804","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":141,"context_line":"            create_something()"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                _do_other_thing()"},{"line_number":144,"context_line":"            except Exception:"},{"line_number":145,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":146,"context_line":"                    delete_something()"}],"source_content_type":"text/x-rst","patch_set":24,"id":"da6ed579_8b36ed01","line":143,"updated":"2016-01-12 13:17:08.000000000","message":"In the code this is more specific (create_bindings).","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"390550fa21c4247031ca63825929287b75a3f432","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":141,"context_line":"            create_something()"},{"line_number":142,"context_line":"            try:"},{"line_number":143,"context_line":"                _do_other_thing()"},{"line_number":144,"context_line":"            except Exception:"},{"line_number":145,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":146,"context_line":"                    delete_something()"}],"source_content_type":"text/x-rst","patch_set":24,"id":"da6ed579_b3276cb5","line":143,"in_reply_to":"da6ed579_8b36ed01","updated":"2016-01-13 12:40:05.000000000","message":"I will rename in _do_other_thing_with_created_object to make it more common.","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"}],"neutron/db/common_db_mixin.py":[{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"f3692e5e44c68d57dada51dc06ea25ba0c61e412","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return query"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _safe_creation(context, creation, deletion, content):"},{"line_number":49,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":50,"context_line":"        if creation is not None:"},{"line_number":51,"context_line":"            object \u003d creation()"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_238d5a7b","line":48,"updated":"2015-12-16 08:33:14.000000000","message":"funciton names starting with underscore indicate local usage.\n\nAlso please add doc string about what is expected.\nAFAIU, we expect functions here without parameters, prepaired with functools. An example would be useful.\n\nAlso, I\u0027d name parameters \u0027create_fn\u0027, \u0027delete_fn\u0027\nWhat is \u0027content\u0027? The name is confusing. Even looking at the code I can\u0027t figure out what it does.","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"194d2acee44f0b906e7dc3c7d325353290780da6","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return query"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _safe_creation(context, creation, deletion, content):"},{"line_number":49,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":50,"context_line":"        if creation is not None:"},{"line_number":51,"context_line":"            object \u003d creation()"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_32d845ef","line":48,"in_reply_to":"5a710552_238d5a7b","updated":"2015-12-16 11:35:59.000000000","message":"Done","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"f3692e5e44c68d57dada51dc06ea25ba0c61e412","unresolved":false,"context_lines":[{"line_number":48,"context_line":"def _safe_creation(context, creation, deletion, content):"},{"line_number":49,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":50,"context_line":"        if creation is not None:"},{"line_number":51,"context_line":"            object \u003d creation()"},{"line_number":52,"context_line":"            try:"},{"line_number":53,"context_line":"                value \u003d content(object[\u0027id\u0027])"},{"line_number":54,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_23bb1a10","line":51,"updated":"2015-12-16 08:33:14.000000000","message":"why isn\u0027t it wrapped with try-except?","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"194d2acee44f0b906e7dc3c7d325353290780da6","unresolved":false,"context_lines":[{"line_number":48,"context_line":"def _safe_creation(context, creation, deletion, content):"},{"line_number":49,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":50,"context_line":"        if creation is not None:"},{"line_number":51,"context_line":"            object \u003d creation()"},{"line_number":52,"context_line":"            try:"},{"line_number":53,"context_line":"                value \u003d content(object[\u0027id\u0027])"},{"line_number":54,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_b20995df","line":51,"in_reply_to":"5a710552_23bb1a10","updated":"2015-12-16 11:35:59.000000000","message":"Why do we need it? If creation failed, there is no point to do something else here.","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"f3692e5e44c68d57dada51dc06ea25ba0c61e412","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            return (object, value)"},{"line_number":58,"context_line":"        else:"},{"line_number":59,"context_line":"            try:"},{"line_number":60,"context_line":"                content()"},{"line_number":61,"context_line":"            except Exception:"},{"line_number":62,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":63,"context_line":"                    deletion()"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_636fe292","line":60,"updated":"2015-12-16 08:33:14.000000000","message":"Are we expecting some other king of \u0027content\u0027 function here?","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"194d2acee44f0b906e7dc3c7d325353290780da6","unresolved":false,"context_lines":[{"line_number":57,"context_line":"            return (object, value)"},{"line_number":58,"context_line":"        else:"},{"line_number":59,"context_line":"            try:"},{"line_number":60,"context_line":"                content()"},{"line_number":61,"context_line":"            except Exception:"},{"line_number":62,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":63,"context_line":"                    deletion()"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_72100da9","line":60,"in_reply_to":"5a710552_636fe292","updated":"2015-12-16 11:35:59.000000000","message":"This case for situation when we are applying some changes without returning any results.","commit_id":"e3984a8cfca15e1d28c4fa24197427de251eec3e"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8756a5700e0ed41b32551eabad05c1d0acba8d91","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return query"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":49,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":50,"context_line":"    atomic way. In case of exception, object is deleted. This function can"},{"line_number":51,"context_line":"    also be used just to wrap some object manipulation after it has been"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a7e1126_cbcb551d","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":18},"updated":"2015-12-18 12:36:10.000000000","message":"in this form the function doesn\u0027t look generic to be placed into common_db_mixin.\nI think in that form it\u0027s better to move it to l3_hamode_db.py\nAnother issue is that it expects different kind of methods depending on one of the parameters. Can this be avoided?","commit_id":"6df756a83d6a63e516edda0e44b6b60d285ea013"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"7c67dfb017d376b3be4cfca90c30dd10441cd1d7","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return query"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":49,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":50,"context_line":"    atomic way. In case of exception, object is deleted. This function can"},{"line_number":51,"context_line":"    also be used just to wrap some object manipulation after it has been"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a7e1126_ca98c41e","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":18},"in_reply_to":"3a7e1126_89d0b520","updated":"2015-12-21 13:37:04.000000000","message":"The method is really specific to l3_ha, it can\u0027t be reused.\nIf Salvatore could suggest really generic look of it, I\u0027d agree, otherwise i think it makes sense to put it back.","commit_id":"6df756a83d6a63e516edda0e44b6b60d285ea013"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3c5e90bf288b583bd36e1ffdc896b4e5673bcf26","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return query"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":49,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":50,"context_line":"    atomic way. In case of exception, object is deleted. This function can"},{"line_number":51,"context_line":"    also be used just to wrap some object manipulation after it has been"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a7e1126_89d0b520","line":48,"range":{"start_line":48,"start_character":4,"end_line":48,"end_character":18},"in_reply_to":"3a7e1126_cbcb551d","updated":"2015-12-19 07:57:41.000000000","message":"On the 13th patch set was already suggested to move it here. \n\nTo avoid dependence of the parameter this method can only be split.","commit_id":"6df756a83d6a63e516edda0e44b6b60d285ea013"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8756a5700e0ed41b32551eabad05c1d0acba8d91","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":70,"context_line":"        if create_fn is not None:"},{"line_number":71,"context_line":"            object \u003d create_fn()"},{"line_number":72,"context_line":"            try:"},{"line_number":73,"context_line":"                value \u003d create_bindings(object[\u0027id\u0027])"},{"line_number":74,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a7e1126_dcfbfdbf","line":71,"range":{"start_line":71,"start_character":12,"end_line":71,"end_character":18},"updated":"2015-12-18 12:36:10.000000000","message":"object is a base calss, let\u0027s use different name here","commit_id":"6df756a83d6a63e516edda0e44b6b60d285ea013"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3c5e90bf288b583bd36e1ffdc896b4e5673bcf26","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":70,"context_line":"        if create_fn is not None:"},{"line_number":71,"context_line":"            object \u003d create_fn()"},{"line_number":72,"context_line":"            try:"},{"line_number":73,"context_line":"                value \u003d create_bindings(object[\u0027id\u0027])"},{"line_number":74,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3a7e1126_a9cd7186","line":71,"range":{"start_line":71,"start_character":12,"end_line":71,"end_character":18},"in_reply_to":"3a7e1126_dcfbfdbf","updated":"2015-12-19 07:57:41.000000000","message":"Done","commit_id":"6df756a83d6a63e516edda0e44b6b60d285ea013"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"a4161fec8fbb3fc3e02682b686cc781d7cb92836","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":68,"context_line":"        except Exception:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                delete_fn(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        return (obj, value)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_74e2fc30","line":70,"updated":"2015-12-19 09:23:22.000000000","message":"I think in both of these cases you will want to catch exceptions from the delete call and just log them as exceptions. We want the original creation exception to be the one that gets delivered back to the caller, not some erorr that might be encountered during the attempted rollback.","commit_id":"301283ff00bd74c5428d236dcb94147ddc31c00b"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"c9bec58d45fe74aaf0897e4d6557bb0fd6243051","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":68,"context_line":"        except Exception:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                delete_fn(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        return (obj, value)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"1a7b0d38_db501661","line":70,"in_reply_to":"1a7b0d38_80bf7ffa","updated":"2015-12-24 10:07:50.000000000","message":"Done","commit_id":"301283ff00bd74c5428d236dcb94147ddc31c00b"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3547bc0b5980408bca901d69f9f36ef1972be964","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":68,"context_line":"        except Exception:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                delete_fn(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        return (obj, value)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3a7e1126_8f0e7539","line":70,"in_reply_to":"3a7e1126_74e2fc30","updated":"2015-12-19 13:41:01.000000000","message":"This is already done by save_and_reraise_exception: it saves original exception, logs exception from with block if it happens and then reraises original exception.\n\nhttp://docs.openstack.org/developer/oslo.utils/api/excutils.html#oslo_utils.excutils.save_and_reraise_exception","commit_id":"301283ff00bd74c5428d236dcb94147ddc31c00b"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"541f8fa011a141749a60b09c90a0055975a7c3d1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":68,"context_line":"        except Exception:"},{"line_number":69,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":70,"context_line":"                delete_fn(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        return (obj, value)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"1a7b0d38_80bf7ffa","line":70,"in_reply_to":"3a7e1126_8f0e7539","updated":"2015-12-24 08:47:12.000000000","message":"I mean if delete_fn raises an exception, that will replace the exception that will be raised by that context manager. This is the important sentence from that doc:\n\n\"If another exception occurs, the saved exception is logged and the new exception is re-raised.\"\n\n\nI\u0027m suggesting that we don\u0027t want the delete_fn exception to be the one that is raised and delivered back to the caller because that isn\u0027t what caused the initial failure. We just want to log the delete_fn failure and return the original exception back to the user that resulted in delete_fn being called.","commit_id":"301283ff00bd74c5428d236dcb94147ddc31c00b"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    return query"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":52,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":53,"context_line":"    atomic way. In case of exception, object is deleted."},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_09bad323","line":51,"range":{"start_line":51,"start_character":18,"end_line":51,"end_character":25},"updated":"2015-12-24 13:15:47.000000000","message":"This method only needs a session, not a context\n\nThis method is not tested\n\nThis method is a bit too specific to be defined in this module?","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    return query"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":52,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":53,"context_line":"    atomic way. In case of exception, object is deleted."},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_71cbe353","line":51,"range":{"start_line":51,"start_character":18,"end_line":51,"end_character":25},"in_reply_to":"1a7b0d38_09bad323","updated":"2016-01-11 14:50:58.000000000","message":"I prefer to leave context here for others -  done.","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":52,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":53,"context_line":"    atomic way. In case of exception, object is deleted."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    :param context: context"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_4969bbbd","line":52,"range":{"start_line":52,"start_character":43,"end_line":52,"end_character":51},"updated":"2015-12-24 13:15:47.000000000","message":"docstring title should be on one line","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def safe_creation(context, create_fn, delete_fn, create_bindings):"},{"line_number":52,"context_line":"    \u0027\u0027\u0027This function wraps logic of object creation with some bindings in safe"},{"line_number":53,"context_line":"    atomic way. In case of exception, object is deleted."},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    :param context: context"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_91c8d756","line":52,"range":{"start_line":52,"start_character":43,"end_line":52,"end_character":51},"in_reply_to":"1a7b0d38_4969bbbd","updated":"2016-01-11 14:50:58.000000000","message":"Done","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    :param delete_fn: function that is called to delete an object. It is called"},{"line_number":61,"context_line":"        with object\u0027s id field as an argument."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    :param create_bindings: function that is called to create binding for an"},{"line_number":64,"context_line":"        object. It is called with object\u0027s id field as an argument."},{"line_number":65,"context_line":"    \u0027\u0027\u0027"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_4469a2bd","line":63,"range":{"start_line":63,"start_character":62,"end_line":63,"end_character":69},"updated":"2015-12-24 13:15:47.000000000","message":"bindings","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    :param delete_fn: function that is called to delete an object. It is called"},{"line_number":61,"context_line":"        with object\u0027s id field as an argument."},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    :param create_bindings: function that is called to create binding for an"},{"line_number":64,"context_line":"        object. It is called with object\u0027s id field as an argument."},{"line_number":65,"context_line":"    \u0027\u0027\u0027"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_f17913fd","line":63,"range":{"start_line":63,"start_character":62,"end_line":63,"end_character":69},"in_reply_to":"1a7b0d38_4469a2bd","updated":"2016-01-11 14:50:58.000000000","message":"Done","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        object. It is called with object\u0027s id field as an argument."},{"line_number":65,"context_line":"    \u0027\u0027\u0027"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":68,"context_line":"        obj \u003d create_fn()"},{"line_number":69,"context_line":"        try:"},{"line_number":70,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_89b6c388","line":67,"range":{"start_line":67,"start_character":9,"end_line":67,"end_character":16},"updated":"2015-12-24 13:15:47.000000000","message":"IMO, it\u0027s really strange to define a subtransaction on session which is used implicitly by create/delete_fn, create_bindings. I would prefer:\n\n\n with session.begin(subtransactions\u003dTrue):\n   obj \u003d create_fn(session)\n   try:\n     with session.begin(nested\u003dTrue):\n       value \u003d create_bindings(session, obj[\u0027id\u0027])\n   except:\n     ...","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":68,"context_line":"        obj \u003d create_fn()"},{"line_number":69,"context_line":"        try:"},{"line_number":70,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        except Exception:"},{"line_number":72,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":73,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_e915079b","line":70,"range":{"start_line":70,"start_character":12,"end_line":70,"end_character":17},"updated":"2015-12-24 13:15:47.000000000","message":"IMO, this method should embed the create_bindings into a nested transaction to avoid breaking the session instead of doing it in create_bindings (as it\u0027s done in this patchset):\n\n with context.session.begin(nested\u003dTrue):\n  value \u003d create_bindings(...)","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":67,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":68,"context_line":"        obj \u003d create_fn()"},{"line_number":69,"context_line":"        try:"},{"line_number":70,"context_line":"            value \u003d create_bindings(obj[\u0027id\u0027])"},{"line_number":71,"context_line":"        except Exception:"},{"line_number":72,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":73,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_d1364f50","line":70,"range":{"start_line":70,"start_character":12,"end_line":70,"end_character":17},"in_reply_to":"1a7b0d38_e915079b","updated":"2016-01-11 14:50:58.000000000","message":"This is not working.","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                except Exception:"},{"line_number":76,"context_line":"                    LOG.error(_LE(\"Cannot clean up created object %s\"),"},{"line_number":77,"context_line":"                              obj[\u0027id\u0027])"},{"line_number":78,"context_line":"        return (obj, value)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def safe_bindings_creation(context, create_bindings, delete_fn):"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_e9d3c75d","line":78,"range":{"start_line":78,"start_character":21,"end_line":78,"end_character":26},"updated":"2015-12-24 13:15:47.000000000","message":"useless braces","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                except Exception:"},{"line_number":76,"context_line":"                    LOG.error(_LE(\"Cannot clean up created object %s\"),"},{"line_number":77,"context_line":"                              obj[\u0027id\u0027])"},{"line_number":78,"context_line":"        return (obj, value)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def safe_bindings_creation(context, create_bindings, delete_fn):"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_3146cbc0","line":78,"range":{"start_line":78,"start_character":21,"end_line":78,"end_character":26},"in_reply_to":"1a7b0d38_e9d3c75d","updated":"2016-01-11 14:50:58.000000000","message":"Done","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        return (obj, value)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"def safe_bindings_creation(context, create_bindings, delete_fn):"},{"line_number":82,"context_line":"    \u0027\u0027\u0027This function wraps logic of some object manipulation after it has been"},{"line_number":83,"context_line":"    created externally."},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_a97fff64","line":81,"range":{"start_line":81,"start_character":4,"end_line":81,"end_character":26},"updated":"2015-12-24 13:15:47.000000000","message":"itto","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    :param context: context"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    :param create_bindings: function that is called to create binding for an"},{"line_number":88,"context_line":"        object. It is called without arguments."},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    :param delete_fn: function that is called to delete an object. It is"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_044cfa6e","line":87,"range":{"start_line":87,"start_character":62,"end_line":87,"end_character":69},"updated":"2015-12-24 13:15:47.000000000","message":"bindings","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7bee744128f0099412941218413850b981b31735","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                try:"},{"line_number":61,"context_line":"                    delete_fn(obj[\u0027id\u0027])"},{"line_number":62,"context_line":"                except Exception:"},{"line_number":63,"context_line":"                    LOG.error(_LE(\"Cannot clean up created object %s\"),"},{"line_number":64,"context_line":"                              obj[\u0027id\u0027])"},{"line_number":65,"context_line":"        return obj, value"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"da6ed579_c205647a","line":63,"updated":"2016-01-13 14:12:13.000000000","message":"do we want to at least pass the exception to the log message?","commit_id":"ecbd91c5390d6697d7fe7900462bb8d80357e978"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"f8e17d4517b1bb1ee129cfb9d05fc2acf1a77e8a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                try:"},{"line_number":61,"context_line":"                    delete_fn(obj[\u0027id\u0027])"},{"line_number":62,"context_line":"                except Exception:"},{"line_number":63,"context_line":"                    LOG.error(_LE(\"Cannot clean up created object %s\"),"},{"line_number":64,"context_line":"                              obj[\u0027id\u0027])"},{"line_number":65,"context_line":"        return obj, value"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"da6ed579_b28da2d4","line":63,"in_reply_to":"da6ed579_c205647a","updated":"2016-01-15 12:34:39.000000000","message":"Done","commit_id":"ecbd91c5390d6697d7fe7900462bb8d80357e978"}],"neutron/db/l3_hamode_db.py":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"347d4677ed01f1d845d40c3564df5e896ffae6e4","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":265,"context_line":"        except Exception:"},{"line_number":266,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":267,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_e26b68b5","side":"PARENT","line":267,"updated":"2015-10-09 12:09:12.000000000","message":"while db operations would be happily rolled back,\nplugin create_network/delete_network might not be\na pure db operations.","commit_id":"7aed40f89154e9f21e20c249e5ae102f7ade38db"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"b509db78dacc456a9e8abaa631bf04646c8c3ef7","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":265,"context_line":"        except Exception:"},{"line_number":266,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":267,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a29b11f_82949f37","side":"PARENT","line":267,"in_reply_to":"3a29b11f_ccc89377","updated":"2015-10-19 12:57:17.000000000","message":"I\u0027ll try to save try-except logic.","commit_id":"7aed40f89154e9f21e20c249e5ae102f7ade38db"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"08aca0eab153485b5bd2db9041144564ca590035","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":265,"context_line":"        except Exception:"},{"line_number":266,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":267,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a29b11f_ccc89377","side":"PARENT","line":267,"in_reply_to":"7a2fa921_d12958e7","updated":"2015-10-19 11:24:44.000000000","message":"how about:\n\n  create_network\n  with begin(nested\u003dTrue):\n    ...\n  if error\n    delete_network","commit_id":"7aed40f89154e9f21e20c249e5ae102f7ade38db"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5a6d8d65958ab730c10830c1a2127e09ae1c2f97","unresolved":false,"context_lines":[{"line_number":264,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":265,"context_line":"        except Exception:"},{"line_number":266,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":267,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        try:"},{"line_number":270,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_d12958e7","side":"PARENT","line":267,"in_reply_to":"7a2fa921_e26b68b5","updated":"2015-10-09 13:45:47.000000000","message":"I agree, but I don\u0027t know how to handle this in other way. Can you suggest something?","commit_id":"7aed40f89154e9f21e20c249e5ae102f7ade38db"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"be46b82c99119a124317874236cc9533a46c6c0f","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":235,"context_line":"                                          network_id):"},{"line_number":236,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":237,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":238,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":239,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_5fbf9960","line":236,"updated":"2015-10-09 12:06:26.000000000","message":"is this savepoint still necessary?","commit_id":"0a4ab95e4846ba0550c03b4c29d57ea14f1a93df"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"08aca0eab153485b5bd2db9041144564ca590035","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":235,"context_line":"                                          network_id):"},{"line_number":236,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":237,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":238,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":239,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a29b11f_0c4c4b41","line":236,"in_reply_to":"7a2fa921_561bfe80","updated":"2015-10-19 11:24:44.000000000","message":"well, i don\u0027t understand how it\u0027s useful after you removed try-except and cleanup code from the caller.","commit_id":"0a4ab95e4846ba0550c03b4c29d57ea14f1a93df"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5a6d8d65958ab730c10830c1a2127e09ae1c2f97","unresolved":false,"context_lines":[{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":235,"context_line":"                                          network_id):"},{"line_number":236,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":237,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":238,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":239,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a2fa921_561bfe80","line":236,"in_reply_to":"7a2fa921_5fbf9960","updated":"2015-10-09 13:45:47.000000000","message":"The main idea is to use it here, see Mike Bayer\u0027s comment for 2nd patch set.","commit_id":"0a4ab95e4846ba0550c03b4c29d57ea14f1a93df"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"9a063ead858878bb2f9e5d176e6b12e5a4028340","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                \u0027device_id\u0027: router_id,"},{"line_number":319,"context_line":"                \u0027device_owner\u0027: constants.DEVICE_OWNER_ROUTER_HA_INTF,"},{"line_number":320,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":321,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":322,"context_line":"            port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":323,"context_line":"                                     {\u0027port\u0027: args})"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_787bb07b","line":321,"updated":"2015-11-12 01:38:44.000000000","message":"shouldn\u0027t this be nested\u003dTrue when called by L342?\n\nprobably you can use autonested_transaction.","commit_id":"2ea39515524bbbf41036156152e1a935b907ad94"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"eb95f600436322aebabe83e34d0bac82b912fe9b","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                \u0027device_id\u0027: router_id,"},{"line_number":319,"context_line":"                \u0027device_owner\u0027: constants.DEVICE_OWNER_ROUTER_HA_INTF,"},{"line_number":320,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":321,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":322,"context_line":"            port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":323,"context_line":"                                     {\u0027port\u0027: args})"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da85f559_a67fa9de","line":321,"in_reply_to":"da85f559_787bb07b","updated":"2015-11-12 09:29:12.000000000","message":"add_ha_port is also called in create_ha_port_and_bind [1] without transaction at this moment. In fact in my other change [2] I add transaction there, but I don\u0027t want to rebase on it, as I already have some twisted relations between my patches. I hope [2] will be merged soon, so I will update this change, thanks for pointing on this!\n\n[1] - https://github.com/openstack/neutron/blob/master/neutron/scheduler/l3_agent_scheduler.py#L295\n[2] - https://review.openstack.org/#/c/227821/","commit_id":"2ea39515524bbbf41036156152e1a935b907ad94"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a9c36a270910c27b8ccf4c76b46677e870e45fc5","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return allocated_vr_ids"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _allocate_vr_id(self, context, network_id, router_id):"},{"line_number":185,"context_line":"        for count in range(MAX_ALLOCATION_TRIES):"},{"line_number":186,"context_line":"            try:"},{"line_number":187,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_15272713","line":184,"updated":"2015-11-23 23:16:10.000000000","message":"Unrelated to this patch: I\u0027m not sure this code ever worked. If I remember correctly the default transaction level in MySQL is repeatable read, which means each loop iteration will see the same data. We may need to return a random value from the range. Ann, what do you think?","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bfb99bdbc7283e0d3bbc3e80782739dd84cc15d0","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return allocated_vr_ids"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _allocate_vr_id(self, context, network_id, router_id):"},{"line_number":185,"context_line":"        for count in range(MAX_ALLOCATION_TRIES):"},{"line_number":186,"context_line":"            try:"},{"line_number":187,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_8318b0d4","line":184,"in_reply_to":"ba8a016a_15272713","updated":"2015-11-24 08:18:51.000000000","message":"I will take a look at this after a while.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"e1039a964c23a9e14650a5cee4aa4858684d047c","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_5771f742","line":390,"updated":"2015-11-19 00:48:32.000000000","message":"This change is substantial and is essentially hidden from the commit message, it puts the creation of the base router and its HA resources in a transaction (Which is  something I think we should do anyway). As discussed in previous patchsets (Either on this patch or another one, I forget), the reason we didn\u0027t do this originally (Although, from the complexity I think it\u0027s clear we do want to this now) is that with this entire block in a transaction, _create_ha_interfaces calls the core_plugin.create_port and in the case of ML2 (And probably most other plugins) we now use the RPC bus and/or REST calls in a transaction. This should be mentioned in the commit message. \n\nThe other question is: If this is all in a transaction, I don\u0027t think we actually need the block from lines 405 to 407 since the router would be rolled back if lines 396 to 404 were to fail anyway. This is probably relevant to other code bits as well.\n\nEssentially this patch as it is, is a mix between two approaches: Maintain the current approach and solve some of the races you\u0027ve seen one by one + Correct the usage of nested transactions, and the second approach is just forget about all of this and work with transactions properly and forgo the paradigm of:\n\nsomething \u003d create_something\ntry:\n  do stuff\nexcept:\n  delete something\n\nI really think it\u0027s time to go for the second approach. In which case there\u0027s no point to correcting the usage of nested transactions. I know we discussed this before, what do you think?","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"2e279a8df0603e1e9cc21a9310ff9297494a3ae6","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_1fb0a842","line":390,"in_reply_to":"ba8a016a_189be973","updated":"2015-11-23 20:00:37.000000000","message":"@Kevin, I know :) Transactions are also opaque to the L3 agent. I think (?) that simply rolling back a router wouldn\u0027t notify the agent(s) hosting it. I don\u0027t think we use this paradigm (try/except/manual rollback) in other places in the code so I thought maybe we\u0027d get away with it here. Instead I think we should do the right thing, starting here.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2a452a8f4191c33fbd7faf5b4f5c00b73c774c42","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9a8ffd7b_8d2034f9","line":390,"in_reply_to":"ba8a016a_1fb0a842","updated":"2015-11-25 00:05:42.000000000","message":"We do in ml2. E.g. If a port create fails in a post commit method of an ml2 driver, we catch the error and then delete the thing that was created.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1d4daf29cf2f46a46a9fbad206c4accd54a7130b","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_189be973","line":390,"in_reply_to":"ba8a016a_482dd1fc","updated":"2015-11-23 17:36:47.000000000","message":"@Assaf, transactions are opaque to backends (e.g. OVN, ODL, ToR drivers, etc). We can\u0027t rely on transactions to rollback multiple core plugin operations.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cada65d1c815f666f2bf90f112f2c931f60d05a3","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_d27e5569","line":390,"in_reply_to":"ba8a016a_5771f742","updated":"2015-11-19 01:06:48.000000000","message":"One more item in support of my argument, check out this trace:\nhttp://logs.openstack.org/75/245975/2/check/gate-neutron-dsvm-fullstack/43dc930/logs/TestHAL3Agent.test_ha_router/neutron-server--2015-11-16--22-05-23-523605.log.txt.gz#_2015-11-16_22_05_36_127\n\nThis is after your two patches were merged (But obviously before this one and the next one). I think it\u0027s again the race between a L3 agent starting up and requesting a sync before router creation actually finishes (After the base router object is committed but before the HA resources are). I don\u0027t think we should try to fix these individual races, just use transactions properly when we create resources and be done with it. Even if we start seeing deadlocks due to L3 HA code creating ports in transactions, I think we\u0027re better off solving those.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"67821c60dfc621a81137f096182f04b99c233af7","unresolved":false,"context_lines":[{"line_number":387,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_482dd1fc","line":390,"in_reply_to":"ba8a016a_d27e5569","updated":"2015-11-19 07:29:22.000000000","message":"In the 4th patch I use approach with removing try/except logic and just having transactions in proper places. But YAMAMOTO Takashi pointed me that not for all plugins create/delete operations are pure db operations, so having just a transaction is not safe. That is why I saved try/except here and in other places.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a9c36a270910c27b8ccf4c76b46677e870e45fc5","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            if is_ha:"},{"line_number":395,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_36b032dd","line":392,"updated":"2015-11-23 23:16:10.000000000","message":"It might be preferable to just \u0027if not is_ha\u0027 and return router_dict here. It\u0027ll save the second \u0027is_ha\u0027 in line 408.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bfb99bdbc7283e0d3bbc3e80782739dd84cc15d0","unresolved":false,"context_lines":[{"line_number":389,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":390,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":391,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":392,"context_line":"                                self).create_router(context, router)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"            if is_ha:"},{"line_number":395,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_43008866","line":392,"in_reply_to":"ba8a016a_36b032dd","updated":"2015-11-24 08:18:51.000000000","message":"Done","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a9c36a270910c27b8ccf4c76b46677e870e45fc5","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                    ha_network \u003d self.get_ha_network(context,"},{"line_number":398,"context_line":"                                                     router_db.tenant_id)"},{"line_number":399,"context_line":"                    if not ha_network:"},{"line_number":400,"context_line":"                        ha_network \u003d self._create_ha_network("},{"line_number":401,"context_line":"                            context, router_db.tenant_id)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    self._set_vr_id(context, router_db, ha_network)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_96923e4a","line":400,"updated":"2015-11-23 23:16:10.000000000","message":"When I read the code (And the changes to nested\u003dTrue) I was thinking how complicated the code was, and how we need a helper for this paradigm. Would something like this work?\n\ndef I_don\u0027t_have_a_name_for_this(context, creation, deletion, content):\n  begin(subtransactions\u003dTrue)\n    creation()\n    try:\n      begin(nested\u003dTrue)\n        content()\n    except Exception:\n      with exeutils...\n        deletion()\n\nYou could use it with functools.partial for creation, deletion and content and their arguments. You would use it in every instance you want to use the plugin layer to clean up a resource, meaning here and in _create_ha_network, add_ha_port, and in _create_ha_interface. The docstring would point to the effective guide part you added. Without the helper, I think we\u0027re asking for too much of our future selves, you need to be aware of too much stuff in order to not screw up and I don\u0027t think it\u0027s realistic in this case. If you have other ideas how to hide some of this complexity I\u0027m all ears.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"e3d3e7dd0719307c2bcbf2c16e4643d311b0e196","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                    ha_network \u003d self.get_ha_network(context,"},{"line_number":398,"context_line":"                                                     router_db.tenant_id)"},{"line_number":399,"context_line":"                    if not ha_network:"},{"line_number":400,"context_line":"                        ha_network \u003d self._create_ha_network("},{"line_number":401,"context_line":"                            context, router_db.tenant_id)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    self._set_vr_id(context, router_db, ha_network)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a8ffd7b_b0c0d67d","line":400,"in_reply_to":"9a8ffd7b_811d7cbf","updated":"2015-11-25 11:53:42.000000000","message":"_create_ha_interfaces does not have creation() and this makes helper function a bit ugly. I will push patch set with refactor. \n\ncreate_router has subtransactions\u003dTrue, not nested\u003dTrue like others, so at this moment I won\u0027t refactor it. Please, add some comments, if I missing something.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4749f8b1157d1cb2b597ac0cd965374b0750d3bd","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                    ha_network \u003d self.get_ha_network(context,"},{"line_number":398,"context_line":"                                                     router_db.tenant_id)"},{"line_number":399,"context_line":"                    if not ha_network:"},{"line_number":400,"context_line":"                        ha_network \u003d self._create_ha_network("},{"line_number":401,"context_line":"                            context, router_db.tenant_id)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    self._set_vr_id(context, router_db, ha_network)"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a8ffd7b_811d7cbf","line":400,"in_reply_to":"ba8a016a_63a26cb0","updated":"2015-11-24 15:21:18.000000000","message":"Can you explain why it or something similar (Context manager perhaps) could not be used with _create_ha_interfaces and create_router? Do you have other thoughts of how we could abstract away this complexity? I think the code after this change is not maintainable, it\u0027s just too nuanced.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"25a88965a92786084a8b3216f1462909ed2396ae","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                    ha_network \u003d self.get_ha_network(context,"},{"line_number":398,"context_line":"                                                     router_db.tenant_id)"},{"line_number":399,"context_line":"                    if not ha_network:"},{"line_number":400,"context_line":"                        ha_network \u003d self._create_ha_network("},{"line_number":401,"context_line":"                            context, router_db.tenant_id)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    self._set_vr_id(context, router_db, ha_network)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_ffc4bd92","line":400,"in_reply_to":"ba8a016a_96923e4a","updated":"2015-11-23 23:32:50.000000000","message":"By the way the pseudo-code above I added is a bit different than what you have here. I suggested creating a savepoint before \u0027content\u0027, I believe it can simplify things a bit.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bfb99bdbc7283e0d3bbc3e80782739dd84cc15d0","unresolved":false,"context_lines":[{"line_number":397,"context_line":"                    ha_network \u003d self.get_ha_network(context,"},{"line_number":398,"context_line":"                                                     router_db.tenant_id)"},{"line_number":399,"context_line":"                    if not ha_network:"},{"line_number":400,"context_line":"                        ha_network \u003d self._create_ha_network("},{"line_number":401,"context_line":"                            context, router_db.tenant_id)"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"                    self._set_vr_id(context, router_db, ha_network)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_63a26cb0","line":400,"in_reply_to":"ba8a016a_ffc4bd92","updated":"2015-11-24 08:18:51.000000000","message":"I realize your concern about simplicity. But such helper function can be used only for add_ha_port, _create_ha_network. In other places _create_ha_interfaces, create_router I can\u0027t use it.I\u0027m not sure that it will make things much better :)","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a9c36a270910c27b8ccf4c76b46677e870e45fc5","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        self._unbind_ha_router(context, router_id)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        if requested_ha_state:"},{"line_number":461,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":462,"context_line":"                if not ha_network:"},{"line_number":463,"context_line":"                    ha_network \u003d self._create_ha_network(context,"},{"line_number":464,"context_line":"                                                         router_db.tenant_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_1cf42f85","line":461,"updated":"2015-11-23 23:16:10.000000000","message":"I think taking another look at the racefulness of the update method is in order, I\u0027m just not sure if this patch is the place. Maybe we wanna tackle create and update in separate patches?","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bfb99bdbc7283e0d3bbc3e80782739dd84cc15d0","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        self._unbind_ha_router(context, router_id)"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        if requested_ha_state:"},{"line_number":461,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":462,"context_line":"                if not ha_network:"},{"line_number":463,"context_line":"                    ha_network \u003d self._create_ha_network(context,"},{"line_number":464,"context_line":"                                                         router_db.tenant_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_e3cbbc51","line":461,"in_reply_to":"ba8a016a_1cf42f85","updated":"2015-11-24 08:18:51.000000000","message":"I don\u0027t want to split this patch. All current changes are related and won\u0027t work without one another.","commit_id":"ef9d1f8163526fc829b6fadacc3748f517b8fbbd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return allocated_vr_ids"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _allocate_vr_id(self, context, network_id, router_id):"},{"line_number":185,"context_line":"        for count in range(MAX_ALLOCATION_TRIES):"},{"line_number":186,"context_line":"            try:"},{"line_number":187,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_30de1d12","side":"PARENT","line":184,"updated":"2015-12-04 17:44:26.000000000","message":"I looked into the caller of this and for some reason it starts a session. I think this particular issue could be resolved by removing the session start in _set_vr_id.","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"620f5dc56031981aebd7bbbde8371f91833d6daf","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return allocated_vr_ids"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _allocate_vr_id(self, context, network_id, router_id):"},{"line_number":185,"context_line":"        for count in range(MAX_ALLOCATION_TRIES):"},{"line_number":186,"context_line":"            try:"},{"line_number":187,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a8ffd7b_ea3686e6","side":"PARENT","line":184,"updated":"2015-12-01 05:30:59.000000000","message":"The last few days I use rally to test ha router and found that the retry here was not running properly.  The code #L199 will cause roll back.  Some exception message may be :\"InvalidRequestError: This Session\u0027s transaction has been rolled back by a nested rollback() call.  To begin a new transaction, issue Session.rollback() first.\" Because of the roll back, the next allocation will stop due to the roll back exception. And if you set more user count in a tenant in rally input JSON file, the exception will get more frequently. It is interesting that only change the #L187 to nested\u003dTrue will solve this. But as Ann Kamyshnikova motioned that l3_ha_db test case will not pass.","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"f30a4f212fd0e82c40074f5bceb618e7f97f3fdd","unresolved":false,"context_lines":[{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return allocated_vr_ids"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _allocate_vr_id(self, context, network_id, router_id):"},{"line_number":185,"context_line":"        for count in range(MAX_ALLOCATION_TRIES):"},{"line_number":186,"context_line":"            try:"},{"line_number":187,"context_line":"                with context.session.begin(subtransactions\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a8ffd7b_d9625916","side":"PARENT","line":184,"in_reply_to":"9a8ffd7b_ea3686e6","updated":"2015-12-01 16:36:17.000000000","message":"Thanks for pointing this! I will update change soon...","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":217,"context_line":"                vr_id\u003dvr_id).delete()"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _set_vr_id(self, context, router, ha_network):"},{"line_number":220,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":221,"context_line":"            router.extra_attributes.ha_vr_id \u003d self._allocate_vr_id("},{"line_number":222,"context_line":"                context, ha_network.network_id, router.id)"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_50e169cf","side":"PARENT","line":220,"updated":"2015-12-04 17:44:26.000000000","message":"I think this session begin can just go away entirely.","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"43dfda6fc9d0f91e5c3ce396b8f4d8051b35af23","unresolved":false,"context_lines":[{"line_number":217,"context_line":"                vr_id\u003dvr_id).delete()"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _set_vr_id(self, context, router, ha_network):"},{"line_number":220,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":221,"context_line":"            router.extra_attributes.ha_vr_id \u003d self._allocate_vr_id("},{"line_number":222,"context_line":"                context, ha_network.network_id, router.id)"},{"line_number":223,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_f6490632","side":"PARENT","line":220,"in_reply_to":"7a740942_50e169cf","updated":"2015-12-12 20:04:50.000000000","message":"Done","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":266,"context_line":"        except Exception:"},{"line_number":267,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":268,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        try:"},{"line_number":271,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_10ebe1b1","side":"PARENT","line":268,"updated":"2015-12-04 17:44:26.000000000","message":"If we are getting a rollback error here I think it would be a simpler change to just use a new admin context.\n\nfrom neutron import context as nctx\n\nself._core_plugin.delete_network(nctx.get_admin_context(), network[\u0027id\u0027])","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f33044e549fb4b3c69dc80e6e892348e8d2867e5","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":266,"context_line":"        except Exception:"},{"line_number":267,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":268,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        try:"},{"line_number":271,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_af6dd19a","side":"PARENT","line":268,"in_reply_to":"7a740942_10ebe1b1","updated":"2015-12-10 11:52:01.000000000","message":"This is true, but I prefer what Ann is doing instead.","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"cb20c7a193550d439de6bb3ca68b492237e776ef","unresolved":false,"context_lines":[{"line_number":265,"context_line":"                                                                network[\u0027id\u0027])"},{"line_number":266,"context_line":"        except Exception:"},{"line_number":267,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":268,"context_line":"                self._core_plugin.delete_network(admin_ctx, network[\u0027id\u0027])"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        try:"},{"line_number":271,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_3e06ae1c","side":"PARENT","line":268,"in_reply_to":"7a740942_10ebe1b1","updated":"2015-12-11 14:04:19.000000000","message":"This seems to be more a work around then fix to use another context here.","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":324,"context_line":"            return self._create_ha_port_binding(context, port[\u0027id\u0027], router_id)"},{"line_number":325,"context_line":"        except Exception:"},{"line_number":326,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":327,"context_line":"                self._core_plugin.delete_port(context, port[\u0027id\u0027],"},{"line_number":328,"context_line":"                                              l3_port_check\u003dFalse)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def _create_ha_interfaces(self, context, router, ha_network):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_b0d20dff","side":"PARENT","line":327,"updated":"2015-12-04 17:44:26.000000000","message":"same comment here as the network case","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":342,"context_line":"        except Exception:"},{"line_number":343,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":344,"context_line":"                for port_id in port_ids:"},{"line_number":345,"context_line":"                    self._core_plugin.delete_port(admin_ctx, port_id,"},{"line_number":346,"context_line":"                                                  l3_port_check\u003dFalse)"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    def _delete_ha_interfaces(self, context, router_id):"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_d0cd595e","side":"PARENT","line":345,"updated":"2015-12-04 17:44:26.000000000","message":"and here","commit_id":"6d9e0538a4495ff076b47c89d56c8d3c210d8ccd"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f33044e549fb4b3c69dc80e6e892348e8d2867e5","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        return p_utils.create_subnet(self._core_plugin, context,"},{"line_number":234,"context_line":"                                     {\u0027subnet\u0027: args})"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def _safe_creation(self, context, creation, deletion, content):"},{"line_number":237,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":238,"context_line":"            if creation is not None:"},{"line_number":239,"context_line":"                object \u003d creation()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_6f9a8970","line":236,"updated":"2015-12-10 11:52:01.000000000","message":"I reckon this module is not the final place for this function, is it?\nWith some work this could be generic enough to be part of common_db module.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"43dfda6fc9d0f91e5c3ce396b8f4d8051b35af23","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        return p_utils.create_subnet(self._core_plugin, context,"},{"line_number":234,"context_line":"                                     {\u0027subnet\u0027: args})"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def _safe_creation(self, context, creation, deletion, content):"},{"line_number":237,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":238,"context_line":"            if creation is not None:"},{"line_number":239,"context_line":"                object \u003d creation()"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_b6661eb9","line":236,"in_reply_to":"7a740942_6f9a8970","updated":"2015-12-12 20:04:50.000000000","message":"Done","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":254,"context_line":"                                          network_id):"},{"line_number":255,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":256,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":257,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":258,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_f0e735e2","line":255,"updated":"2015-12-04 17:44:26.000000000","message":"I looked at the code and couldn\u0027t figure out when this method is called during a rollback. Why does it need to be converted to nested?","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"1d50ba6b983a36f0bcead04f655fb5b2b2efae09","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":254,"context_line":"                                          network_id):"},{"line_number":255,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":256,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":257,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":258,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_d0cbb251","line":255,"in_reply_to":"7a740942_3b8000bf","updated":"2015-12-12 10:30:24.000000000","message":"We cannot just rollback parent transaction, because creation of network is not pure db operation for all plugins.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"cb20c7a193550d439de6bb3ca68b492237e776ef","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":254,"context_line":"                                          network_id):"},{"line_number":255,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":256,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":257,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":258,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_3b8000bf","line":255,"in_reply_to":"7a740942_afe2b1ff","updated":"2015-12-11 14:04:19.000000000","message":"This function is called in content(), if we leave subtransaction here and during ha_network creation appear exception, transaction from line 237 will be rolled back and deletion will be called incorrectly.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"f33044e549fb4b3c69dc80e6e892348e8d2867e5","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def _create_ha_network_tenant_binding(self, context, tenant_id,"},{"line_number":254,"context_line":"                                          network_id):"},{"line_number":255,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":256,"context_line":"            ha_network \u003d L3HARouterNetwork(tenant_id\u003dtenant_id,"},{"line_number":257,"context_line":"                                           network_id\u003dnetwork_id)"},{"line_number":258,"context_line":"            context.session.add(ha_network)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_afe2b1ff","line":255,"in_reply_to":"7a740942_f0e735e2","updated":"2015-12-10 11:52:01.000000000","message":"I actually might understand what\u0027s your intention, but I\u0027m not sure is related to this patch.\n\nAlso, would it be correct to use nested transactions here? I think if this one aborts perhaps the parent one should abort too.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"37d4fbfaa0c132f40dba05b1436755047c102c2c","unresolved":false,"context_lines":[{"line_number":402,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":405,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":406,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":407,"context_line":"                                self).create_router(context, router)"},{"line_number":408,"context_line":"            if not is_ha:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_080e8234","line":405,"updated":"2015-12-04 00:46:20.000000000","message":"As noted by Kevin on IRC, we can\u0027t actually do this \u003d/\n\nThe reason is that this change puts everything in one transaction. There\u0027s calls to core_plugin.create_{network_port}, and ML2 has post-commit hooks for those resources. Drivers expect that the code they execute actually runs *after the commit*, but if we wrap out those calls like this in an exterior transaction, that won\u0027t be the case. This will cause bugs with a lot of ML2 drivers.\n\nI know that I\u0027m the one that suggested we wrap everything in a transaction and I apologize for leading you down the wrong path. We will need to come up with an alternative solution that doesn\u0027t wrap up ML2 operations in transactions.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":402,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":405,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":406,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":407,"context_line":"                                self).create_router(context, router)"},{"line_number":408,"context_line":"            if not is_ha:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_70dca512","line":405,"updated":"2015-12-04 17:44:26.000000000","message":"IIRC this one is being performed to prevent a partially finished router from being sent to the L3 agent. As a solution to this problem, can you set the status of the router to \u0027BUILD\u0027 at the start here and then update it to ACTIVE after all of the interfaces have been created?\n\nThen all we need to do is adjust the L3 agent to skip any routers in the BUILD state.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"b9fab6be950ea9761ef4d18029452c06c3eaed10","unresolved":false,"context_lines":[{"line_number":402,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":405,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":406,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":407,"context_line":"                                self).create_router(context, router)"},{"line_number":408,"context_line":"            if not is_ha:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_d15502c2","line":405,"in_reply_to":"7a740942_70dca512","updated":"2015-12-07 23:57:20.000000000","message":"The initial transaction that commits the router (line 406-407) would have to commit the router with the \u0027BUILD\u0027 state. The filtering of routers in \u0027BUILD\u0027 state should be happening server-side though, not at the agent.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"bc590c7391756728cefc2c2ed3b88f1bb0a1b64a","unresolved":false,"context_lines":[{"line_number":402,"context_line":"            raise l3_ha.DistributedHARouterNotSupported()"},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        router[\u0027router\u0027][\u0027ha\u0027] \u003d is_ha"},{"line_number":405,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":406,"context_line":"            router_dict \u003d super(L3_HA_NAT_db_mixin,"},{"line_number":407,"context_line":"                                self).create_router(context, router)"},{"line_number":408,"context_line":"            if not is_ha:"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_473da6db","line":405,"in_reply_to":"7a740942_d15502c2","updated":"2015-12-08 16:12:03.000000000","message":"So, what else could we do here to make it simpler?","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"b64a097f7802dcf1dc3ab551192fa0c65783c549","unresolved":false,"context_lines":[{"line_number":474,"context_line":"        self._unbind_ha_router(context, router_id)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        if requested_ha_state:"},{"line_number":477,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":478,"context_line":"                if not ha_network:"},{"line_number":479,"context_line":"                    ha_network \u003d self._create_ha_network(context,"},{"line_number":480,"context_line":"                                                         router_db.tenant_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7a740942_90d7d1ec","line":477,"updated":"2015-12-04 17:44:26.000000000","message":"similar here. start by changing the status to BUILD and then back to ACTIVE after everything is set.","commit_id":"917121a1120448a012295aefdd0da42a8b193dea"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"e1c6c97e5c6ce09911bc553dcc64d68669f65685","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":"#"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import functools"},{"line_number":17,"context_line":"import netaddr"},{"line_number":18,"context_line":"from oslo_config import cfg"},{"line_number":19,"context_line":"from oslo_db import exception as db_exc"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a7b0d38_8909e32e","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":6},"updated":"2015-12-24 13:15:47.000000000","message":"functools is stdlib, so its import shouldn\u0027t be grouped with external lib ones","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5c8161989844fd470fdfd074518a560d97da7ee8","unresolved":false,"context_lines":[{"line_number":13,"context_line":"# under the License."},{"line_number":14,"context_line":"#"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import functools"},{"line_number":17,"context_line":"import netaddr"},{"line_number":18,"context_line":"from oslo_config import cfg"},{"line_number":19,"context_line":"from oslo_db import exception as db_exc"}],"source_content_type":"text/x-python","patch_set":21,"id":"fa69d971_b17f1bde","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":6},"in_reply_to":"1a7b0d38_8909e32e","updated":"2016-01-11 14:50:58.000000000","message":"Done","commit_id":"d22c4d6904ab312e57a44e628499cbadc0f03703"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ab229a15c540c2cc05f5730913a8eaa4036073e2","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                \u0027device_id\u0027: router_id,"},{"line_number":334,"context_line":"                \u0027device_owner\u0027: constants.DEVICE_OWNER_ROUTER_HA_INTF,"},{"line_number":335,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":336,"context_line":"        port \u003d p_utils.create_port(self._core_plugin, context,"},{"line_number":337,"context_line":"                                 {\u0027port\u0027: args})"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":22,"id":"da6ed579_15047713","side":"PARENT","line":336,"range":{"start_line":336,"start_character":34,"end_line":336,"end_character":35},"updated":"2016-01-12 03:09:56.000000000","message":"Race between last HA router deleting and new router create API call","commit_id":"a8c715ab0fae82563252e815a12ddeb37ee5cc23"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ab229a15c540c2cc05f5730913a8eaa4036073e2","unresolved":false,"context_lines":[{"line_number":337,"context_line":"                                 {\u0027port\u0027: args})"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        try:"},{"line_number":340,"context_line":"            return self._create_ha_port_binding(context, port[\u0027id\u0027], router_id)"},{"line_number":341,"context_line":"        except Exception:"},{"line_number":342,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":343,"context_line":"                self._core_plugin.delete_port(context, port[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":22,"id":"da6ed579_f52523ae","side":"PARENT","line":340,"range":{"start_line":340,"start_character":24,"end_line":340,"end_character":47},"updated":"2016-01-12 03:09:56.000000000","message":"race between HA router delete and L3 agent router sync info","commit_id":"a8c715ab0fae82563252e815a12ddeb37ee5cc23"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"47215613ba20d74c6e94753a9ee227eb336d5082","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":"        In case of exception, object is deleted."},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        :param context: context"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa69d971_8723b5bc","line":180,"updated":"2016-01-11 15:01:32.000000000","message":"I\u0027d probably add a link to the devref content that explains why this function is needed and when should it be used.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3654e32df421db91dafa9eb4b74c6ac68f774323","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":"        In case of exception, object is deleted."},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        :param context: context"}],"source_content_type":"text/x-python","patch_set":22,"id":"da6ed579_372bba84","line":180,"in_reply_to":"fa69d971_8723b5bc","updated":"2016-01-12 07:45:08.000000000","message":"The link http://docs.openstack.org/developer/neutron/devref/effective_neutron.html#database-interaction is too long to paste here, so I will add just explanation where to look for.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ab229a15c540c2cc05f5730913a8eaa4036073e2","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_network,"},{"line_number":303,"context_line":"                                     admin_ctx)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        network, ha_network \u003d self._safe_creation("},{"line_number":306,"context_line":"            context, creation, deletion, content)"},{"line_number":307,"context_line":"        try:"},{"line_number":308,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"da6ed579_7225c9c0","line":305,"range":{"start_line":305,"start_character":35,"end_line":305,"end_character":49},"updated":"2016-01-12 03:09:56.000000000","message":"I don\u0027t know what exceptions will be raised here? Seems here do not have any race, so that atomic here is no more needed.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"3654e32df421db91dafa9eb4b74c6ac68f774323","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_network,"},{"line_number":303,"context_line":"                                     admin_ctx)"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        network, ha_network \u003d self._safe_creation("},{"line_number":306,"context_line":"            context, creation, deletion, content)"},{"line_number":307,"context_line":"        try:"},{"line_number":308,"context_line":"            self._create_ha_subnet(admin_ctx, network[\u0027id\u0027], tenant_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"da6ed579_9795c641","line":305,"range":{"start_line":305,"start_character":35,"end_line":305,"end_character":49},"in_reply_to":"da6ed579_7225c9c0","updated":"2016-01-12 07:45:08.000000000","message":"If any exception occurs here (DB connection issue, concurrent transaction issue, bad code issue), we need to properly handle it and leave database (ours or any external one) in a consistent state.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"47215613ba20d74c6e94753a9ee227eb336d5082","unresolved":false,"context_lines":[{"line_number":338,"context_line":"                                                   num_agents\u003dnum_agents)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        return num_agents"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def _create_ha_port_binding(self, context, router_id, port_id):"},{"line_number":343,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":344,"context_line":"            portbinding \u003d L3HARouterAgentPortBinding(port_id\u003dport_id,"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa69d971_22c5175b","line":341,"updated":"2016-01-11 15:01:32.000000000","message":"It\u0027s not necessary to switch the order of the parameters is it?","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"49d3c4006f73ee74e1d737a0b2082145bae76d76","unresolved":false,"context_lines":[{"line_number":338,"context_line":"                                                   num_agents\u003dnum_agents)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"        return num_agents"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    def _create_ha_port_binding(self, context, router_id, port_id):"},{"line_number":343,"context_line":"        with context.session.begin(nested\u003dTrue):"},{"line_number":344,"context_line":"            portbinding \u003d L3HARouterAgentPortBinding(port_id\u003dport_id,"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa69d971_6226afcf","line":341,"in_reply_to":"fa69d971_22c5175b","updated":"2016-01-11 15:07:08.000000000","message":"OK I see why you switched the order.","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"47215613ba20d74c6e94753a9ee227eb336d5082","unresolved":false,"context_lines":[{"line_number":366,"context_line":"        creation \u003d functools.partial(p_utils.create_port, self._core_plugin,"},{"line_number":367,"context_line":"                                     context, {\u0027port\u0027: args})"},{"line_number":368,"context_line":"        content \u003d functools.partial(self._create_ha_port_binding, context,"},{"line_number":369,"context_line":"                                    router_id)"},{"line_number":370,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_port, context,"},{"line_number":371,"context_line":"                                     l3_port_check\u003dFalse)"},{"line_number":372,"context_line":"        port, bindings \u003d self._safe_creation(context, creation, deletion,"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa69d971_82f723d2","line":369,"updated":"2016-01-11 15:01:32.000000000","message":"You\u0027re not passing the port_id here, this can\u0027t work. I see that unit tests are passing also...","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"49d3c4006f73ee74e1d737a0b2082145bae76d76","unresolved":false,"context_lines":[{"line_number":366,"context_line":"        creation \u003d functools.partial(p_utils.create_port, self._core_plugin,"},{"line_number":367,"context_line":"                                     context, {\u0027port\u0027: args})"},{"line_number":368,"context_line":"        content \u003d functools.partial(self._create_ha_port_binding, context,"},{"line_number":369,"context_line":"                                    router_id)"},{"line_number":370,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_port, context,"},{"line_number":371,"context_line":"                                     l3_port_check\u003dFalse)"},{"line_number":372,"context_line":"        port, bindings \u003d self._safe_creation(context, creation, deletion,"}],"source_content_type":"text/x-python","patch_set":22,"id":"fa69d971_a237c7fc","line":369,"in_reply_to":"fa69d971_82f723d2","updated":"2016-01-11 15:07:08.000000000","message":"Never mind I see you\u0027re passing the port_id in line 198. Fancy :)","commit_id":"2ebfd0c3da075db1f732eec788804b74bfe59bda"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"76a9a3b82aacba2b6ea18d4d3ae263851d0285d4","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        self._verify_configuration()"},{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        In case of exception, object is deleted."}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_6afcd83e","line":179,"updated":"2016-01-12 12:43:41.000000000","message":"Did I already comment that I believe this method is better placed in neutron.db.common?","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"5d38521d38d086f02353737d497b22237eee4fe7","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        self._verify_configuration()"},{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        In case of exception, object is deleted."}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_e8f8cf7e","line":179,"in_reply_to":"da6ed579_6afcd83e","updated":"2016-01-12 12:51:52.000000000","message":"Yes, I moved this code there after you comment, but then I got comments from other reviewers that this code is too specific for neutron.db.common, so in 22d patch set I return it here.","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"390550fa21c4247031ca63825929287b75a3f432","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        self._verify_configuration()"},{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        In case of exception, object is deleted."}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_d01c5679","line":179,"in_reply_to":"da6ed579_ab1ad115","updated":"2016-01-13 12:40:05.000000000","message":"OK, I will move to neutron.db.common module. I just hope that this will be the last movement of it :)","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"56f505190c60512eb173e10ba8d59a0bf10c1804","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        self._verify_configuration()"},{"line_number":177,"context_line":"        super(L3_HA_NAT_db_mixin, self).__init__()"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def _safe_creation(sekf, context, create_fn, delete_fn, create_bindings):"},{"line_number":180,"context_line":"        \u0027\u0027\u0027This function wraps logic of object creation in safe atomic way."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        In case of exception, object is deleted."}],"source_content_type":"text/x-python","patch_set":24,"id":"da6ed579_ab1ad115","line":179,"in_reply_to":"da6ed579_e8f8cf7e","updated":"2016-01-12 13:17:08.000000000","message":"I feel the same way as Salvatore. I am sure we can convince Cedric to agree. Helpers like this should be in a common place to encourage usage.","commit_id":"816041de3338b5cedb4c14b5dc7bc4f1f2b07d3e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"2db1dc421f7398dcc79026206dda6ddebdc2e059","unresolved":false,"context_lines":[{"line_number":338,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":339,"context_line":"        creation \u003d functools.partial(p_utils.create_port, self._core_plugin,"},{"line_number":340,"context_line":"                                     context, {\u0027port\u0027: args})"},{"line_number":341,"context_line":"        content \u003d functools.partial(self._create_ha_port_binding, context,"},{"line_number":342,"context_line":"                                    router_id)"},{"line_number":343,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_port, context,"},{"line_number":344,"context_line":"                                     l3_port_check\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9a68dd71_f66f4590","line":341,"range":{"start_line":341,"start_character":41,"end_line":341,"end_character":64},"updated":"2016-01-21 02:55:08.000000000","message":"self._create_ha_port_binding is still facing the DBReferenceError (Integrity Error, I\u0027m dealing with it here  https://review.openstack.org/#/c/260303/) because of the race between HA router create and delete. While only testing this patch, I got so many DBReferenceError  error logs both in the neutron server side and l3 agent side. The error message is very very long which cause a huge using of RPC connection, then L3 agent will sometimes get exceptions:  MessagingTimeout. So is there anyone has some thoughts about this?\n\nAnd I\u0027ve done the work to test merging this patch with https://review.openstack.org/#/c/260303/, it seems the safe_creation do not handle the \u0027content\u0027 return None, then the result is that the \"creation\" lives.","commit_id":"924f19e8f16aebf103a3b70f2bd236afc846933b"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"83e8b534e2bbeda9ab72f89854867718556910da","unresolved":false,"context_lines":[{"line_number":338,"context_line":"                \u0027name\u0027: constants.HA_PORT_NAME % tenant_id}"},{"line_number":339,"context_line":"        creation \u003d functools.partial(p_utils.create_port, self._core_plugin,"},{"line_number":340,"context_line":"                                     context, {\u0027port\u0027: args})"},{"line_number":341,"context_line":"        content \u003d functools.partial(self._create_ha_port_binding, context,"},{"line_number":342,"context_line":"                                    router_id)"},{"line_number":343,"context_line":"        deletion \u003d functools.partial(self._core_plugin.delete_port, context,"},{"line_number":344,"context_line":"                                     l3_port_check\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9a68dd71_7700ea01","line":341,"range":{"start_line":341,"start_character":41,"end_line":341,"end_character":64},"in_reply_to":"9a68dd71_f66f4590","updated":"2016-01-22 08:14:51.000000000","message":"I have WIP change https://review.openstack.org/#/c/257059/ this can help handle problems with ha router creation. \n\nYou can improve this in your patch.","commit_id":"924f19e8f16aebf103a3b70f2bd236afc846933b"}],"neutron/tests/unit/db/test_common_db_mixin.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"79e37931ffab9cee9a2aa6295a90363fcc6e077a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"da6ed579_a023fabb","line":45,"updated":"2016-01-15 16:57:19.000000000","message":"Nit: It might make sense to also check that delete_fn was called with the expected object id.","commit_id":"7592f3ae1fb871e99707f555c119673015f5a4fa"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"575be539b77cfe6969d7a145953953184b493126","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"da6ed579_68a4c907","line":45,"in_reply_to":"da6ed579_a023fabb","updated":"2016-01-18 09:26:32.000000000","message":"Done","commit_id":"7592f3ae1fb871e99707f555c119673015f5a4fa"}]}