)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d10a822230120a734aad1a00af9c36a1e984b45b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"95d5fbd7_bdfce659","updated":"2022-08-19 17:25:19.000000000","message":"Carrying over Tristan\u0027s +2 as this new patchset only modifies the test in a very minor way (a redundant line is removed and two extra assertions are added).","commit_id":"e6530d11d058e50c41872f3c2c9ac286b57ed70e"}],"tests/unit/test_gerrit.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"b24f29b5a9bb12924cdb1cff2256a7dac74a1509","unresolved":true,"context_lines":[{"line_number":893,"context_line":"        A \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027A\u0027)"},{"line_number":894,"context_line":"        B \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027B\u0027)"},{"line_number":895,"context_line":"        B.setDependsOn(A, 1)"},{"line_number":896,"context_line":"        B.addApproval(\u0027Code-Review\u0027, 2)"},{"line_number":897,"context_line":"        # Hold the connection queue processing so these events get"},{"line_number":898,"context_line":"        # processed together"},{"line_number":899,"context_line":"        with lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"41232e6b_f7bf9b6d","line":896,"updated":"2022-08-18 21:48:25.000000000","message":"Is this line redundant with line 902?","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"73c303a25fb974a375cb0981181d3ef4460993ec","unresolved":true,"context_lines":[{"line_number":893,"context_line":"        A \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027A\u0027)"},{"line_number":894,"context_line":"        B \u003d self.fake_gerrit.addFakeChange(\u0027org/project\u0027, \u0027master\u0027, \u0027B\u0027)"},{"line_number":895,"context_line":"        B.setDependsOn(A, 1)"},{"line_number":896,"context_line":"        B.addApproval(\u0027Code-Review\u0027, 2)"},{"line_number":897,"context_line":"        # Hold the connection queue processing so these events get"},{"line_number":898,"context_line":"        # processed together"},{"line_number":899,"context_line":"        with lock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c7635e5_5cac8a02","line":896,"in_reply_to":"41232e6b_f7bf9b6d","updated":"2022-08-19 16:37:50.000000000","message":"I think so.  I think we could remove this line, but it\u0027s harmless.","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"b24f29b5a9bb12924cdb1cff2256a7dac74a1509","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            dict(name\u003d\"project-test2\", result\u003d\"SUCCESS\", changes\u003d\"1,1 2,1\"),"},{"line_number":917,"context_line":"        ], ordered\u003dFalse)"},{"line_number":918,"context_line":"        self.assertEqual(A.queried, 3)"},{"line_number":919,"context_line":"        self.assertEqual(B.queried, 2)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ed928248_c47aeb04","line":919,"updated":"2022-08-18 21:48:25.000000000","message":"Does it make sense to hold the lock for all the approvals so that we can see each of them is queried once and then the jobs run?","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"73c303a25fb974a375cb0981181d3ef4460993ec","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            dict(name\u003d\"project-test2\", result\u003d\"SUCCESS\", changes\u003d\"1,1 2,1\"),"},{"line_number":917,"context_line":"        ], ordered\u003dFalse)"},{"line_number":918,"context_line":"        self.assertEqual(A.queried, 3)"},{"line_number":919,"context_line":"        self.assertEqual(B.queried, 2)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2d3bf7b3_3365f7ae","line":919,"in_reply_to":"ed928248_c47aeb04","updated":"2022-08-19 16:37:50.000000000","message":"I\u0027m not sure I follow this question, can you elaborate?","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b1ae3ba5793ac808f76e8b14df3807ec09746db9","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            dict(name\u003d\"project-test2\", result\u003d\"SUCCESS\", changes\u003d\"1,1 2,1\"),"},{"line_number":917,"context_line":"        ], ordered\u003dFalse)"},{"line_number":918,"context_line":"        self.assertEqual(A.queried, 3)"},{"line_number":919,"context_line":"        self.assertEqual(B.queried, 2)"}],"source_content_type":"text/x-python","patch_set":1,"id":"47cb84c2_a02b7547","line":919,"in_reply_to":"ed928248_c47aeb04","updated":"2022-08-19 17:08:35.000000000","message":"The first batch of approvals added is 3 events that result in 2 queries, which verifies this change is working as expected.  You\u0027re right that the lock isn\u0027t needed for the second batch of approvals since it\u0027s a batch size of one.  But I do think we want to verify the query count before that to make sure it isn\u0027t 3 (as it would have been before this change).  I\u0027ll add comments to explain the query count.","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"eaf33313e7438585cc7d12a618fada37f35d06f9","unresolved":true,"context_lines":[{"line_number":916,"context_line":"            dict(name\u003d\"project-test2\", result\u003d\"SUCCESS\", changes\u003d\"1,1 2,1\"),"},{"line_number":917,"context_line":"        ], ordered\u003dFalse)"},{"line_number":918,"context_line":"        self.assertEqual(A.queried, 3)"},{"line_number":919,"context_line":"        self.assertEqual(B.queried, 2)"}],"source_content_type":"text/x-python","patch_set":1,"id":"48bc6a89_2f966344","line":919,"in_reply_to":"ed928248_c47aeb04","updated":"2022-08-19 16:41:34.000000000","message":"To clarify I mean instead of holding the lock in two locations  (line 899 and then line 907) with assertions between them, do we (also) need to do a single hold of the lock that allows all the changes to enqueue together at once and then assert the changes are queried once?\n\nIn particular I think after the first set of assertions (line 906) nothing has entered the gate pipeline yet because the parent change is not approved. We then approve the parent change with the lock, but that is a single event so the lock isn\u0027t doing much? We then assert the number of queries which seems higher than expected (2 new queries for A and 1 new query for B).","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"}],"zuul/driver/gerrit/gerritconnection.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"b24f29b5a9bb12924cdb1cff2256a7dac74a1509","unresolved":true,"context_lines":[{"line_number":359,"context_line":"            else:"},{"line_number":360,"context_line":"                # We have a cache entry for this change Get the"},{"line_number":361,"context_line":"                # query ltime for the cache entry; if it\u0027s after the"},{"line_number":362,"context_line":"                # event ltime, we don\u0027t need to refresh."},{"line_number":363,"context_line":"                if (change.zuul_query_ltime and"},{"line_number":364,"context_line":"                    change.zuul_query_ltime \u003e connection_event_ltime):"},{"line_number":365,"context_line":"                    refresh \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"bdfe8cfa_b2ddfba6","line":362,"updated":"2022-08-18 21:48:25.000000000","message":"I think this does introduce a subtle behavior difference. For certain events if the change had updated after the event we would have processed the change in an updated fashion. But now we\u0027ll process the previous version.\n\nI think this is actually more correct this way. But do we need to make note of that?","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"73c303a25fb974a375cb0981181d3ef4460993ec","unresolved":true,"context_lines":[{"line_number":359,"context_line":"            else:"},{"line_number":360,"context_line":"                # We have a cache entry for this change Get the"},{"line_number":361,"context_line":"                # query ltime for the cache entry; if it\u0027s after the"},{"line_number":362,"context_line":"                # event ltime, we don\u0027t need to refresh."},{"line_number":363,"context_line":"                if (change.zuul_query_ltime and"},{"line_number":364,"context_line":"                    change.zuul_query_ltime \u003e connection_event_ltime):"},{"line_number":365,"context_line":"                    refresh \u003d False"}],"source_content_type":"text/x-python","patch_set":1,"id":"9d06166b_e321ed3f","line":362,"in_reply_to":"bdfe8cfa_b2ddfba6","updated":"2022-08-19 16:37:50.000000000","message":"I agree.  I don\u0027t think we\u0027ve described this behavior in detail in the docs, so I\u0027m not sure it warrants a release note (especially since we\u0027re making things more \"technically correct\").","commit_id":"764f00f13605ecca7aee6a3c71b43644c7891d9f"}]}