)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":597,"name":"Pete Zaitcev","email":"zaitcev@kotori.zaitcev.us","username":"zaitcev"},"change_message_id":"fbcc85bd345581818353706dc8bf2edb7197cd53","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b6863b14_bd21a446","updated":"2022-07-28 17:50:16.000000000","message":"I have questions about these practices, but the code is obviously better than before.","commit_id":"85d063b20ac21207d7f23e48865f19656e83c087"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e0931cbb4aef353ae74b71ae756bf5d8358cd315","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"51077712_3fcb0948","updated":"2022-07-28 20:36:18.000000000","message":"i wonder if this change would seem more sensible in a larger context of the ring v2 work and where you think the ring module\u0027s logging should go","commit_id":"85d063b20ac21207d7f23e48865f19656e83c087"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dd75dbdd61992659b18d471854807535eb4d285a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e1547662_1f6c7ec3","in_reply_to":"51077712_3fcb0948","updated":"2022-07-29 21:02:12.000000000","message":"I see a bug, I write a patch. I don\u0027t actually care much about logging in rings/builders -- though I *do* want some ability to get a warning out. This was just some weirdness I noticed because you pointed me at some code :P","commit_id":"85d063b20ac21207d7f23e48865f19656e83c087"}],"swift/common/ring/builder.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e0931cbb4aef353ae74b71ae756bf5d8358cd315","unresolved":true,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        self.logger \u003d logging.getLogger(\"swift.ring.builder\")"},{"line_number":138,"context_line":"        if not self.logger.handlers:"},{"line_number":139,"context_line":"            self.logger.disabled \u003d True"},{"line_number":140,"context_line":"            # silence \"no handler for X\" error messages"},{"line_number":141,"context_line":"            self.logger.addHandler(logging.NullHandler())"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"310a25fe_ecfb58fb","line":139,"updated":"2022-07-28 20:36:18.000000000","message":"but.... this is like the ring builder\u0027s logger?  It apparently expects to have full control over the logger\u0027s enabled-ness.  I feel like we\u0027re trying to change the interface for some unspecified purpose - that will have almost no impact in practice.","commit_id":"85d063b20ac21207d7f23e48865f19656e83c087"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dd75dbdd61992659b18d471854807535eb4d285a","unresolved":true,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"        self.logger \u003d logging.getLogger(\"swift.ring.builder\")"},{"line_number":138,"context_line":"        if not self.logger.handlers:"},{"line_number":139,"context_line":"            self.logger.disabled \u003d True"},{"line_number":140,"context_line":"            # silence \"no handler for X\" error messages"},{"line_number":141,"context_line":"            self.logger.addHandler(logging.NullHandler())"},{"line_number":142,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"50652f87_2688f9f7","line":139,"in_reply_to":"310a25fe_ecfb58fb","updated":"2022-07-29 21:02:12.000000000","message":"Principle of least surprise -- the context manager should do its best to put things back the way they were. If you do what the CLI does and go to the trouble of saying\n\n logger \u003d logging.getLogger(\"swift.ring.builder\")\n logger.disabled \u003d False\n logger.setLevel(logging.DEBUG)\n logger.addHandler(handler)\n\nbefore eventually doing something like\n\n with rb.debug():\n     rb.rebalance()\n\nI can\u0027t avoid this feeling that it\u0027ll be surprising to discover that logger.disabled is now True.\n\nSide note: \"disabled\" isn\u0027t even a documented part of the Logger API! It doesn\u0027t appear *anywhere* on https://docs.python.org/3/library/logging.html\n\nI feel like we\u0027d be better off just doing something like\n\n if self.logger.level \u003d\u003d logging.NOTSET:\n     self.logger.setLevel(logging.INFO)\n\nhere and letting the CLI adjust the level down.","commit_id":"85d063b20ac21207d7f23e48865f19656e83c087"}]}