)]}'
{"foo.log":[{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"a6a096ddd1c65299e42185a4cf714aaf6ebdd8a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bab6814e_03f06090","updated":"2016-05-25 21:34:59.000000000","message":"Did you mean to check this file in?","commit_id":"7655982dfc5af887fd83ba30be374224b32bd371"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"987baeb875a837172e6afe86ce1d7586150f3897","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bab6814e_9c3778fe","in_reply_to":"bab6814e_03f06090","updated":"2016-05-26 09:29:31.000000000","message":"No, some bad test keeps creating it and I check it in by accident :(","commit_id":"7655982dfc5af887fd83ba30be374224b32bd371"}],"oslo_log/spool.py":[{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"eb4dd877f014d40f3c1cae69181b35d78a4c651c","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        # TODO(lxsli) max_spools"},{"line_number":87,"context_line":"        self._logger \u003d logging.getLogger(\u0027spool.%s\u0027 % self._kind)"},{"line_number":88,"context_line":"        self._logger.level \u003d 1  # grab everything"},{"line_number":89,"context_line":"        self._logger.propagate \u003d 0"},{"line_number":90,"context_line":"        self._spools \u003d {}"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"dab17558_67a92924","line":89,"updated":"2016-05-17 09:34:36.000000000","message":"need to test these values can be overridden by logconfig","commit_id":"b9c6cd791106d8f9636232351227827ca3e40ff8"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"02c7d0f64721456db58a30d360b30216aeab2e94","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        \"\"\"Add a record to the spool.\"\"\""},{"line_number":46,"context_line":"        with self._lock:"},{"line_number":47,"context_line":"            size \u003d sys.getsizeof(record)"},{"line_number":48,"context_line":"            self._check_space(size)"},{"line_number":49,"context_line":"            self._bytes +\u003d size"},{"line_number":50,"context_line":"            self._records.append((record, size))"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"dab17558_8074c935","line":48,"updated":"2016-05-17 23:01:48.000000000","message":"Qq, how are clients supposed to handle \u0027SpoolOverflowException\u0027 being raised from this? If I\u0027m a normal logger user am I supposed to start doing try/except around all my logging calls? Something else?","commit_id":"440511107a82af33128b33961a1df0947c6a8bcb"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"691cc6d84af2268b135f5cc5d5fc4206cb134546","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        \"\"\"Add a record to the spool.\"\"\""},{"line_number":46,"context_line":"        with self._lock:"},{"line_number":47,"context_line":"            size \u003d sys.getsizeof(record)"},{"line_number":48,"context_line":"            self._check_space(size)"},{"line_number":49,"context_line":"            self._bytes +\u003d size"},{"line_number":50,"context_line":"            self._records.append((record, size))"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_0091d2f0","line":48,"in_reply_to":"dab17558_8074c935","updated":"2016-05-18 14:17:37.000000000","message":"You should have a try/except Exception around the whole request handler which cleanly fails the request. The assumption is that you have set a high limit and if the limit is breached, something is probably looping forever.","commit_id":"440511107a82af33128b33961a1df0947c6a8bcb"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"5aeccb9ce0f4fd9d4fd0b242d7162e3afccdc5be","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        ref \u003d self._spools.get(req_id)"},{"line_number":107,"context_line":"        if ref:"},{"line_number":108,"context_line":"            return ref()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        def _del(it):"},{"line_number":111,"context_line":"            del self._spools[req_id]"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_ebb615db","line":109,"updated":"2016-05-19 00:33:07.000000000","message":"Could this just have used https://docs.python.org/2/library/weakref.html#weakref.WeakValueDictionary instead?","commit_id":"0994a557d00c6fb0f7d3a1a2d3b5b592bf229c7b"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"b07f12d015f4d26e677cf810f6b5ce1dbcc4fa67","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        ref \u003d self._spools.get(req_id)"},{"line_number":107,"context_line":"        if ref:"},{"line_number":108,"context_line":"            return ref()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        def _del(it):"},{"line_number":111,"context_line":"            del self._spools[req_id]"},{"line_number":112,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_9d07bd80","line":109,"in_reply_to":"bab6814e_ebb615db","updated":"2016-05-19 11:57:22.000000000","message":"Nice! The warning is a bit scary but looking at the code, they\u0027re doing a far better job than I naively had","commit_id":"0994a557d00c6fb0f7d3a1a2d3b5b592bf229c7b"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"5aeccb9ce0f4fd9d4fd0b242d7162e3afccdc5be","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        def _del(it):"},{"line_number":111,"context_line":"            del self._spools[req_id]"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        spool \u003d SpoolLogger(self, req_id)"},{"line_number":114,"context_line":"        self._spools[req_id] \u003d weakref.ref(spool, _del)"},{"line_number":115,"context_line":"        return spool"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_abc52d3d","line":112,"updated":"2016-05-19 00:33:07.000000000","message":"Also does this need to be thread-safe? Can 2 threads be in get_spool and end up with 2 different spool loggers of the same name being used  by different threads?","commit_id":"0994a557d00c6fb0f7d3a1a2d3b5b592bf229c7b"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"b07f12d015f4d26e677cf810f6b5ce1dbcc4fa67","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        def _del(it):"},{"line_number":111,"context_line":"            del self._spools[req_id]"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        spool \u003d SpoolLogger(self, req_id)"},{"line_number":114,"context_line":"        self._spools[req_id] \u003d weakref.ref(spool, _del)"},{"line_number":115,"context_line":"        return spool"}],"source_content_type":"text/x-python","patch_set":6,"id":"bab6814e_5afa70be","line":112,"in_reply_to":"bab6814e_abc52d3d","updated":"2016-05-19 11:57:22.000000000","message":"Added a lock, thanks","commit_id":"0994a557d00c6fb0f7d3a1a2d3b5b592bf229c7b"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"63c8f15bce1197b890ffc5c9047c2b42c8b6c6ff","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        name supplied to the formatter will be \"spool.\u003ckind\u003e.\u003creq_id\u003e\", so this"},{"line_number":104,"context_line":"        string will probably appear in log files."},{"line_number":105,"context_line":"        \"\"\""},{"line_number":106,"context_line":"        self._lock.acquire()"},{"line_number":107,"context_line":"        spool \u003d self._spools.get(req_id)"},{"line_number":108,"context_line":"        if spool:"},{"line_number":109,"context_line":"            self._lock.release()"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_0047ab4b","line":106,"updated":"2016-05-20 05:37:56.000000000","message":"Use the lock as a context manager??\n\n  with self._lock??","commit_id":"5c5999b82f021891df3f23acfb06a9d8a4919d94"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"bd4afe3bb0ee367b0aa6d2323030fb6b2f106a8a","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        name supplied to the formatter will be \"spool.\u003ckind\u003e.\u003creq_id\u003e\", so this"},{"line_number":104,"context_line":"        string will probably appear in log files."},{"line_number":105,"context_line":"        \"\"\""},{"line_number":106,"context_line":"        self._lock.acquire()"},{"line_number":107,"context_line":"        spool \u003d self._spools.get(req_id)"},{"line_number":108,"context_line":"        if spool:"},{"line_number":109,"context_line":"            self._lock.release()"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_46e424ab","line":106,"in_reply_to":"bab6814e_0047ab4b","updated":"2016-05-20 10:06:50.000000000","message":"Doh! Done","commit_id":"5c5999b82f021891df3f23acfb06a9d8a4919d94"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"63c8f15bce1197b890ffc5c9047c2b42c8b6c6ff","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self._lock.release()"},{"line_number":115,"context_line":"        return spool"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def each_spool(self):"},{"line_number":118,"context_line":"        \"\"\"Generates each (req_id, logger) pair.\"\"\""},{"line_number":119,"context_line":"        for req_id in sorted(self._spools):"},{"line_number":120,"context_line":"            yield req_id, self._spools[req_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_20610fd4","line":117,"updated":"2016-05-20 05:37:56.000000000","message":"So only concern about this one is the note in https://docs.python.org/2/library/weakref.html#weakref.WeakValueDictionary that says\n\n  \u0027Caution: Because a WeakValueDictionary is built on top of a Python dictionary, it must not change size when iterating over it. This can be difficult to ensure for a WeakValueDictionary because actions performed by the program during iteration may cause items in the dictionary to vanish “by magic” (as a side effect of garbage collection).\u0027","commit_id":"5c5999b82f021891df3f23acfb06a9d8a4919d94"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"bd4afe3bb0ee367b0aa6d2323030fb6b2f106a8a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self._lock.release()"},{"line_number":115,"context_line":"        return spool"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def each_spool(self):"},{"line_number":118,"context_line":"        \"\"\"Generates each (req_id, logger) pair.\"\"\""},{"line_number":119,"context_line":"        for req_id in sorted(self._spools):"},{"line_number":120,"context_line":"            yield req_id, self._spools[req_id]"}],"source_content_type":"text/x-python","patch_set":8,"id":"bab6814e_ecc9b95a","line":117,"in_reply_to":"bab6814e_20610fd4","updated":"2016-05-20 10:06:50.000000000","message":"Yep scary warning. I thought I might be safe because I\u0027m getting the list of keys up front and WVD has internal logic to protect itself during that. But then of course I could try to get a key that\u0027s no longer present.\n\nSoo a few approaches are available. I could simply catch the AttributeError and pass, which will give Bandit conniptions but actually be fine apart from the overhead of raising an exception and unwinding the stack. I could take temporary hardrefs to all values with \"list(self._spools.values())\" which would prevent collection. Or I could use the _IterationGuard which WVD uses itself to protect iteration methods, despite that being private. This is a bundled version of strategy 2.\n\nI\u0027ve taken the last approach because it seems neatest and I don\u0027t see any reason a helpful thing like that should be private. If you don\u0027t like it, I have no objection to using strategy 2 though. I don\u0027t like raising exceptions for normal logic though.","commit_id":"5c5999b82f021891df3f23acfb06a9d8a4919d94"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"a6a096ddd1c65299e42185a4cf714aaf6ebdd8a9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from _weakrefset import _IterationGuard"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import sys"},{"line_number":16,"context_line":"import threading"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_431388b8","line":13,"updated":"2016-05-25 21:34:59.000000000","message":"Is this module meant to be imported like this? Those _ make me think it\u0027s private and so is _IterationGuard?","commit_id":"7655982dfc5af887fd83ba30be374224b32bd371"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"987baeb875a837172e6afe86ce1d7586150f3897","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from _weakrefset import _IterationGuard"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import sys"},{"line_number":16,"context_line":"import threading"}],"source_content_type":"text/x-python","patch_set":9,"id":"bab6814e_5c2a3091","line":13,"in_reply_to":"bab6814e_431388b8","updated":"2016-05-26 09:29:31.000000000","message":"Well, I presume the same. I don\u0027t see /why/, there\u0027s no comment, but that doesn\u0027t mean there isn\u0027t a reason. I could not use it fairly easily, but I\u0027d have to rely on Python not optimising code (safe :\u003c) and above all it just seems neater to use the thing designed for this exact purpose.","commit_id":"7655982dfc5af887fd83ba30be374224b32bd371"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"dfb5f314c895ea4fd4aff7457a4f95fe15497bfb","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from _weakrefset import _IterationGuard"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import sys"},{"line_number":16,"context_line":"import threading"}],"source_content_type":"text/x-python","patch_set":11,"id":"7aa08908_61b89ce9","line":13,"updated":"2016-06-15 18:31:09.000000000","message":"I\u0027m still not comfortable using this private feature.","commit_id":"3f384c8cdfb5b23ca1ad7ce160fc2475454ffa68"},{"author":{"_account_id":1297,"name":"Joshua Harlow","email":"jxharlow@godaddy.com","username":"harlowja"},"change_message_id":"ea9da922fc70c9854811763c46faab59fba020a8","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    def handle(self, record):"},{"line_number":42,"context_line":"        \"\"\"Add a record to the spool.\"\"\""},{"line_number":43,"context_line":"        with self._lock:"},{"line_number":44,"context_line":"            size \u003d sys.getsizeof(record)"},{"line_number":45,"context_line":"            self._check_space(size)"},{"line_number":46,"context_line":"            self._bytes +\u003d size"},{"line_number":47,"context_line":"            self._records.append((record, size))"}],"source_content_type":"text/x-python","patch_set":11,"id":"7aa08908_869e70b6","line":44,"updated":"2016-06-16 00:13:49.000000000","message":"Ya, so this is the whole we are storing a record, which itself contains references to all the things sent to the LOG message, which typically includes custom objects.\n\nI\u0027m somewhat/pretty concerned that by buffering these record objects internally that we are going to produce some awfully confusing log messages when we finally format those log messages into actually messages (because people that use loggers typically pass objects to those loggers and expect the log message to reflect the object at the point of logging and not at a later point):\n\nFor example::\n\n    \u003e\u003e\u003e class A(object):\n    ...    def __init__(self):\n    ...       self.state \u003d \u0027on\u0027\n    ...    def __repr__(self):\n    ...       return \u0027A\u003c%s\u003e\u0027 % self.state\n    ... \n    \u003e\u003e\u003e import logging\n    \u003e\u003e\u003e logging.basicConfig(level\u003dlogging.DEBUG)\n    \u003e\u003e\u003e \n    \u003e\u003e\u003e a \u003d A()\n    \u003e\u003e\u003e logging.debug(\"At time zero a\u003d%s\", a)\n    DEBUG:root:At time zero a\u003dA\u003con\u003e\n    \u003e\u003e\u003e a.state \u003d \u0027off\u0027\n    \u003e\u003e\u003e logging.debug(\"At time zero+1 a\u003d%s\", a)\n    DEBUG:root:At time zero+1 a\u003dA\u003coff\u003e\n\nNow if we spool the first log message and capture the string and the \u0027a\u0027 object and then the code does the same at time zero+1, both being spooled, then when we flush the spool (due to failure or other), what will be output, it surely won\u0027t be \u0027at time zero a \u003d A\u003con\u003e\u0027 so therefore this will be confusing (and incorrect) for anyone looking at the logs.\n\nTo me that incorrectness will be worse than just wasting space running at DEBUG level (cause meh, space cheap)...","commit_id":"3f384c8cdfb5b23ca1ad7ce160fc2475454ffa68"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"3e4694abada32c449636240c18b6d04058d106dd","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    def handle(self, record):"},{"line_number":42,"context_line":"        \"\"\"Add a record to the spool.\"\"\""},{"line_number":43,"context_line":"        with self._lock:"},{"line_number":44,"context_line":"            size \u003d sys.getsizeof(record)"},{"line_number":45,"context_line":"            self._check_space(size)"},{"line_number":46,"context_line":"            self._bytes +\u003d size"},{"line_number":47,"context_line":"            self._records.append((record, size))"}],"source_content_type":"text/x-python","patch_set":11,"id":"7aa08908_cdf082cc","line":44,"in_reply_to":"7aa08908_869e70b6","updated":"2016-06-16 11:22:28.000000000","message":"I agree this isn\u0027t OK and will control my urge to say \"don\u0027t do that then\". It\u0027ll be endemic. I don\u0027t see an easy workaround though.\n\nWe could deepcopy or serialise. Deepcopy doesn\u0027t work on locks and I can\u0027t promise there won\u0027t be any. Maybe we could write our own that strips them out instead of panicking? Hopefully formatters won\u0027t rely on them.\n\nSerialising has to be done by the right formatter or the output format will be wrong. Formatters may be called by handlers\u0027 #emit method. We could add a filter to each handler that pre-formats and spools the record and prevents the handler trying to reformat it.\n\nSome handlers don\u0027t use formatters, EG logging.handlers.SocketHandler pickles. There\u0027s no API to get in between the pickling and the sending, so if we chose this strategy, some handlers wouldn\u0027t work.\n\nWe could also petition the TC to allow Clojure/Haskell and purge impurity from the world.\n\nAny other ideas please?","commit_id":"3f384c8cdfb5b23ca1ad7ce160fc2475454ffa68"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"dfb5f314c895ea4fd4aff7457a4f95fe15497bfb","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            return spool"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def each_spool(self):"},{"line_number":121,"context_line":"        \"\"\"Generates each (req_id, logger) pair.\"\"\""},{"line_number":122,"context_line":"        with _IterationGuard(self._spools):"},{"line_number":123,"context_line":"            for req_id in sorted(self._spools):"},{"line_number":124,"context_line":"                yield req_id, self._spools[req_id]"}],"source_content_type":"text/x-python","patch_set":11,"id":"7aa08908_4192c04c","line":121,"updated":"2016-06-15 18:31:09.000000000","message":"Could this just be\n\n  return list(sorted(self._spools.items()))\n\nThat would eliminate the need to guard the iteration, right? How many of these things do you expect to have at a time?\n\nIf the implementation proposed is OK, maybe change the name to all_spools() or something to reflect the fact that it\u0027s returning data rather than iterating over it.","commit_id":"3f384c8cdfb5b23ca1ad7ce160fc2475454ffa68"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"3e4694abada32c449636240c18b6d04058d106dd","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            return spool"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def each_spool(self):"},{"line_number":121,"context_line":"        \"\"\"Generates each (req_id, logger) pair.\"\"\""},{"line_number":122,"context_line":"        with _IterationGuard(self._spools):"},{"line_number":123,"context_line":"            for req_id in sorted(self._spools):"},{"line_number":124,"context_line":"                yield req_id, self._spools[req_id]"}],"source_content_type":"text/x-python","patch_set":11,"id":"7aa08908_fb86db14","line":121,"in_reply_to":"7aa08908_4192c04c","updated":"2016-06-16 11:22:28.000000000","message":"Done:\n\n  def all_spools(self):\n      \"\"\"Returns a sorted list of (request-id, spool) tuples for all spools\n      owned by this manager.\n\n      This list contains hard references to prevent spools from being\n      garbage-collected during iteration.\n      \"\"\"\n      return list(sorted((k, ref()) for k, ref in self._spools.items()))","commit_id":"3f384c8cdfb5b23ca1ad7ce160fc2475454ffa68"}]}