)]}'
{"/PATCHSET_LEVEL":[{"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":"1de4f13a5e439982e260cd81617080a23c42d0c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c64ca93b_5e3a8213","updated":"2022-05-25 13:15:14.000000000","message":"adding of course, that both fixes here are using private API on SessionTransaction, and in SQLAlchemy, we\u0027d like to make public API available here, if we can figure out exactly what it is you need to know.","commit_id":"2f15b21f8857a0e5ed09621be5b6a1b10309977d"},{"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":"c230b86d5ec36d2e0000a0833e5f6496f9e7711e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0b963ba7_b0b67da6","updated":"2022-05-25 13:13:14.000000000","message":"not an easy answer because neutron is itself in an uneasy state with this sort of thing","commit_id":"2f15b21f8857a0e5ed09621be5b6a1b10309977d"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"0b88b9e123c9b8eaf4f494568e26962ae56436ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8d2071ed_bf88a0f7","updated":"2022-05-30 16:03:13.000000000","message":"This patch fixed the issue we see in networking-baremetal CI: https://review.opendev.org/c/openstack/networking-baremetal/+/842578","commit_id":"161a54bf71e734d143fd1be603e5cb1f4f807760"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ce6e4772a69602da6fd4cd566507410a7f738882","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0a79b3c6_889c63bb","updated":"2022-06-09 12:11:50.000000000","message":"recheck","commit_id":"6b529ea3c559ed1a016e2aa2c30769429192d528"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4f4dc77aacc38b06e7848c9f79bd4d8e3ffaec31","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"237727c1_58d77e26","updated":"2022-06-09 09:50:31.000000000","message":"recheck","commit_id":"6b529ea3c559ed1a016e2aa2c30769429192d528"}],"neutron/common/utils.py":[{"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":"c230b86d5ec36d2e0000a0833e5f6496f9e7711e","unresolved":true,"context_lines":[{"line_number":1044,"context_line":"        return session.is_active"},{"line_number":1045,"context_line":"    if not session.transaction:"},{"line_number":1046,"context_line":"        return False"},{"line_number":1047,"context_line":"    if not (session.transaction._dirty or session.transaction._deleted or"},{"line_number":1048,"context_line":"            session.transaction._new):"},{"line_number":1049,"context_line":"        return False"},{"line_number":1050,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"d9c4df63_43c94029","line":1047,"updated":"2022-05-25 13:13:14.000000000","message":"it looks like what this is checking is if the SessionTransaction has accumulated object changes that have been flushed.   It\u0027s true that if a flush has proceeded, that will indicate that the SessionTransaction is holding onto active connections.\n\n\nHowever, if we are in fact interested in whether or not the SessionTransaction is actually holding onto checked-out database connections (which from a DBAPI point of view are therefore in a \"tranasction\"), you could simply look to see if it has any database connections, like this:\n\n\n    if bool(session.transaction._connections):\n        return True\n    else:\n        return False\n\nnow the above is a much more broad check -  a Session that was used just to emit some SELECT queries will report itself as \"active\" (which it is.  but i dont fully know what it is you want to determine here).\n\nIf, OTOH, you are only interested in a SessionTransaction where actual data changes (i.e. INSERT, UPDATE, DELETE) statements have proceeded, the previous check you have will **partially** work - it won\u0027t tell you if someone has run an UPDATE or DELETE outside of the unit of work, such as if someone said session.query(Model).update(), which it\u0027s very likely that Neutron makes use of somewhere.\n\nSo testing for \"_connections\" is definitely the more broad-based, \"do we have any database resources in play\" test and it might be better if you can make that work for you.\n\n\nOverall the biggest problem is that neutron really shouldn\u0027t have cases where it even needs to know if the session has active changes or not, it would ideally organize its operations such that if an operation needs to proceed with no transaction to start with, then no transaction is started in the first place; that is, it shouldn\u0027t be asking questions about what other parts of the code might have done.   This is obviously a longer term architectural thing though.","commit_id":"2f15b21f8857a0e5ed09621be5b6a1b10309977d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f9fda6ad6c10ca6311d8b8d8762781125cdc9325","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"        return session.is_active"},{"line_number":1045,"context_line":"    if not session.transaction:"},{"line_number":1046,"context_line":"        return False"},{"line_number":1047,"context_line":"    if not (session.transaction._dirty or session.transaction._deleted or"},{"line_number":1048,"context_line":"            session.transaction._new):"},{"line_number":1049,"context_line":"        return False"},{"line_number":1050,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"f305a20c_3353aa8b","line":1047,"in_reply_to":"d9c4df63_43c94029","updated":"2022-05-30 08:15:36.000000000","message":"I\u0027ll start with the last comment. I agree with you: Neutron should not check this status. The code should have been implemented in a way that we don\u0027t need to check the status of the DB session (or transaction). But there are some bits of the code implemented when nested transaction where allowed and other sections use this check to protect from modifying the DB during another operation transaction.\n\nYou are right on the fact that checking the connections is a more broad check. But in this case is a valid check and better that checking of there is any pending operation. Read (SELECT) operations do not increase any \"new\", \"dirty\" or \"deleted\" counters. I\u0027ll use it instead.\n\nIn any case, I\u0027ll review all Neutron uses of this method in order to refactor the code and avoid it.","commit_id":"2f15b21f8857a0e5ed09621be5b6a1b10309977d"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"cdca1770271ea43d0b9ecbaef3186f9cedfc60e4","unresolved":true,"context_lines":[{"line_number":1037,"context_line":"    session transaction will not end at the end of a reader/writer context."},{"line_number":1038,"context_line":"    In this case, a session could have an active transaction even when it is"},{"line_number":1039,"context_line":"    not inside a reader/writer context. In order to mimic the previous"},{"line_number":1040,"context_line":"    behaviour, this method checks the pending new, deleted and dirty elements"},{"line_number":1041,"context_line":"    to be flushed."},{"line_number":1042,"context_line":"    \"\"\""},{"line_number":1043,"context_line":"    if session.autocommit:  # old behaviour, to be removed with sqlalchemy 2.0"},{"line_number":1044,"context_line":"        return session.is_active"}],"source_content_type":"text/x-python","patch_set":3,"id":"41605835_223b582d","line":1041,"range":{"start_line":1040,"start_character":15,"end_line":1041,"end_character":17},"updated":"2022-06-07 22:43:17.000000000","message":"Should we update this docstring?","commit_id":"161a54bf71e734d143fd1be603e5cb1f4f807760"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"089d8d6f6d970b61bbbfe1893f6e8835146f40d6","unresolved":false,"context_lines":[{"line_number":1037,"context_line":"    session transaction will not end at the end of a reader/writer context."},{"line_number":1038,"context_line":"    In this case, a session could have an active transaction even when it is"},{"line_number":1039,"context_line":"    not inside a reader/writer context. In order to mimic the previous"},{"line_number":1040,"context_line":"    behaviour, this method checks the pending new, deleted and dirty elements"},{"line_number":1041,"context_line":"    to be flushed."},{"line_number":1042,"context_line":"    \"\"\""},{"line_number":1043,"context_line":"    if session.autocommit:  # old behaviour, to be removed with sqlalchemy 2.0"},{"line_number":1044,"context_line":"        return session.is_active"}],"source_content_type":"text/x-python","patch_set":3,"id":"dcc7d008_ddfda55d","line":1041,"range":{"start_line":1040,"start_character":15,"end_line":1041,"end_character":17},"in_reply_to":"41605835_223b582d","updated":"2022-06-08 08:17:25.000000000","message":"Done","commit_id":"161a54bf71e734d143fd1be603e5cb1f4f807760"}]}