)]}'
{"/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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3bf01020_be04ca45","updated":"2024-05-07 21:28:08.000000000","message":"It seems like this can be made more succinct by just filtering for is_modified() right where you get the session.dirty list, and if you dont need session.deleted then just remove that from the expression.","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6c94966d479e7348aea62760171dea98af866f14","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bc571960_c4e289b0","updated":"2024-05-08 09:08:18.000000000","message":"Follow up on this another one https://review.opendev.org/c/openstack/neutron/+/918492","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8c44260143efd9d6fef2b32be669fa7a868ba145","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f0e35ee9_a2fb2bc8","updated":"2024-05-08 01:00:22.000000000","message":"Posted a follow-up with what I think should fix the test failures: https://review.opendev.org/c/openstack/neutron/+/918490","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dbd56aff79b80c80648e83c6cf17247c3a6ab23d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"55016d89_9b20cde7","updated":"2024-05-07 22:04:06.000000000","message":"Thanks Mike for the inputs!","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"}],"neutron/services/revisions/revision_plugin.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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        # see if any created/updated/deleted objects bump the revision"},{"line_number":86,"context_line":"        # of another object"},{"line_number":87,"context_line":"        objects_with_related_revisions \u003d ["},{"line_number":88,"context_line":"            o for o in session.deleted | session.dirty | session.new"},{"line_number":89,"context_line":"            if getattr(o, \u0027revises_on_change\u0027, ())"},{"line_number":90,"context_line":"        ]"},{"line_number":91,"context_line":"        collected \u003d session.info.setdefault(\u0027_related_bumped\u0027, set())"}],"source_content_type":"text/x-python","patch_set":1,"id":"173fcec2_c5889c83","line":88,"updated":"2024-05-07 21:28:08.000000000","message":"if you are filtering out deleted down below, maybe just remove session.deleted from here?","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dbd56aff79b80c80648e83c6cf17247c3a6ab23d","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        # see if any created/updated/deleted objects bump the revision"},{"line_number":86,"context_line":"        # of another object"},{"line_number":87,"context_line":"        objects_with_related_revisions \u003d ["},{"line_number":88,"context_line":"            o for o in session.deleted | session.dirty | session.new"},{"line_number":89,"context_line":"            if getattr(o, \u0027revises_on_change\u0027, ())"},{"line_number":90,"context_line":"        ]"},{"line_number":91,"context_line":"        collected \u003d session.info.setdefault(\u0027_related_bumped\u0027, set())"}],"source_content_type":"text/x-python","patch_set":1,"id":"1b870e28_90bcd845","line":88,"in_reply_to":"173fcec2_c5889c83","updated":"2024-05-07 22:04:06.000000000","message":"To my understanding _collect_related_tobump() is recursive so it needs to process also deleted objects and its related objects.","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"4e0ebbd694c8730552cb70e91c6fba9163116907","unresolved":true,"context_lines":[{"line_number":85,"context_line":"        # see if any created/updated/deleted objects bump the revision"},{"line_number":86,"context_line":"        # of another object"},{"line_number":87,"context_line":"        objects_with_related_revisions \u003d ["},{"line_number":88,"context_line":"            o for o in session.deleted | session.dirty | session.new"},{"line_number":89,"context_line":"            if getattr(o, \u0027revises_on_change\u0027, ())"},{"line_number":90,"context_line":"        ]"},{"line_number":91,"context_line":"        collected \u003d session.info.setdefault(\u0027_related_bumped\u0027, set())"}],"source_content_type":"text/x-python","patch_set":1,"id":"d9e38230_071cff84","line":88,"in_reply_to":"1b870e28_90bcd845","updated":"2024-05-07 22:12:52.000000000","message":"OK but it looks like you added a skip for deleted in that function, which makes it seem like they are never considered.  im not really sure if that\u0027s correct here.  if this is recursive, a deleted object could have related objects that dont get deleted, in theory.","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":true,"context_lines":[{"line_number":86,"context_line":"        # of another object"},{"line_number":87,"context_line":"        objects_with_related_revisions \u003d ["},{"line_number":88,"context_line":"            o for o in session.deleted | session.dirty | session.new"},{"line_number":89,"context_line":"            if getattr(o, \u0027revises_on_change\u0027, ())"},{"line_number":90,"context_line":"        ]"},{"line_number":91,"context_line":"        collected \u003d session.info.setdefault(\u0027_related_bumped\u0027, set())"},{"line_number":92,"context_line":"        self._collect_related_tobump("}],"source_content_type":"text/x-python","patch_set":1,"id":"46577f64_9cfca311","line":89,"updated":"2024-05-07 21:28:08.000000000","message":"maybe you could do the is_modified() limit here?","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dbd56aff79b80c80648e83c6cf17247c3a6ab23d","unresolved":true,"context_lines":[{"line_number":86,"context_line":"        # of another object"},{"line_number":87,"context_line":"        objects_with_related_revisions \u003d ["},{"line_number":88,"context_line":"            o for o in session.deleted | session.dirty | session.new"},{"line_number":89,"context_line":"            if getattr(o, \u0027revises_on_change\u0027, ())"},{"line_number":90,"context_line":"        ]"},{"line_number":91,"context_line":"        collected \u003d session.info.setdefault(\u0027_related_bumped\u0027, set())"},{"line_number":92,"context_line":"        self._collect_related_tobump("}],"source_content_type":"text/x-python","patch_set":1,"id":"1781fa7d_161df12d","line":89,"in_reply_to":"46577f64_9cfca311","updated":"2024-05-07 22:04:06.000000000","message":"I think one of the multiple versions I crafted had it there. I can investigate.","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        for obj in objects:"},{"line_number":114,"context_line":"            if obj in collected:"},{"line_number":115,"context_line":"                continue"},{"line_number":116,"context_line":"            if not (session.is_modified(obj) or obj in session.deleted):"},{"line_number":117,"context_line":"                continue"},{"line_number":118,"context_line":"            for revises_col in getattr(obj, \u0027revises_on_change\u0027, ()):"},{"line_number":119,"context_line":"                related_obj \u003d self._find_related_obj(obj, revises_col)"}],"source_content_type":"text/x-python","patch_set":1,"id":"172a0730_f4c0c950","line":116,"updated":"2024-05-07 21:28:08.000000000","message":"im not sure what session.is_modified() does for an object from session.new...it should return True?   are you able to test?","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6c94966d479e7348aea62760171dea98af866f14","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        for obj in objects:"},{"line_number":114,"context_line":"            if obj in collected:"},{"line_number":115,"context_line":"                continue"},{"line_number":116,"context_line":"            if not (session.is_modified(obj) or obj in session.deleted):"},{"line_number":117,"context_line":"                continue"},{"line_number":118,"context_line":"            for revises_col in getattr(obj, \u0027revises_on_change\u0027, ()):"},{"line_number":119,"context_line":"                related_obj \u003d self._find_related_obj(obj, revises_col)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f589c2cb_6948ef2f","line":116,"in_reply_to":"172a0730_f4c0c950","updated":"2024-05-08 09:08:18.000000000","message":"Yeah, I just tested, an object from session.net returns True","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":false,"context_lines":[{"line_number":174,"context_line":"    def _bump_obj_revisions(self, session, objects, version_check\u003dTrue):"},{"line_number":175,"context_line":"        \"\"\"Increment object revisions."},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        If version_check\u003dTrue, uses SQLAlchemy ORM\u0027s compare-and-swap feature"},{"line_number":178,"context_line":"        (known as \"version_id_col\" in the ORM mapping), which is part of the"},{"line_number":179,"context_line":"        StandardAttribute class."},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b802f045_02f1f27f","line":177,"updated":"2024-05-07 21:28:08.000000000","message":"I know I wrote this but this is nutty code huh?    version_check\u003dTrue is the before_flush version, where session.is_modified() is meaningful, version_check\u003dFalse is the after_flush_postexec version, where objects would have been reset.  OK!","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"0b4307744564d5c79d99e2ebb87167d59d63d020","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            to_bump \u003d ["},{"line_number":194,"context_line":"                obj for obj in objects"},{"line_number":195,"context_line":"                if not getattr(obj, \u0027_rev_bumped\u0027, False)"},{"line_number":196,"context_line":"                and session.is_modified(obj)]"},{"line_number":197,"context_line":"        else:"},{"line_number":198,"context_line":"            to_bump \u003d ["},{"line_number":199,"context_line":"                obj for obj in objects"}],"source_content_type":"text/x-python","patch_set":1,"id":"38da7911_c6ca06be","line":196,"updated":"2024-05-07 21:28:08.000000000","message":"OK, as long as this comes from before_flush where it\u0027s meaningful, it should have an effect...","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"dbd56aff79b80c80648e83c6cf17247c3a6ab23d","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            to_bump \u003d ["},{"line_number":194,"context_line":"                obj for obj in objects"},{"line_number":195,"context_line":"                if not getattr(obj, \u0027_rev_bumped\u0027, False)"},{"line_number":196,"context_line":"                and session.is_modified(obj)]"},{"line_number":197,"context_line":"        else:"},{"line_number":198,"context_line":"            to_bump \u003d ["},{"line_number":199,"context_line":"                obj for obj in objects"}],"source_content_type":"text/x-python","patch_set":1,"id":"ef3b7cd8_aea4e166","line":196,"in_reply_to":"38da7911_c6ca06be","updated":"2024-05-07 22:04:06.000000000","message":"Yeah, that\u0027s why I had to branch the code - it was always False after the flush as you told me.","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"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":"4e0ebbd694c8730552cb70e91c6fba9163116907","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            to_bump \u003d ["},{"line_number":194,"context_line":"                obj for obj in objects"},{"line_number":195,"context_line":"                if not getattr(obj, \u0027_rev_bumped\u0027, False)"},{"line_number":196,"context_line":"                and session.is_modified(obj)]"},{"line_number":197,"context_line":"        else:"},{"line_number":198,"context_line":"            to_bump \u003d ["},{"line_number":199,"context_line":"                obj for obj in objects"}],"source_content_type":"text/x-python","patch_set":1,"id":"952363a8_893bc007","line":196,"in_reply_to":"ef3b7cd8_aea4e166","updated":"2024-05-07 22:12:52.000000000","message":"OK dont go crazy then, this code is very weird and it was very hard to write","commit_id":"fc6c41e80167bbc1dbe04515050d78a2a3de7064"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0c0dde7cb37900ccd33469f1356d687be3022695","unresolved":true,"context_lines":[{"line_number":80,"context_line":"        # bump revision number for updated objects in the session"},{"line_number":81,"context_line":"        self._bump_obj_revisions("},{"line_number":82,"context_line":"            session,"},{"line_number":83,"context_line":"            self._get_objects_to_bump_revision(session.dirty))"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        # see if any created/updated/deleted objects bump the revision"},{"line_number":86,"context_line":"        # of another object"}],"source_content_type":"text/x-python","patch_set":2,"id":"649a8cdd_c1833300","line":83,"updated":"2024-05-07 23:13:18.000000000","message":"I think if you\u0027d not pass all dirty objects here, but instead pass only those of actual interest (\"is_modified\"), then you would not have to branch on version_check in line 192.","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"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":"4e0ebbd694c8730552cb70e91c6fba9163116907","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        for obj in objects:"},{"line_number":114,"context_line":"            if obj in collected:"},{"line_number":115,"context_line":"                continue"},{"line_number":116,"context_line":"            if not (session.is_modified(obj) or obj in session.deleted):"},{"line_number":117,"context_line":"                continue"},{"line_number":118,"context_line":"            for revises_col in getattr(obj, \u0027revises_on_change\u0027, ()):"},{"line_number":119,"context_line":"                related_obj \u003d self._find_related_obj(obj, revises_col)"}],"source_content_type":"text/x-python","patch_set":2,"id":"d50b3968_830492e5","line":116,"updated":"2024-05-07 22:12:52.000000000","message":"right here anything in .deleted is filtered out","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0c0dde7cb37900ccd33469f1356d687be3022695","unresolved":true,"context_lines":[{"line_number":113,"context_line":"        for obj in objects:"},{"line_number":114,"context_line":"            if obj in collected:"},{"line_number":115,"context_line":"                continue"},{"line_number":116,"context_line":"            if not (session.is_modified(obj) or obj in session.deleted):"},{"line_number":117,"context_line":"                continue"},{"line_number":118,"context_line":"            for revises_col in getattr(obj, \u0027revises_on_change\u0027, ()):"},{"line_number":119,"context_line":"                related_obj \u003d self._find_related_obj(obj, revises_col)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bd552771_f840470a","line":116,"in_reply_to":"d50b3968_830492e5","updated":"2024-05-07 23:13:18.000000000","message":"Yep. I think before the patch, we were bumping related objects on delete. Maybe it was not intended, but such change should not be part of this bug fix and should be considered separately.","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0c0dde7cb37900ccd33469f1356d687be3022695","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        \"\"\""},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # filter objects for which we\u0027ve already bumped the revision"},{"line_number":192,"context_line":"        if version_check:"},{"line_number":193,"context_line":"            to_bump \u003d ["},{"line_number":194,"context_line":"                obj for obj in objects"},{"line_number":195,"context_line":"                if not getattr(obj, \u0027_rev_bumped\u0027, False) and"}],"source_content_type":"text/x-python","patch_set":2,"id":"17fe7aee_d9282c57","line":192,"updated":"2024-05-07 23:13:18.000000000","message":"it could help the next reader if you could explain why you branch here (I assume it\u0027s because the to-be-bumped objects are \"related\" objects and they were not touched, yet.)","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"051c7430bee6c022d3f35ff8a275f7ba4f21f84a","unresolved":true,"context_lines":[{"line_number":193,"context_line":"            to_bump \u003d ["},{"line_number":194,"context_line":"                obj for obj in objects"},{"line_number":195,"context_line":"                if not getattr(obj, \u0027_rev_bumped\u0027, False) and"},{"line_number":196,"context_line":"                session.is_modified(obj)]"},{"line_number":197,"context_line":"        else:"},{"line_number":198,"context_line":"            to_bump \u003d ["},{"line_number":199,"context_line":"                obj for obj in objects"}],"source_content_type":"text/x-python","patch_set":2,"id":"12132f22_200d30ac","line":196,"updated":"2024-05-08 00:45:40.000000000","message":"this won\u0027t work for StandardAttributes objects. I think this is a good reason to move this check to _get_objects_to_bump_revision where the filter can apply to HasStandardAttributes objects only.","commit_id":"3783d32459a27cc0d24d10780ebbd2099d6adb66"}]}