)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2958cb1c6274789b19eb01675ea5dcf209441f8d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5e489476_75f8acb9","updated":"2022-05-12 18:02:13.000000000","message":"It\u0027d probably be good to clean up the warnings about\n\n Got notification message from PID \u003cx\u003e, but reception only permitted for main PID \u003cy\u003e","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"6e28bc62662f0d6e4f2b42877acc914f957aee4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5eb57d4e_431a0cac","updated":"2022-04-14 04:23:23.000000000","message":"recheck","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a3e817bd19b1992462db2c085d28ff0481f76e43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9525b169_17f24a01","updated":"2022-05-18 22:09:02.000000000","message":"recheck\n\nTimeout looks like another eventlet logging deadlock bug. Seems to be fixed now.","commit_id":"069334d1dd4dd2d8c1029f4b2ab6347783c91918"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"e5fabb9542928c8f51a0f6015752379b190954b5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4a84d705_0522f095","updated":"2023-06-09 07:17:52.000000000","message":"Cool, I think I\u0027m grokking this. So this basically allows us to send back more systemd state machine notifications. The follow up patch will let swift-reload also take the same systemd notifications and use them to better monitor the children workers by listening to the mainpid and not having to keep polling children pids and making assumptions.\n\nI like it! Now I just need to test it some more in my VSAIO (this and the next patch). But great idea Tim!","commit_id":"2a5669c6babf434e00c145648a7d5bdf5a724eb8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"60f7b261c4ad8cc26a91a31e0f9c47e46ea8af4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"4aa0ddd3_734f44d2","updated":"2023-08-21 22:09:06.000000000","message":"recheck\n\nLooks like it may have been an error uploading job logs?","commit_id":"96e934e0b055f38479470bf6fa8b1c25822c17e6"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa65f39628334ded3b6f8274008e94d53dbf5f45","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"4cecdb8b_b4aa7b67","updated":"2023-11-03 18:59:30.000000000","message":"note to self: you\u0027re wrong sometimes","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"01502ee65cf6ddfcb772c154349f05e25ff59e3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e22b7a12_a1c184ca","updated":"2023-10-17 17:11:28.000000000","message":"recheck\n\nSome cinder failure.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"69fc9ba0_696319a8","updated":"2023-11-03 15:25:48.000000000","message":"so this is sort of a neat operational ergonomic improvement.  I don\u0027t have systemd setup on my vsaio - but in a prod like environment where we have this deployed I could catch the systemd notifications with `systemctl status \u003cservice\u003e`:\n\n● ssswift-account-replication@replicator.service - SwiftStack swift account replication replicator service\n   Loaded: loaded (/usr/lib/systemd/system/ssswift-account-replication@.service; enabled; vendor preset: disabled)\n   Active: active (running) since Fri 2023-11-03 14:04:25 UTC; 13s ago\n● ssswift-account-replication@replicator.service - SwiftStack swift account replication replicator service\n   Loaded: loaded (/usr/lib/systemd/system/ssswift-account-replication@.service; enabled; vendor preset: disabled)\n   Active: deactivating (stop) since Fri 2023-11-03 14:04:40 UTC; 7ms ago\n● ssswift-account-replication@replicator.service - SwiftStack swift account replication replicator service\n   Loaded: loaded (/usr/lib/systemd/system/ssswift-account-replication@.service; enabled; vendor preset: disabled)\n   Active: inactive (dead) since Fri 2023-11-03 14:04:40 UTC; 482ms ago\n\nfrom `while true; do systemctl status ssswift-account-replication@replicator.service | head -n 3; sleep 0.5; done`\n\nOne idea I had that might help with mainteance would be for the daemon and wsgi managment process to pull out NOTIFY_SOCKET form the environ early and hold onto it so THEY can re-use it but no children could accidently signal systemd\n\nhttps://review.opendev.org/c/openstack/swift/+/900054\n\n... seems like that\u0027s easier than \"always remembering\" to clean up in fork","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"swift/common/daemon.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2958cb1c6274789b19eb01675ea5dcf209441f8d","unresolved":true,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _fork(self, once, **kwargs):"},{"line_number":166,"context_line":"        pid \u003d os.fork()"},{"line_number":167,"context_line":"        if pid \u003d\u003d 0:"},{"line_number":168,"context_line":"            signal.signal(signal.SIGHUP, signal.SIG_DFL)"},{"line_number":169,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"4f31bfef_d19f27d8","line":167,"updated":"2022-05-12 18:02:13.000000000","message":"We should pop the NOTIFY_SOCKET env var here now that we\u0027re not doing it in systemd_notify().","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8f8eb904754b9c53a51d6dbc2a2ef30e0665f62c","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _fork(self, once, **kwargs):"},{"line_number":166,"context_line":"        pid \u003d os.fork()"},{"line_number":167,"context_line":"        if pid \u003d\u003d 0:"},{"line_number":168,"context_line":"            signal.signal(signal.SIGHUP, signal.SIG_DFL)"},{"line_number":169,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":170,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"405ceeab_f629d40d","line":167,"in_reply_to":"4f31bfef_d19f27d8","updated":"2023-02-16 23:01:12.000000000","message":"Done","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"136ea8721f96b63d6c7a6c146379aef84af111a9","unresolved":true,"context_lines":[{"line_number":168,"context_line":"            signal.signal(signal.SIGHUP, signal.SIG_DFL)"},{"line_number":169,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":170,"context_line":"            # only MAINPID should be sending notifications"},{"line_number":171,"context_line":"            os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            self.daemon.run(once, **kwargs)"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c559af89_b95a8f4b","line":171,"updated":"2022-07-06 05:41:05.000000000","message":"Oh i see, by popping it here. The socket wont be in the forked child env anymore so calls to systemd_notify() are basically noops unless it\u0027s the management/main PID.","commit_id":"069334d1dd4dd2d8c1029f4b2ab6347783c91918"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8f8eb904754b9c53a51d6dbc2a2ef30e0665f62c","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            signal.signal(signal.SIGHUP, signal.SIG_DFL)"},{"line_number":169,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":170,"context_line":"            # only MAINPID should be sending notifications"},{"line_number":171,"context_line":"            os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            self.daemon.run(once, **kwargs)"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"51af3183_265deb08","line":171,"in_reply_to":"c559af89_b95a8f4b","updated":"2023-02-16 23:01:12.000000000","message":"Ack","commit_id":"069334d1dd4dd2d8c1029f4b2ab6347783c91918"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        :returns: an iterable of dicts, each element represents the kwargs to"},{"line_number":93,"context_line":"                  be passed to a single worker\u0027s :meth:`run` method after fork."},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":"        return []"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def is_healthy(self):"},{"line_number":98,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"5e7ef623_8032b370","line":95,"updated":"2023-11-03 15:25:48.000000000","message":"right so all swift daemons inhert from this class, but MOST do not return worker args so our strategy\u0027s _run will _run_inline; but the finally block in run will still want to cleanup; so ... if any daemons run methods do forking they\u0027ll want to manually remember to sanitize the SYSTEMD_NOTIFIY from the children\u0027s environment.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d4293d4e2b261570bd73a8f96d6d1bfbb6acee18","unresolved":true,"context_lines":[{"line_number":92,"context_line":"        :returns: an iterable of dicts, each element represents the kwargs to"},{"line_number":93,"context_line":"                  be passed to a single worker\u0027s :meth:`run` method after fork."},{"line_number":94,"context_line":"        \"\"\""},{"line_number":95,"context_line":"        return []"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    def is_healthy(self):"},{"line_number":98,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"82a238b6_9943f432","line":95,"in_reply_to":"5e7ef623_8032b370","updated":"2023-11-03 19:10:03.000000000","message":"\u003e all swift daemons inhert from this class, but MOST do not return worker args\n\nThis really kinda bugs me 😞 We have this infra, but just about every daemon does multi-process a little differently.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":168,"context_line":"            signal.signal(signal.SIGHUP, signal.SIG_DFL)"},{"line_number":169,"context_line":"            signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":170,"context_line":"            # only MAINPID should be sending notifications"},{"line_number":171,"context_line":"            os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"            self.daemon.run(once, **kwargs)"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"df12ac28_3d20daca","line":171,"updated":"2023-11-03 15:25:48.000000000","message":"again, good hygine.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"01502ee65cf6ddfcb772c154349f05e25ff59e3e","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        if not self.daemon.is_healthy():"},{"line_number":206,"context_line":"            self.logger.debug("},{"line_number":207,"context_line":"                \u0027Daemon needs to change options, aborting workers\u0027)"},{"line_number":208,"context_line":"            self.cleanup()"},{"line_number":209,"context_line":"            return True"},{"line_number":210,"context_line":"        return False"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"c6421f2a_db393389","line":208,"updated":"2023-10-17 17:11:28.000000000","message":"This is the other `cleanup()` call, the one where we don\u0027t send the `STOPPING\u003d1` notification. *Maybe* we should? \\*shrug\\*\n\nOh! Or send `RELOADING\u003d1` ...","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":255,"context_line":"        \"\"\""},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        if stopping:"},{"line_number":258,"context_line":"            utils.systemd_notify(self.logger, \"STOPPING\u003d1\")"},{"line_number":259,"context_line":"        for p in self.spawned_pids():"},{"line_number":260,"context_line":"            try:"},{"line_number":261,"context_line":"                os.kill(p, signal.SIGTERM)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f49cf1e_800e3c98","line":258,"updated":"2023-11-03 15:25:48.000000000","message":"This is in the DaemonStrategy class, the other call to systemd_notify(READY\u003d1) is in DaemonStrategy.setup","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":6227,"context_line":"            try:"},{"line_number":6228,"context_line":"                sock.connect(notify_socket)"},{"line_number":6229,"context_line":"                sock.sendall(msg)"},{"line_number":6230,"context_line":"                del os.environ[\u0027NOTIFY_SOCKET\u0027]"},{"line_number":6231,"context_line":"            except EnvironmentError:"},{"line_number":6232,"context_line":"                if logger:"},{"line_number":6233,"context_line":"                    logger.debug(\"Systemd notification failed\", exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"37b7accc_65866f7d","side":"PARENT","line":6230,"updated":"2023-11-03 15:25:48.000000000","message":"this is significant; but probably overly cautious of the original and managing process environment is reasonably not the domain of this function.\n\nalthough maye there\u0027s some prior art for notify helper methods unsetting this var and future calls failing:\n\nIf the unset_environment parameter is non-zero, sd_notify() will unset the $NOTIFY_SOCKET environment variable before returning (regardless of whether the function call itself succeeded or not). Further calls to sd_notify() will then fail, but the variable is no longer inherited by child processes.\n\nhttps://www.freedesktop.org/software/systemd/man/latest/sd_notify.html","commit_id":"212525118c520606ba94895c522ba21ed0373d2f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","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\"):"},{"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":12,"id":"e771f8fa_1a166abf","line":6209,"updated":"2023-11-03 15:25:48.000000000","message":"msg as a kwarg is reasonable with a default; but given the generic name of this method (depending on how much we\u0027re going to use it) it might be reasonable to update existing callers to explicity say when they\u0027re specifically notifying systemd they are READY\u003d1 by always sending the msg argument.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":6208,"context_line":""},{"line_number":6209,"context_line":"def systemd_notify(logger\u003dNone, msg\u003db\"READY\u003d1\"):"},{"line_number":6210,"context_line":"    \"\"\""},{"line_number":6211,"context_line":"    Send systemd-compatible notifications."},{"line_number":6212,"context_line":""},{"line_number":6213,"context_line":"    Notify the service manager that started this process, if it has set the"},{"line_number":6214,"context_line":"    NOTIFY_SOCKET environment variable. For example, systemd will set this"}],"source_content_type":"text/x-python","patch_set":12,"id":"bf023e38_6d3dead6","line":6211,"updated":"2023-11-03 15:25:48.000000000","message":", if NOTIFY_SOCKET is availabe.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"swift/common/wsgi.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"2958cb1c6274789b19eb01675ea5dcf209441f8d","unresolved":true,"context_lines":[{"line_number":1102,"context_line":"        for sock, sock_info in strategy.new_worker_socks():"},{"line_number":1103,"context_line":"            read_fd, write_fd \u003d os.pipe()"},{"line_number":1104,"context_line":"            pid \u003d os.fork()"},{"line_number":1105,"context_line":"            if pid \u003d\u003d 0:"},{"line_number":1106,"context_line":"                os.close(read_fd)"},{"line_number":1107,"context_line":"                signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":1108,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3b6f94cd_075786c2","line":1105,"updated":"2022-05-12 18:02:13.000000000","message":"Maybe here, too?","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8f8eb904754b9c53a51d6dbc2a2ef30e0665f62c","unresolved":false,"context_lines":[{"line_number":1102,"context_line":"        for sock, sock_info in strategy.new_worker_socks():"},{"line_number":1103,"context_line":"            read_fd, write_fd \u003d os.pipe()"},{"line_number":1104,"context_line":"            pid \u003d os.fork()"},{"line_number":1105,"context_line":"            if pid \u003d\u003d 0:"},{"line_number":1106,"context_line":"                os.close(read_fd)"},{"line_number":1107,"context_line":"                signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":1108,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fb73e1b1_91dd1b6d","line":1105,"in_reply_to":"3b6f94cd_075786c2","updated":"2023-02-16 23:01:12.000000000","message":"Done in post_fork_hook, which seemed like a better spot.","commit_id":"bfc4adb6b98699d670d857c18206110e646173a5"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":488,"context_line":"        drop_privileges(self.conf.get(\u0027user\u0027, \u0027swift\u0027))"},{"line_number":489,"context_line":"        del self.tracking_data  # children don\u0027t need to track siblings"},{"line_number":490,"context_line":"        # only MAINPID should be sending systemd notifications"},{"line_number":491,"context_line":"        os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"    def shutdown_sockets(self):"},{"line_number":494,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":12,"id":"4a69e813_e9dcf97d","line":491,"updated":"2023-11-03 15:25:48.000000000","message":"this seems like good hygine","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":896,"context_line":"    def stop_with_signal(signum, *args):"},{"line_number":897,"context_line":"        \"\"\"Set running flag to False and capture the signum\"\"\""},{"line_number":898,"context_line":"        running_context[0] \u003d False"},{"line_number":899,"context_line":"        running_context[1] \u003d signum"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    # context to hold boolean running state and stop signum"},{"line_number":902,"context_line":"    running_context \u003d [True, None]"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f259189_4f56be3f","line":899,"updated":"2023-11-03 15:25:48.000000000","message":"i like it that this method just modifies state and clearly always sets [0] \u003d False","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d4293d4e2b261570bd73a8f96d6d1bfbb6acee18","unresolved":true,"context_lines":[{"line_number":896,"context_line":"    def stop_with_signal(signum, *args):"},{"line_number":897,"context_line":"        \"\"\"Set running flag to False and capture the signum\"\"\""},{"line_number":898,"context_line":"        running_context[0] \u003d False"},{"line_number":899,"context_line":"        running_context[1] \u003d signum"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    # context to hold boolean running state and stop signum"},{"line_number":902,"context_line":"    running_context \u003d [True, None]"}],"source_content_type":"text/x-python","patch_set":12,"id":"aa115c51_d8f48961","line":899,"in_reply_to":"5f259189_4f56be3f","updated":"2023-11-03 19:10:03.000000000","message":"Yeah, bad/weird things happen if you try to do too much in a signal handler. I know that\u0027s true in C, fairly certain it\u0027s true in Python, too. https://review.opendev.org/c/openstack/swift/+/569837 was probably always a bad idea.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":943,"context_line":""},{"line_number":944,"context_line":"        # TODO: signal_ready() as soon as we have at least one new worker for"},{"line_number":945,"context_line":"        # each port, instead of waiting for all of them"},{"line_number":946,"context_line":"        strategy.signal_ready()"},{"line_number":947,"context_line":""},{"line_number":948,"context_line":"        # The strategy may need to pay attention to something in addition to"},{"line_number":949,"context_line":"        # child process exits (like new ports showing up in a ring)."}],"source_content_type":"text/x-python","patch_set":12,"id":"53922b95_5945768c","line":946,"updated":"2023-11-03 15:25:48.000000000","message":"just because we CAN `systemd_notify` from anywhere in the process doesn\u0027t mean we *should* - here we at least keep it confined to the wsgi/deamon *modules* but ultimately we may decide that the responsibility belongs to either this main loop function or the strategy class.\n\nI wouldn\u0027t be opposed to moving the systemd_notify call out of the strategy signal-ready method (which seems to be about messaging the reload process to shutdown) into here, where we\u0027re doing the REST of the signalling to systemd.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d4293d4e2b261570bd73a8f96d6d1bfbb6acee18","unresolved":true,"context_lines":[{"line_number":943,"context_line":""},{"line_number":944,"context_line":"        # TODO: signal_ready() as soon as we have at least one new worker for"},{"line_number":945,"context_line":"        # each port, instead of waiting for all of them"},{"line_number":946,"context_line":"        strategy.signal_ready()"},{"line_number":947,"context_line":""},{"line_number":948,"context_line":"        # The strategy may need to pay attention to something in addition to"},{"line_number":949,"context_line":"        # child process exits (like new ports showing up in a ring)."}],"source_content_type":"text/x-python","patch_set":12,"id":"e4f10c88_c94dc399","line":946,"in_reply_to":"53922b95_5945768c","updated":"2023-11-03 19:10:03.000000000","message":"I\u0027m torn about it -- I get what you\u0027re saying, but I also kinda like the idea that there is *exactly one* notion of \"ready\".","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":983,"context_line":"                         running_context[1])"},{"line_number":984,"context_line":"        else:"},{"line_number":985,"context_line":"            logger.notice(\u0027%s received (%s)\u0027, signame, os.getpid())"},{"line_number":986,"context_line":"    if running_context[1] \u003d\u003d signal.SIGTERM:"},{"line_number":987,"context_line":"        systemd_notify(logger, \"STOPPING\u003d1\")"},{"line_number":988,"context_line":"        os.killpg(0, signal.SIGTERM)"},{"line_number":989,"context_line":"    elif running_context[1] \u003d\u003d signal.SIGUSR1:"}],"source_content_type":"text/x-python","patch_set":12,"id":"08f8cd99_bba88178","line":986,"updated":"2023-11-03 15:25:48.000000000","message":"running_context is not \"just\"  a nonlocal, the second item was the signal captured in the signal handler.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"swift/container/updater.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":158,"context_line":"                    pid2filename[pid] \u003d tmpfilename"},{"line_number":159,"context_line":"                else:"},{"line_number":160,"context_line":"                    signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":161,"context_line":"                    os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":162,"context_line":"                    eventlet_monkey_patch()"},{"line_number":163,"context_line":"                    self.no_changes \u003d 0"},{"line_number":164,"context_line":"                    self.successes \u003d 0"}],"source_content_type":"text/x-python","patch_set":12,"id":"abea24d6_8559a56e","line":161,"updated":"2023-11-03 15:25:48.000000000","message":"oh interesting - we have so many forking strategies","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"swift/obj/updater.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":381,"context_line":"                    pids.append(pid)"},{"line_number":382,"context_line":"                else:"},{"line_number":383,"context_line":"                    signal.signal(signal.SIGTERM, signal.SIG_DFL)"},{"line_number":384,"context_line":"                    os.environ.pop(\u0027NOTIFY_SOCKET\u0027, None)"},{"line_number":385,"context_line":"                    eventlet_monkey_patch()"},{"line_number":386,"context_line":"                    self.stats.reset()"},{"line_number":387,"context_line":"                    forkbegin \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":12,"id":"16fa9fe9_b242cb0d","line":384,"updated":"2023-11-03 15:25:48.000000000","message":"i\u0027m assuming Tim grepped for fork in the code base - I didn\u0027t.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":4061,"context_line":"            utils.systemd_notify(logger, \"RELOADING\u003d1\")"},{"line_number":4062,"context_line":"            m_socket.assert_called_once_with(socket.AF_UNIX, socket.SOCK_DGRAM)"},{"line_number":4063,"context_line":"            m_sock.connect.assert_called_once_with(\u0027foobar\u0027)"},{"line_number":4064,"context_line":"            m_sock.sendall.assert_called_once_with(b\u0027RELOADING\u003d1\u0027)"},{"line_number":4065,"context_line":""},{"line_number":4066,"context_line":"            # Abstract notification socket"},{"line_number":4067,"context_line":"            m_socket.reset_mock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"8751f2f7_d2997edf","line":4064,"updated":"2023-11-03 15:25:48.000000000","message":"and maybe close too\n\n\tdiff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py\n\tindex 2de0daaf7..df3809fe1 100644\n\t--- a/test/unit/common/test_utils.py\n\t+++ b/test/unit/common/test_utils.py\n\t@@ -4062,6 +4062,7 @@ cluster_dfw1 \u003d http://dfw1.host/v1/\n\t\t     m_socket.assert_called_once_with(socket.AF_UNIX, socket.SOCK_DGRAM)\n\t\t     m_sock.connect.assert_called_once_with(\u0027foobar\u0027)\n\t\t     m_sock.sendall.assert_called_once_with(b\u0027RELOADING\u003d1\u0027)\n\t+            m_sock.close.assert_called_once()\n\t \n\t\t     # Abstract notification socket\n\t\t     m_socket.reset_mock()","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}],"test/unit/common/test_wsgi.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"9a97dc8ef57f93d6a1ea93c58428a05f4d997a96","unresolved":true,"context_lines":[{"line_number":990,"context_line":"                                                           thread\u003dTrue)"},{"line_number":991,"context_line":"        self.assertEqual(mock_notify.mock_calls, ["},{"line_number":992,"context_line":"            mock.call(\u0027logger\u0027, \"STOPPING\u003d1\"),"},{"line_number":993,"context_line":"        ])"},{"line_number":994,"context_line":""},{"line_number":995,"context_line":"    def test_run_server_success(self):"},{"line_number":996,"context_line":"        calls \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":12,"id":"31d268fc_70ae2efd","line":993,"updated":"2023-11-03 15:25:48.000000000","message":"I don\u0027t think any of the other functions where we mock and then assert calls are OUR interfaces - they\u0027re system/library functions where we don\u0027t maintain the signature/code.\n\nFor the tests you\u0027re adding (or in this case extending) to ensure mainteance of this new behavior I think it would be better to actually set NOTIFY_SOCKET and mock socket if that\u0027s the context we expect and the behavior we want.\n\nBut that could be personaly experience; you may have had a lot of success building working software with thin testing of it layer by layer - some early TDD proponents advocated that way and I\u0027d previously tried the \"fake it till you make it\" pattern - but ultimately what I found were a bunch of bugs where the mocks worked and the real code didn\u0027t.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fa65f39628334ded3b6f8274008e94d53dbf5f45","unresolved":false,"context_lines":[{"line_number":990,"context_line":"                                                           thread\u003dTrue)"},{"line_number":991,"context_line":"        self.assertEqual(mock_notify.mock_calls, ["},{"line_number":992,"context_line":"            mock.call(\u0027logger\u0027, \"STOPPING\u003d1\"),"},{"line_number":993,"context_line":"        ])"},{"line_number":994,"context_line":""},{"line_number":995,"context_line":"    def test_run_server_success(self):"},{"line_number":996,"context_line":"        calls \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":12,"id":"002c4178_05274761","line":993,"in_reply_to":"31d268fc_70ae2efd","updated":"2023-11-03 18:59:30.000000000","message":"this was WAY wrong - like ALL of those interfaces we\u0027re mocking out our own interfaces - the only difference it was we weren\u0027t really asserting how they were called; we were just stubbig out huge parts of the code to get to the part we cared about.  It\u0027s super hard to say what\u0027s most correct here - this is a difficult module to test.","commit_id":"55f7833d86502a2c6c4d526df8883c25719b1c45"}]}
