)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"867b65c8_b8b70f9e","updated":"2023-04-10 21:41:58.000000000","message":"Should the documentation be updated as well to indicate zookeeper 3.6 or newer is now required? Or maybe an upgrade release note?","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"}],"nodepool/tests/unit/test_zk.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":true,"context_lines":[{"line_number":1170,"context_line":"        paths \u003d set(contents.keys())"},{"line_number":1171,"context_line":"        for _ in iterate_timeout(10, Exception, \u0027cache to sync\u0027, interval\u003d0.1):"},{"line_number":1172,"context_line":"            object_paths \u003d set("},{"line_number":1173,"context_line":"                [x.path for x in cache._cached_objects.values()])"},{"line_number":1174,"context_line":"            cached_paths \u003d cache._cached_paths.copy()"},{"line_number":1175,"context_line":"            cached_paths.discard(\u0027/test\u0027)"},{"line_number":1176,"context_line":"            object_paths \u003d set("}],"source_content_type":"text/x-python","patch_set":6,"id":"21feddce_4e49c996","line":1173,"updated":"2023-04-10 21:41:58.000000000","message":"I don\u0027t think accessing cache._cached_objects.values() side effects loading the cache._cached_paths values. Is this set comprehension redundant with the one on line 1176 - 1177?","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5069c89194927f2a1ebfc46a589705a83504ab24","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"        paths \u003d set(contents.keys())"},{"line_number":1171,"context_line":"        for _ in iterate_timeout(10, Exception, \u0027cache to sync\u0027, interval\u003d0.1):"},{"line_number":1172,"context_line":"            object_paths \u003d set("},{"line_number":1173,"context_line":"                [x.path for x in cache._cached_objects.values()])"},{"line_number":1174,"context_line":"            cached_paths \u003d cache._cached_paths.copy()"},{"line_number":1175,"context_line":"            cached_paths.discard(\u0027/test\u0027)"},{"line_number":1176,"context_line":"            object_paths \u003d set("}],"source_content_type":"text/x-python","patch_set":6,"id":"5c9a7a9f_05a0eb4e","line":1173,"in_reply_to":"21feddce_4e49c996","updated":"2023-04-10 21:57:49.000000000","message":"Yes this is duplicative.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"}],"nodepool/zk/zookeeper.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"3f0e6f0422e5be26d88e7d0f4ffda6e310515454","unresolved":true,"context_lines":[{"line_number":747,"context_line":""},{"line_number":748,"context_line":"    def _start(self):"},{"line_number":749,"context_line":"        self.log.debug(\"Initialize cache\")"},{"line_number":750,"context_line":"        self.zk.kazoo_client.add_watch(self.root, self._cacheListener,"},{"line_number":751,"context_line":"                                       AddWatchMode.PERSISTENT_RECURSIVE)"},{"line_number":752,"context_line":"        self._walkTree()"},{"line_number":753,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"266a3da5_3b744ad5","line":750,"updated":"2023-03-28 12:33:34.000000000","message":"In contrast to the cache listener for the tree cache, the callbacks will be processed in the Kazoo callback worker (single threaded IIRC).\n\n\u003e Watch and completion callbacks should avoid blocking behavior as the next callback of that type won’t be run until it completes. If you need to block, spawn a new thread and return immediately so callbacks can proceed.\nhttps://kazoo.readthedocs.io/en/latest/api/handlers/threading.html#kazoo.handlers.threading.SequentialThreadingHandler\n\nThe cache listener is doing quite a lot, including ZK requests. I\u0027m wondering if we should instead mimic the Kazoo tree cache impl. to have the watch listener dispatch the event to a separate thread that\u0027s only processing events for the nodepool tree cache.","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"449b0da9c037f1908a7c60eb59f3ce41a08003f6","unresolved":false,"context_lines":[{"line_number":747,"context_line":""},{"line_number":748,"context_line":"    def _start(self):"},{"line_number":749,"context_line":"        self.log.debug(\"Initialize cache\")"},{"line_number":750,"context_line":"        self.zk.kazoo_client.add_watch(self.root, self._cacheListener,"},{"line_number":751,"context_line":"                                       AddWatchMode.PERSISTENT_RECURSIVE)"},{"line_number":752,"context_line":"        self._walkTree()"},{"line_number":753,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"2214f8a4_5f117d75","line":750,"in_reply_to":"266a3da5_3b744ad5","updated":"2023-03-28 21:15:02.000000000","message":"We can move all the callback handling into a background queue processing thread.","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"3f0e6f0422e5be26d88e7d0f4ffda6e310515454","unresolved":true,"context_lines":[{"line_number":780,"context_line":"                        event \u003d WatchedEvent("},{"line_number":781,"context_line":"                            EventType.DELETED,"},{"line_number":782,"context_line":"                            self.zk.kazoo_client._state,"},{"line_number":783,"context_line":"                            root)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _cacheListener(self, event):"},{"line_number":786,"context_line":"        self.log.debug(\"Cache event %s\", event)"}],"source_content_type":"text/x-python","patch_set":4,"id":"dc0514f6_9a46fa00","line":783,"updated":"2023-03-28 12:33:34.000000000","message":"I think we need to call `self._cacheListener(event)` here","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"449b0da9c037f1908a7c60eb59f3ce41a08003f6","unresolved":false,"context_lines":[{"line_number":780,"context_line":"                        event \u003d WatchedEvent("},{"line_number":781,"context_line":"                            EventType.DELETED,"},{"line_number":782,"context_line":"                            self.zk.kazoo_client._state,"},{"line_number":783,"context_line":"                            root)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _cacheListener(self, event):"},{"line_number":786,"context_line":"        self.log.debug(\"Cache event %s\", event)"}],"source_content_type":"text/x-python","patch_set":4,"id":"289de98d_a04d0db2","line":783,"in_reply_to":"dc0514f6_9a46fa00","updated":"2023-03-28 21:15:02.000000000","message":"Yep; I\u0027ve added a smoke test to catch this and other errors.","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"3f0e6f0422e5be26d88e7d0f4ffda6e310515454","unresolved":true,"context_lines":[{"line_number":821,"context_line":"            # Perform an in-place update of the cached object if possible"},{"line_number":822,"context_line":"            old_obj \u003d self._cached_objects.get(key)"},{"line_number":823,"context_line":"            if old_obj:"},{"line_number":824,"context_line":"                if stat.version \u003c\u003d old_obj.stat.version:"},{"line_number":825,"context_line":"                    # Don\u0027t update to older data"},{"line_number":826,"context_line":"                    return"},{"line_number":827,"context_line":"                if getattr(old_obj, \u0027lock\u0027, None):"}],"source_content_type":"text/x-python","patch_set":4,"id":"363722cd_c4070748","line":824,"updated":"2023-03-28 12:33:34.000000000","message":"Just a note as it\u0027s been like this before: when the znode is deleted and re-created the version will start again from 0, so it might go backward.\n\nComparing the mzxids is IMHO the safer choice here.","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"449b0da9c037f1908a7c60eb59f3ce41a08003f6","unresolved":false,"context_lines":[{"line_number":821,"context_line":"            # Perform an in-place update of the cached object if possible"},{"line_number":822,"context_line":"            old_obj \u003d self._cached_objects.get(key)"},{"line_number":823,"context_line":"            if old_obj:"},{"line_number":824,"context_line":"                if stat.version \u003c\u003d old_obj.stat.version:"},{"line_number":825,"context_line":"                    # Don\u0027t update to older data"},{"line_number":826,"context_line":"                    return"},{"line_number":827,"context_line":"                if getattr(old_obj, \u0027lock\u0027, None):"}],"source_content_type":"text/x-python","patch_set":4,"id":"274dc107_35fef837","line":824,"in_reply_to":"363722cd_c4070748","updated":"2023-03-28 21:15:02.000000000","message":"Done.","commit_id":"fd5fc17735a4864e5d68b2da69eb3fc4713f2a08"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":true,"context_lines":[{"line_number":741,"context_line":"        self.root \u003d root"},{"line_number":742,"context_line":"        self._cached_objects \u003d {}"},{"line_number":743,"context_line":"        self._cached_paths \u003d set()"},{"line_number":744,"context_line":"        self._started \u003d False"},{"line_number":745,"context_line":"        self._stopped \u003d False"},{"line_number":746,"context_line":"        self._queue \u003d queue.Queue()"},{"line_number":747,"context_line":"        self._background_thread \u003d threading.Thread("}],"source_content_type":"text/x-python","patch_set":6,"id":"cc0eb0d6_d1d0435e","line":744,"updated":"2023-04-10 21:41:58.000000000","message":"I don\u0027t see anything that flips self._started to True. Should that be set in _start()?","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5069c89194927f2a1ebfc46a589705a83504ab24","unresolved":false,"context_lines":[{"line_number":741,"context_line":"        self.root \u003d root"},{"line_number":742,"context_line":"        self._cached_objects \u003d {}"},{"line_number":743,"context_line":"        self._cached_paths \u003d set()"},{"line_number":744,"context_line":"        self._started \u003d False"},{"line_number":745,"context_line":"        self._stopped \u003d False"},{"line_number":746,"context_line":"        self._queue \u003d queue.Queue()"},{"line_number":747,"context_line":"        self._background_thread \u003d threading.Thread("}],"source_content_type":"text/x-python","patch_set":6,"id":"f22c6eaf_b0aee6cb","line":744,"in_reply_to":"cc0eb0d6_d1d0435e","updated":"2023-04-10 21:57:49.000000000","message":"Yep.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":true,"context_lines":[{"line_number":802,"context_line":"            for child in self.zk.kazoo_client.get_children(root):"},{"line_number":803,"context_line":"                self._walkTree(\u0027/\u0027.join([root, child]), seen_paths)"},{"line_number":804,"context_line":"        if am_root:"},{"line_number":805,"context_line":"            for path in self._cached_paths:"},{"line_number":806,"context_line":"                if path not in seen_paths:"},{"line_number":807,"context_line":"                    event \u003d WatchedEvent("},{"line_number":808,"context_line":"                        EventType.NONE,"}],"source_content_type":"text/x-python","patch_set":6,"id":"e6a891aa_d4c07771","line":805,"updated":"2023-04-10 21:41:58.000000000","message":"Does order matter? Currently _cached_paths is a set which is unordered which means we may get a delete event for /foo before /foo/bar. I think for the cache consistency itself this isn\u0027t a big deal because we simply manipulate the contents of this set, but if other operations operating on the cache don\u0027t expect this possibly inconsistent state we may have problems.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5069c89194927f2a1ebfc46a589705a83504ab24","unresolved":false,"context_lines":[{"line_number":802,"context_line":"            for child in self.zk.kazoo_client.get_children(root):"},{"line_number":803,"context_line":"                self._walkTree(\u0027/\u0027.join([root, child]), seen_paths)"},{"line_number":804,"context_line":"        if am_root:"},{"line_number":805,"context_line":"            for path in self._cached_paths:"},{"line_number":806,"context_line":"                if path not in seen_paths:"},{"line_number":807,"context_line":"                    event \u003d WatchedEvent("},{"line_number":808,"context_line":"                        EventType.NONE,"}],"source_content_type":"text/x-python","patch_set":6,"id":"42b79aa7_8bcbdd6e","line":805,"in_reply_to":"e6a891aa_d4c07771","updated":"2023-04-10 21:57:49.000000000","message":"Order doesn\u0027t matter here.  This is not a delete or create event or any other kind of event that is inherently sequenced, this is a \"check\" event that just notifies recipients they should update the state of the path.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":true,"context_lines":[{"line_number":838,"context_line":"        else:"},{"line_number":839,"context_line":"            self._cached_paths.discard(event.path)"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        # Ignore root node"},{"line_number":842,"context_line":"        if event.path \u003d\u003d self.root:"},{"line_number":843,"context_line":"            return"},{"line_number":844,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"86262358_a90e76bc","line":841,"updated":"2023-04-10 21:41:58.000000000","message":"Might be worth noting why we ignore the root node? I think because nodepool (and zuul) don\u0027t write operational data to the root its all organizational at that level?","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5069c89194927f2a1ebfc46a589705a83504ab24","unresolved":false,"context_lines":[{"line_number":838,"context_line":"        else:"},{"line_number":839,"context_line":"            self._cached_paths.discard(event.path)"},{"line_number":840,"context_line":""},{"line_number":841,"context_line":"        # Ignore root node"},{"line_number":842,"context_line":"        if event.path \u003d\u003d self.root:"},{"line_number":843,"context_line":"            return"},{"line_number":844,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"c5e226e6_3a1c55ce","line":841,"in_reply_to":"86262358_a90e76bc","updated":"2023-04-10 21:57:49.000000000","message":"Sure that would be better.  I don\u0027t recall writing this comment, and it\u0027s several years old.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"49953bc3917461f61d0b2169e31adf05448a666d","unresolved":true,"context_lines":[{"line_number":1088,"context_line":"    # Private Methods"},{"line_number":1089,"context_line":"    # \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":1090,"context_line":"    def _onConnect(self):"},{"line_number":1091,"context_line":"        if self.enable_cache and self._node_cache is None:"},{"line_number":1092,"context_line":"            self._node_cache \u003d NodeCache(self, self.NODE_ROOT)"},{"line_number":1093,"context_line":"            self._request_cache \u003d RequestCache(self, self.REQUEST_ROOT)"},{"line_number":1094,"context_line":"            self._image_cache \u003d ImageCache(self, self.IMAGE_ROOT)"}],"source_content_type":"text/x-python","patch_set":6,"id":"20429a89_18b266ad","line":1091,"updated":"2023-04-10 21:41:58.000000000","message":"I guess the idea with the new check here is to check for initial startup. Otherwise all three caches will exist until the process dies as we don\u0027t replace them on reconnection.","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"7b2f10f9cb411f0e028aa9f5ed9cada0c55129b6","unresolved":false,"context_lines":[{"line_number":1088,"context_line":"    # Private Methods"},{"line_number":1089,"context_line":"    # \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":1090,"context_line":"    def _onConnect(self):"},{"line_number":1091,"context_line":"        if self.enable_cache and self._node_cache is None:"},{"line_number":1092,"context_line":"            self._node_cache \u003d NodeCache(self, self.NODE_ROOT)"},{"line_number":1093,"context_line":"            self._request_cache \u003d RequestCache(self, self.REQUEST_ROOT)"},{"line_number":1094,"context_line":"            self._image_cache \u003d ImageCache(self, self.IMAGE_ROOT)"}],"source_content_type":"text/x-python","patch_set":6,"id":"47e83503_33801be7","line":1091,"in_reply_to":"20429a89_18b266ad","updated":"2023-04-10 22:04:42.000000000","message":"Done","commit_id":"6739766c2113153c3bbd500b93b30f891523d104"}]}