)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1bb344ff2c32f89d0dfba6ff4d8ed6d7aaa32e58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cd620013_b31659d3","updated":"2023-11-03 17:43:42.000000000","message":"I could definately improve this.. just not sure if it\u0027s a good idea.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"66b035310902c31fc3fc251770d83e98730d8cc2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b2a4dcdb_79b0a7ce","updated":"2023-11-03 20:01:03.000000000","message":"I\u0027m cool with this direction -- pretty sure it\u0027ll conflict with https://review.opendev.org/c/openstack/swift/+/837641 tho","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1df8359c668223e258bde827aee6f4769342391c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"27d291e5_6509ae28","updated":"2023-11-03 15:33:02.000000000","message":"not sure if this is worth pushing on, if there\u0027s intrest I could try to flesh it out.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d26847923fa378b17f1b05e1309b299697cea940","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c0946767_78161334","updated":"2023-11-03 17:36:22.000000000","message":"probe test failures look legit:\n\n\t    def wait_for_server_to_hangup(ipport):\n\t\ttry_until \u003d time() + 30\n\t\twhile True:\n\t\t    try:\n\t\t\tconn \u003d HTTPConnection(*ipport)\n\t\t\tconn.request(\u0027GET\u0027, \u0027/\u0027)\n\t\t\tconn.getresponse()\n\t\t    except Exception:\n\t\t\tbreak\n\t\t    if time() \u003e try_until:\n\t\t\traise Exception(\n\t\u003e                   \u0027Still answering on %s:%s after 30 seconds\u0027 % ipport)\n\tE               Exception: Still answering on 127.0.0.4:6242 after 30 seconds\n\nI definately don\u0027t understad this stuff well enough; maybe I need to write some better unittests.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"005ac15332b15385a5e0098d941c4b25f17b043b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0ec0d5fb_9a8f663a","updated":"2023-11-13 03:41:01.000000000","message":"Yeah, this is looking pretty good. Pulling out early means there is no chance for a child to use the socket.\n\nYour new test fails for py2, tested locally and is fialing on mine. In situations like this I wonder if we\u0027re going to remove py2 support soon, if we can be lazy and `if six.PY2: self.skip`","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8e85162482ab8e014f2fbd63bd81b0ef28503a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9ec00b60_b3d12446","updated":"2023-11-03 19:52:39.000000000","message":"idk, I like the idea of cleaning the environ early - I don\u0027t love how hard it is to test all of this and i\u0027m not 100% this would make it only easier in the future; but it\u0027s something that works and would have blown up on the NameErrors.","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"}],"swift/common/daemon.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1df8359c668223e258bde827aee6f4769342391c","unresolved":true,"context_lines":[{"line_number":262,"context_line":"        if stopping:"},{"line_number":263,"context_line":"            utils.systemd_notify("},{"line_number":264,"context_line":"                self.logger, \"STOPPING\u003d1\","},{"line_number":265,"context_line":"                notify_socket_fd\u003dself.systemd_notify_socket_fd)"},{"line_number":266,"context_line":"        for p in self.spawned_pids():"},{"line_number":267,"context_line":"            try:"},{"line_number":268,"context_line":"                os.kill(p, signal.SIGTERM)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e247665e_74f5d9bf","line":265,"updated":"2023-11-03 15:33:02.000000000","message":"i\u0027m not sure it\u0027s good or bad the daemon tests didn\u0027t break; I should add asserts about the environ being cleaned up","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"66b035310902c31fc3fc251770d83e98730d8cc2","unresolved":true,"context_lines":[{"line_number":6206,"context_line":"    return sorted(results)"},{"line_number":6207,"context_line":""},{"line_number":6208,"context_line":""},{"line_number":6209,"context_line":"def systemd_notify(logger\u003dNone, msg\u003db\"READY\u003d1\", notify_socket_fd\u003dNone):"},{"line_number":6210,"context_line":"    \"\"\""},{"line_number":6211,"context_line":"    Send systemd-compatible notifications."},{"line_number":6212,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a8718326_b0c37fcb","line":6209,"range":{"start_line":6209,"start_character":48,"end_line":6209,"end_character":69},"updated":"2023-11-03 20:01:03.000000000","message":"There are no external callers -- we can just make this required. May as well do the same for `msg` if we\u0027re plumbing it everywhere anyway.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"}],"swift/common/wsgi.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"66b035310902c31fc3fc251770d83e98730d8cc2","unresolved":true,"context_lines":[{"line_number":880,"context_line":"    # server without forking."},{"line_number":881,"context_line":"    clean_up_daemon_hygiene()"},{"line_number":882,"context_line":"    # we clean this out here so children can\u0027t accidently notify systemd"},{"line_number":883,"context_line":"    # and we have to start thinking about \u0027sd_notify_barrier\u0027"},{"line_number":884,"context_line":"    systemd_notify_socket_fd \u003d os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":885,"context_line":""},{"line_number":886,"context_line":"    def signal_ready():"}],"source_content_type":"text/x-python","patch_set":1,"id":"9b0fc564_e08a9795","line":883,"updated":"2023-11-03 20:01:03.000000000","message":"IME systemd will ignore messages from the children with something like\n```\nsystemd[1]: swift-object-updater.service: Got notification message from PID 3716963, but reception only permitted for main PID 3716537\n```\nbut I suppose that\u0027s because I just take the default behavior for `NotifyAccess` -- I don\u0027t really understand when you would want `NotifyAccess\u003dall` or what you would do with it.\n\nFWIW, my insistence on cleaning up the child environments has always been about avoiding that warning, and even then it\u0027s mainly so I can (try to) keep `journalctl -p warning` useful.\n\nThat it also allows me to remain blissfully unaware of `sd_notify_barrier` is a happy coincidence.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1bb344ff2c32f89d0dfba6ff4d8ed6d7aaa32e58","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"                pass"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        # SIGHUP or, less likely, run in \"once\" mode"},{"line_number":1059,"context_line":"        systemd_notify(logger\u003dself.logger, msg\u003db\"STOPPING\u003d1\","},{"line_number":1060,"context_line":"                       notify_socket_fd\u003dsystemd_notify_socket_fd)"},{"line_number":1061,"context_line":""},{"line_number":1062,"context_line":"    strategy.shutdown_sockets()"}],"source_content_type":"text/x-python","patch_set":1,"id":"837ce7cf_5ff0fbf9","line":1059,"in_reply_to":"4b3df12c_94057528","updated":"2023-11-03 17:43:42.000000000","message":"oh wow, yeah test_wsgi still passes with this name error!","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"66b035310902c31fc3fc251770d83e98730d8cc2","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"                pass"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        # SIGHUP or, less likely, run in \"once\" mode"},{"line_number":1059,"context_line":"        systemd_notify(logger\u003dself.logger, msg\u003db\"STOPPING\u003d1\","},{"line_number":1060,"context_line":"                       notify_socket_fd\u003dsystemd_notify_socket_fd)"},{"line_number":1061,"context_line":""},{"line_number":1062,"context_line":"    strategy.shutdown_sockets()"}],"source_content_type":"text/x-python","patch_set":1,"id":"f2eaaa57_aa6a7204","line":1059,"in_reply_to":"837ce7cf_5ff0fbf9","updated":"2023-11-03 20:01:03.000000000","message":"Yeah, it\u0027s terrifying trying to unit test any function with `fork`s in it. When you mess it up, things can go *so* sideways...","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"}],"test/unit/common/test_wsgi.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1df8359c668223e258bde827aee6f4769342391c","unresolved":true,"context_lines":[{"line_number":1007,"context_line":"            # both connects use the same fd"},{"line_number":1008,"context_line":"            mock.call(\u002755\u0027), mock.call(\u002755\u0027)])"},{"line_number":1009,"context_line":"        self.assertEqual(m_sock.sendall.call_args_list, ["},{"line_number":1010,"context_line":"            mock.call(b\u0027READY\u003d1\u0027), mock.call(b\u0027STOPPING\u003d1\u0027)])"},{"line_number":1011,"context_line":""},{"line_number":1012,"context_line":"    def test_run_server_success(self):"},{"line_number":1013,"context_line":"        calls \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6737a2fc_36ce013d","line":1010,"updated":"2023-11-03 15:33:02.000000000","message":"some of these asserts duplicate with testing of systemd_notify; but for me it feels like a better compromise than just asserting I called systemd_notify with the arguments I called it with.\n\nI could imagine reasonable experienced developers to have some differences in taste/style and also some appreciation for that vairety.","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"1df8359c668223e258bde827aee6f4769342391c","unresolved":true,"context_lines":[{"line_number":1046,"context_line":"        self.assertEqual(mock_notify.mock_calls, ["},{"line_number":1047,"context_line":"            mock.call(logger\u003d\u0027logger\u0027, msg\u003db\"STOPPING\u003d1\","},{"line_number":1048,"context_line":"                      notify_socket_fd\u003dNone),"},{"line_number":1049,"context_line":"        ])"},{"line_number":1050,"context_line":"        # run_wsgi() no longer calls drop_privileges() in the parent process,"},{"line_number":1051,"context_line":"        # just clean_up_daemon_hygene()"},{"line_number":1052,"context_line":"        self.assertEqual([], _d_privs.mock_calls)"}],"source_content_type":"text/x-python","patch_set":1,"id":"808a69f5_6c332f38","line":1049,"updated":"2023-11-03 15:33:02.000000000","message":"this comment isn\u0027t that helpful; maybe we should pick one style or the other - maybe this doesn\u0027t add much over `test_run_server_global_conf_callback`\n\n... probbalby a dedicated \"test_run_wsgi_signals_systemd\" would be ideal; we could even assert what happens when NOTIFY_SOCKET isn\u0027t set; or if it\u0027s a magic abstracti socket:\n\n\u003e If the first character of $NOTIFY_SOCKET is \"/\" or \"@\", the string is understood as an AF_UNIX or Linux abstract namespace socket","commit_id":"6a7c06f62985af2ef98e77789a5254a99c5c1ee4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8e85162482ab8e014f2fbd63bd81b0ef28503a8","unresolved":true,"context_lines":[{"line_number":2465,"context_line":"            try:"},{"line_number":2466,"context_line":"                return child_pids.pop(0)"},{"line_number":2467,"context_line":"            except IndexError:"},{"line_number":2468,"context_line":"                # this is the SIGUSR1 case!"},{"line_number":2469,"context_line":"                os.write(self.write_fd, b\u00275678\u0027)"},{"line_number":2470,"context_line":"                # we\u0027ll return into the new child"},{"line_number":2471,"context_line":"                return 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"aaf29f0c_b3775dda","line":2468,"updated":"2023-11-03 19:52:39.000000000","message":"it\u0027s not exactly clear what I\u0027m testing here; I don\u0027t really understand how the reload process reexec management works","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2188b223cbad9491188ead9a57921a1554c09cd2","unresolved":true,"context_lines":[{"line_number":2465,"context_line":"            try:"},{"line_number":2466,"context_line":"                return child_pids.pop(0)"},{"line_number":2467,"context_line":"            except IndexError:"},{"line_number":2468,"context_line":"                # this is the SIGUSR1 case!"},{"line_number":2469,"context_line":"                os.write(self.write_fd, b\u00275678\u0027)"},{"line_number":2470,"context_line":"                # we\u0027ll return into the new child"},{"line_number":2471,"context_line":"                return 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"ec7b3f0a_7fc7a8e8","line":2468,"in_reply_to":"aaf29f0c_b3775dda","updated":"2023-11-13 19:54:29.000000000","message":"You\u0027re not exactly instilling me with confidence about these tests 😄","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8e85162482ab8e014f2fbd63bd81b0ef28503a8","unresolved":true,"context_lines":[{"line_number":2467,"context_line":"            except IndexError:"},{"line_number":2468,"context_line":"                # this is the SIGUSR1 case!"},{"line_number":2469,"context_line":"                os.write(self.write_fd, b\u00275678\u0027)"},{"line_number":2470,"context_line":"                # we\u0027ll return into the new child"},{"line_number":2471,"context_line":"                return 0"},{"line_number":2472,"context_line":""},{"line_number":2473,"context_line":"        def m_capture_stdio(logger):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7fbcfff6_6113d8b2","line":2470,"updated":"2023-11-03 19:52:39.000000000","message":"and where does the loop go after this?  is the plan for this child process to shutdown?\n\nI guess the reason we do it this way is so that the process that lives on has the same pid as the original parent, mabye this makes systemd less unhappy.","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2188b223cbad9491188ead9a57921a1554c09cd2","unresolved":true,"context_lines":[{"line_number":2467,"context_line":"            except IndexError:"},{"line_number":2468,"context_line":"                # this is the SIGUSR1 case!"},{"line_number":2469,"context_line":"                os.write(self.write_fd, b\u00275678\u0027)"},{"line_number":2470,"context_line":"                # we\u0027ll return into the new child"},{"line_number":2471,"context_line":"                return 0"},{"line_number":2472,"context_line":""},{"line_number":2473,"context_line":"        def m_capture_stdio(logger):"}],"source_content_type":"text/x-python","patch_set":2,"id":"24af0d78_3088bd90","line":2470,"in_reply_to":"7fbcfff6_6113d8b2","updated":"2023-11-13 19:54:29.000000000","message":"\u003e where does the loop go after this? is the plan for this child process to shutdown?\n\nOK, I think I\u0027ve managed to follow along -- we\u0027re returning as the socket-closer child (so we don\u0027t need to go patching out `execv`) where we\u0027ll read the notification from the old manager, shutdown listen sockets, and exit. So, yes, assuming that\u0027s what you meant by shutdown.\n\n\u003e the process that lives on has the same pid as the original parent, mabye this makes systemd less unhappy.\n\nYes, this was an explicit goal of https://github.com/openstack/swift/commit/1107f241 -- if we were to, say, fork off a new manager (which in turn forks off its own workers) then close our listen sockets and exit, systemd sees that the process died and tries to start it up again.","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2188b223cbad9491188ead9a57921a1554c09cd2","unresolved":true,"context_lines":[{"line_number":2495,"context_line":"                6010, 6011}"},{"line_number":2496,"context_line":"            try:"},{"line_number":2497,"context_line":"                with eventlet.Timeout(3):"},{"line_number":2498,"context_line":"                    wsgi.run_wsgi(\u0027conf_file\u0027, \u0027object-server\u0027)"},{"line_number":2499,"context_line":"            except eventlet.Timeout:"},{"line_number":2500,"context_line":"                self.fail(\u0027test did not exit run_wgi!\u0027)"},{"line_number":2501,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df89cd3c_cb28d2fa","line":2498,"updated":"2023-11-13 19:54:29.000000000","message":"FWIW, the py2 failure pops here like\n```\nhome/zuul/src/opendev.org/openstack/swift/test/unit/common/test_wsgi.py:2498: in _test_server_per_socket_shutdown_with_signal\n    wsgi.run_wsgi(\u0027conf_file\u0027, \u0027object-server\u0027)\nhome/zuul/src/opendev.org/openstack/swift/swift/common/wsgi.py:1014: in run_wsgi\n    strategy.set_close_on_exec_on_listen_sockets()\n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n\nself \u003d \u003cswift.common.wsgi.ServersPerPortStrategy object at 0x7f5c88450410\u003e\n\n    def set_close_on_exec_on_listen_sockets(self):\n        \"\"\"\n        Set the close-on-exec flag on any listen sockets.\n        \"\"\"\n    \n        for sock in self.iter_sockets():\n            if six.PY2:\n\u003e               fcntl.fcntl(sock.fileno(), fcntl.F_SETFD, fcntl.FD_CLOEXEC)\nE               TypeError: fileno() returned a non-integer\n```","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8e85162482ab8e014f2fbd63bd81b0ef28503a8","unresolved":true,"context_lines":[{"line_number":2522,"context_line":"            mock.call(m_socket.AF_UNIX, m_socket.SOCK_DGRAM),"},{"line_number":2523,"context_line":"            mock.call(m_socket.AF_UNIX, m_socket.SOCK_DGRAM),"},{"line_number":2524,"context_line":"        ])"},{"line_number":2525,"context_line":"        self.m_sock \u003d m_sock \u003d m_socket.socket.return_value"},{"line_number":2526,"context_line":"        self.assertEqual(m_sock.connect.call_args_list, ["},{"line_number":2527,"context_line":"            # both connects use the same fd"},{"line_number":2528,"context_line":"            mock.call(\u002711\u0027), mock.call(\u002711\u0027)])"}],"source_content_type":"text/x-python","patch_set":2,"id":"1e8f7c60_322885b5","line":2525,"updated":"2023-11-03 19:52:39.000000000","message":"some of this alias stuff is just me being lazy; I could clean up - use some patchers in the setup.","commit_id":"e83cc0faf1e67e74fbde1a3546f3f60ac0742806"}]}