)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"4d0da91d1221408983849dcd08e35aa578b83e93","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7a05cdc4_0345c0ed","updated":"2024-10-25 22:44:46.000000000","message":"Wow.","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"}],"tests/fixtures/config/remote-zuul-stream/git/org_project/playbooks/command.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"776406bea1c4429bd7bfdc19aef06358bbe61f51","unresolved":true,"context_lines":[{"line_number":171,"context_line":"    - name: Output 70KB each to stdout and stderr"},{"line_number":172,"context_line":"      # We use print (as opposed to write) so this uses buffered"},{"line_number":173,"context_line":"      # output in order to encourage a deadlock."},{"line_number":174,"context_line":"      command: python3 -c \u0027import sys; print(\"x\" * 70000, file\u003dsys.stdout); print(\"x\" * 70000, file\u003dsys.stderr)\u0027"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"43ce7081_12f260f6","line":174,"updated":"2024-10-25 20:19:40.000000000","message":"Re my comments in the implementation side of things I wonder if we should assert the total size of the resulting console output to ensure we don\u0027t get short reads after the fact due to short/over writes.","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"18d5164d8c5af1fdd6b2e07d54850c4c7243605d","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    - name: Output 70KB each to stdout and stderr"},{"line_number":172,"context_line":"      # We use print (as opposed to write) so this uses buffered"},{"line_number":173,"context_line":"      # output in order to encourage a deadlock."},{"line_number":174,"context_line":"      command: python3 -c \u0027import sys; print(\"x\" * 70000, file\u003dsys.stdout); print(\"x\" * 70000, file\u003dsys.stderr)\u0027"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"ff3ce0fc_cc138c90","line":174,"in_reply_to":"43ce7081_12f260f6","updated":"2024-10-25 21:16:11.000000000","message":"It may still be a good idea to make this test more robust but I am satisfied with the proof through docs unless we see contradictory behavior later.","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"}],"zuul/ansible/8/library/command.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"776406bea1c4429bd7bfdc19aef06358bbe61f51","unresolved":true,"context_lines":[{"line_number":309,"context_line":"        if not line[-1] !\u003d b\u0027\\n\u0027:"},{"line_number":310,"context_line":"            line +\u003d b\u0027\\n\u0027"},{"line_number":311,"context_line":"            newline_warning \u003d True"},{"line_number":312,"context_line":"        console.addLine(line)"},{"line_number":313,"context_line":"    if newline_warning:"},{"line_number":314,"context_line":"        console.addLine(\u0027[Zuul] No trailing newline\\n\u0027)"},{"line_number":315,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5ce64d1b_95601c4f","line":312,"updated":"2024-10-25 20:19:40.000000000","message":"See comment in the base implementation.","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"18d5164d8c5af1fdd6b2e07d54850c4c7243605d","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        if not line[-1] !\u003d b\u0027\\n\u0027:"},{"line_number":310,"context_line":"            line +\u003d b\u0027\\n\u0027"},{"line_number":311,"context_line":"            newline_warning \u003d True"},{"line_number":312,"context_line":"        console.addLine(line)"},{"line_number":313,"context_line":"    if newline_warning:"},{"line_number":314,"context_line":"        console.addLine(\u0027[Zuul] No trailing newline\\n\u0027)"},{"line_number":315,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"76f3edc7_ee5410af","line":312,"in_reply_to":"5ce64d1b_95601c4f","updated":"2024-10-25 21:16:11.000000000","message":"Done","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"}],"zuul/ansible/base/library/command.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"776406bea1c4429bd7bfdc19aef06358bbe61f51","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        if not line[-1] !\u003d b\u0027\\n\u0027:"},{"line_number":323,"context_line":"            line +\u003d b\u0027\\n\u0027"},{"line_number":324,"context_line":"            newline_warning \u003d True"},{"line_number":325,"context_line":"        console.addLine(line)"},{"line_number":326,"context_line":"    if newline_warning:"},{"line_number":327,"context_line":"        console.addLine(\u0027[Zuul] No trailing newline\\n\u0027)"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1d37157c_727566c6","line":325,"updated":"2024-10-25 20:19:40.000000000","message":"The console object is shared between two threads which means we share a single self.logfile reference that we write against. With multiple threads attempting to write to this file is it possible that the file position indicator for our write location to not be thread safe resulting in us writing stdout over stderr or vice versa?\n\nDo we need to synchronize around the lock to prevent that from happening?","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b5afc688c7078ffe1c964ae6c1966bb1f96e6412","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        if not line[-1] !\u003d b\u0027\\n\u0027:"},{"line_number":323,"context_line":"            line +\u003d b\u0027\\n\u0027"},{"line_number":324,"context_line":"            newline_warning \u003d True"},{"line_number":325,"context_line":"        console.addLine(line)"},{"line_number":326,"context_line":"    if newline_warning:"},{"line_number":327,"context_line":"        console.addLine(\u0027[Zuul] No trailing newline\\n\u0027)"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b81ad8c9_8e87dbf1","line":325,"in_reply_to":"1d37157c_727566c6","updated":"2024-10-25 21:01:17.000000000","message":"This should be safe because (logical proof follows!):\n\n* We open the file with buffering\u003d0; this returns an io.FileIO object[1].\n\n* \"FileIO objects are thread-safe to the extent that the operating\n  system calls (such as read(2) under Unix) they wrap are thread-safe too. [2]\n\n* The OS call we are concerned with is \"write\".\n\n* write is thread-safe starting in Linux 3.14[3].\n\n* Linux 3.14 was released in 2014.\n\nAssuming that the remote node has a kernel no older than 10 years, we should not see any problems.  If we decide we do need to support an older kernel, we should\nadd a lock.\n\nOf course, it may produce interleaved output, but we\u0027re okay with that.  In fact,\nwe should be so lucky.  :)\n\n[1] https://docs.python.org/3/library/io.html#raw-i-o\n[2] https://docs.python.org/3/library/io.html#multi-threading\n[3] https://man7.org/linux/man-pages/man2/write.2.html#BUGS","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"18d5164d8c5af1fdd6b2e07d54850c4c7243605d","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        if not line[-1] !\u003d b\u0027\\n\u0027:"},{"line_number":323,"context_line":"            line +\u003d b\u0027\\n\u0027"},{"line_number":324,"context_line":"            newline_warning \u003d True"},{"line_number":325,"context_line":"        console.addLine(line)"},{"line_number":326,"context_line":"    if newline_warning:"},{"line_number":327,"context_line":"        console.addLine(\u0027[Zuul] No trailing newline\\n\u0027)"},{"line_number":328,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c334231b_ea677307","line":325,"in_reply_to":"b81ad8c9_8e87dbf1","updated":"2024-10-25 21:16:11.000000000","message":"\u003e This should be safe because (logical proof follows!):\n\u003e \n\u003e * We open the file with buffering\u003d0; this returns an io.FileIO object[1].\n\u003e \n\u003e * \"FileIO objects are thread-safe to the extent that the operating\n\u003e   system calls (such as read(2) under Unix) they wrap are thread-safe too. [2]\n\u003e \n\u003e * The OS call we are concerned with is \"write\".\n\u003e \n\u003e * write is thread-safe starting in Linux 3.14[3].\n\u003e \n\u003e * Linux 3.14 was released in 2014.\n\nThis all makes sense to me thank you for putting the proof together.\n\n\u003e \n\u003e Assuming that the remote node has a kernel no older than 10 years, we should not see any problems.  If we decide we do need to support an older kernel, we should\n\u003e add a lock.\n\nRHEL/CentOS/etc 7 is on a 3.10 kernel. I suspect that most everything else we may care about is newer than that. I think that Amazon ec2 linux may also have a RHEL 7 variant?\n\n\u003e \n\u003e Of course, it may produce interleaved output, but we\u0027re okay with that.  In fact,\n\u003e we should be so lucky.  :)\n\nYup interleaved is fine. I was only concerned about lost bytes due to overwriting at the wrong position.\n\n\u003e \n\u003e [1] https://docs.python.org/3/library/io.html#raw-i-o\n\u003e [2] https://docs.python.org/3/library/io.html#multi-threading\n\u003e [3] https://man7.org/linux/man-pages/man2/write.2.html#BUGS\n\nFrom an OpenDev perspective I don\u0027t think we care about CentOS 7/RHEL 7 any longer. The release went end of maintenance a few months ago so most people should be moving off of it at this point. I won\u0027t approve this change right now in case we feel we need more feedback from other users.","commit_id":"7c25cbcc7f2778892aba717debd54d0b1efa7f3a"}]}