)]}'
{".zuul.yaml":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"f5ea553caea854ae6cc2169fafdd3f629c13f21b","unresolved":false,"context_lines":[{"line_number":228,"context_line":"              - zuul/cmd/migrate.py"},{"line_number":229,"context_line":"              - playbooks/zuul-migrate/.*"},{"line_number":230,"context_line":"            nodeset: ubuntu-bionic"},{"line_number":231,"context_line":"            timeout: 7200  # 120 minutes / 2 hours"},{"line_number":232,"context_line":"            vars: \u0026zuul_tox_vars"},{"line_number":233,"context_line":"                test_setup_environment:"},{"line_number":234,"context_line":"                    ZUUL_TEST_ROOT: /tmp/zuul-test"}],"source_content_type":"text/x-yaml","patch_set":59,"id":"9816dd95_8e04b255","line":231,"updated":"2021-02-05 17:56:18.000000000","message":"-1: We\u0027re not doing anything in this change that makes tests longer, right?  All we\u0027re doing is masking a later expansion of test time?\n\nIt seems like it might be more appropriate to raise the time limit on the change where we made the tests run a lot longer, then we can have a conversation about whether that\u0027s appropriate and how we want to deal with it.","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"f5f43f08517d26b05a919a7a152d178bbb8a2395","unresolved":false,"context_lines":[{"line_number":228,"context_line":"              - zuul/cmd/migrate.py"},{"line_number":229,"context_line":"              - playbooks/zuul-migrate/.*"},{"line_number":230,"context_line":"            nodeset: ubuntu-bionic"},{"line_number":231,"context_line":"            timeout: 7200  # 120 minutes / 2 hours"},{"line_number":232,"context_line":"            vars: \u0026zuul_tox_vars"},{"line_number":233,"context_line":"                test_setup_environment:"},{"line_number":234,"context_line":"                    ZUUL_TEST_ROOT: /tmp/zuul-test"}],"source_content_type":"text/x-yaml","patch_set":59,"id":"99ca6b00_ccc92e0c","line":231,"in_reply_to":"9816dd95_8e04b255","updated":"2021-02-08 13:45:27.000000000","message":"Zookeeper may introduce some lags to the system, therefore the tox-py3x jobs timeouts are increased from 80 to 120 mins.\n\nWhile it is true that reasons for longer tests are happening later on, this was increased here due to, splitting and reordering changes time to time. This change was to be always the first change in the pipeline before other ZK changes and therefore I put it here. But yes there is nothing in this change needing this increase.","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"2c6041a7f222eaab6aa78fc841ba3a42b10c2c2a","unresolved":false,"context_lines":[{"line_number":228,"context_line":"              - zuul/cmd/migrate.py"},{"line_number":229,"context_line":"              - playbooks/zuul-migrate/.*"},{"line_number":230,"context_line":"            nodeset: ubuntu-bionic"},{"line_number":231,"context_line":"            timeout: 7200  # 120 minutes / 2 hours"},{"line_number":232,"context_line":"            vars: \u0026zuul_tox_vars"},{"line_number":233,"context_line":"                test_setup_environment:"},{"line_number":234,"context_line":"                    ZUUL_TEST_ROOT: /tmp/zuul-test"}],"source_content_type":"text/x-yaml","patch_set":59,"id":"e28e6f4b_a07c5335","line":231,"in_reply_to":"99ca6b00_ccc92e0c","updated":"2021-02-08 14:23:27.000000000","message":"It looks like at least starting with https://review.opendev.org/c/zuul/zuul/+/716221 we need to increase the test timeout. I\u0027ll move this into 716221.","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"61d942dcb32416942e02257912753c456da95cc4","unresolved":false,"context_lines":[{"line_number":228,"context_line":"              - zuul/cmd/migrate.py"},{"line_number":229,"context_line":"              - playbooks/zuul-migrate/.*"},{"line_number":230,"context_line":"            nodeset: ubuntu-bionic"},{"line_number":231,"context_line":"            timeout: 7200  # 120 minutes / 2 hours"},{"line_number":232,"context_line":"            vars: \u0026zuul_tox_vars"},{"line_number":233,"context_line":"                test_setup_environment:"},{"line_number":234,"context_line":"                    ZUUL_TEST_ROOT: /tmp/zuul-test"}],"source_content_type":"text/x-yaml","patch_set":59,"id":"cabe9bcf_60aa68c9","line":231,"in_reply_to":"e28e6f4b_a07c5335","updated":"2021-02-09 09:38:21.000000000","message":"Done","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"}],"tests/base.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"f4b15ddbf4086b358f8d7ece1fcb8fad9dcd3171","unresolved":false,"context_lines":[{"line_number":3853,"context_line":"        merge_client \u003d RecordingMergeClient(self.config, self.sched)"},{"line_number":3854,"context_line":"        nodepool \u003d zuul.nodepool.Nodepool(self.sched)"},{"line_number":3855,"context_line":"        zk_client \u003d zuul.zk.ZooKeeperClient()"},{"line_number":3856,"context_line":"        zk_nodepool \u003d zuul.zk.nodepool.ZooKeeperNodepool(zk_client)"},{"line_number":3857,"context_line":"        zk_client.connect(self.zk_config, timeout\u003d30.0)"},{"line_number":3858,"context_line":""},{"line_number":3859,"context_line":"        self.sched.setExecutor(executor_client)"}],"source_content_type":"text/x-python","patch_set":48,"id":"7f6b1bfe_df73d74c","line":3856,"updated":"2020-10-13 14:59:43.000000000","message":"I think we don\u0027t need to create zk_nodepool here as the scheduler can easily create it itself when we pass in the zk_client.","commit_id":"0b71dc97066138d250cb740498dcacca5d7bcbac"}],"zuul/cmd/scheduler.py":[{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"561cb9fd2ffdc722d0974dcfa75672fbbf05804e","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import zuul.cmd"},{"line_number":22,"context_line":"import zuul.executor.client"},{"line_number":23,"context_line":"from zuul.lib.config import get_default"},{"line_number":24,"context_line":"from zuul.lib.statsd import get_statsd_config"},{"line_number":25,"context_line":"import zuul.merger.client"},{"line_number":26,"context_line":"import zuul.nodepool"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_9bd04ccc","line":23,"updated":"2020-10-21 13:20:57.000000000","message":"nit: The import order looks a little messy here, but maybe that\u0027s only me :D","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"}],"zuul/zk.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":13,"context_line":"import json"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_84b6288a","line":16,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F401 \u0027typing.Any\u0027 imported but unused","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":13,"context_line":"import json"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_64bb2472","line":16,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F401 \u0027typing.Callable\u0027 imported but unused","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from kazoo.client import KazooClient, KazooState"}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_c4af9029","line":17,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F401 \u0027typing.List\u0027 imported but unused","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":356,"context_line":""},{"line_number":357,"context_line":"        lock_path \u003d \u0027%s/%s/lock\u0027 % (self.NODE_ROOT, node.id)"},{"line_number":358,"context_line":"        try:"},{"line_number":359,"context_line":"            lock \u003d Lock(self.client, lock_path)"},{"line_number":360,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":361,"context_line":"        except kze.LockTimeout:"},{"line_number":362,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_a4ac0c36","line":359,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F821 undefined name \u0027Lock\u0027","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":403,"context_line":""},{"line_number":404,"context_line":"        path \u003d \"%s/%s\" % (self.REQUEST_LOCK_ROOT, request.id)"},{"line_number":405,"context_line":"        try:"},{"line_number":406,"context_line":"            lock \u003d Lock(self.client, path)"},{"line_number":407,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":408,"context_line":"        except kze.LockTimeout:"},{"line_number":409,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_04aa1818","line":406,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F821 undefined name \u0027Lock\u0027","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"5f9cefb23fddb28df2183e2accdb2f9b91d66be1","unresolved":false,"context_lines":[{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        path \u003d \"%s/%s/lock\" % (self.HOLD_REQUEST_ROOT, request.id)"},{"line_number":677,"context_line":"        try:"},{"line_number":678,"context_line":"            lock \u003d Lock(self.client, path)"},{"line_number":679,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":680,"context_line":"        except kze.LockTimeout:"},{"line_number":681,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_e4a61454","line":678,"updated":"2020-04-03 13:50:20.000000000","message":"pep8: F821 undefined name \u0027Lock\u0027","commit_id":"03900fca590f8b8d66349b5a41a8bc34efe70398"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"import json"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_5a315da2","line":16,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F401 \u0027typing.Any\u0027 imported but unused","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":13,"context_line":"import json"},{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_ba286134","line":16,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F401 \u0027typing.Callable\u0027 imported but unused","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":14,"context_line":"import logging"},{"line_number":15,"context_line":"import time"},{"line_number":16,"context_line":"from typing import Dict, Any, Callable"},{"line_number":17,"context_line":"from typing import List"},{"line_number":18,"context_line":"from typing import Optional"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from kazoo.client import KazooClient, KazooState"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_9a2be52e","line":17,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F401 \u0027typing.List\u0027 imported but unused","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":369,"context_line":""},{"line_number":370,"context_line":"        lock_path \u003d \u0027%s/%s/lock\u0027 % (self.NODE_ROOT, node.id)"},{"line_number":371,"context_line":"        try:"},{"line_number":372,"context_line":"            lock \u003d Lock(self.client, lock_path)"},{"line_number":373,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":374,"context_line":"        except kze.LockTimeout:"},{"line_number":375,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_fa21494f","line":372,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F821 undefined name \u0027Lock\u0027","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        path \u003d \"%s/%s\" % (self.REQUEST_LOCK_ROOT, request.id)"},{"line_number":418,"context_line":"        try:"},{"line_number":419,"context_line":"            lock \u003d Lock(self.client, path)"},{"line_number":420,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":421,"context_line":"        except kze.LockTimeout:"},{"line_number":422,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_da1c4d15","line":419,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F821 undefined name \u0027Lock\u0027","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"34a30bcee55d64cb685b0411b0bbb22a2048413c","unresolved":false,"context_lines":[{"line_number":688,"context_line":""},{"line_number":689,"context_line":"        path \u003d \"%s/%s/lock\" % (self.HOLD_REQUEST_ROOT, request.id)"},{"line_number":690,"context_line":"        try:"},{"line_number":691,"context_line":"            lock \u003d Lock(self.client, path)"},{"line_number":692,"context_line":"            have_lock \u003d lock.acquire(blocking, timeout)"},{"line_number":693,"context_line":"        except kze.LockTimeout:"},{"line_number":694,"context_line":"            raise LockException("}],"source_content_type":"text/x-python","patch_set":4,"id":"3f4c43b2_3a1c5111","line":691,"updated":"2020-04-15 14:01:28.000000000","message":"linters: F821 undefined name \u0027Lock\u0027","commit_id":"4dee3f08fa5da948a46e68f1abd64c9f36cb0942"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"72717e19212170760ddda2694969bf66bbfdb428","unresolved":false,"context_lines":[{"line_number":720,"context_line":""},{"line_number":721,"context_line":"    # Scheduler part begins here"},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    CONFIG_ROOT \u003d \"/zuul\""},{"line_number":724,"context_line":"    # Node content max size: keep ~100kB as a reserve form the 1MB limit"},{"line_number":725,"context_line":"    CONFIG_MAX_SIZE \u003d 1024 * 1024 - 100 * 1024"},{"line_number":726,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_c44ff131","line":723,"range":{"start_line":723,"start_character":4,"end_line":723,"end_character":25},"updated":"2020-06-22 21:40:22.000000000","message":"I think we should make this root configurable. We do that for nodepool which enables us to do things like run tests for many fake nodepools simultaneously against the same db server. I expect we\u0027ll need to do similar with zuul in the near future.","commit_id":"bfd3d6384f279cb0b10887231e2da7e1f8d8e51e"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"678518701e93b81c0348a430f77ce17c636cd201","unresolved":false,"context_lines":[{"line_number":720,"context_line":""},{"line_number":721,"context_line":"    # Scheduler part begins here"},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    CONFIG_ROOT \u003d \"/zuul\""},{"line_number":724,"context_line":"    # Node content max size: keep ~100kB as a reserve form the 1MB limit"},{"line_number":725,"context_line":"    CONFIG_MAX_SIZE \u003d 1024 * 1024 - 100 * 1024"},{"line_number":726,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_722a9591","line":723,"range":{"start_line":723,"start_character":4,"end_line":723,"end_character":25},"in_reply_to":"bf51134e_c44ff131","updated":"2020-09-30 16:18:46.000000000","message":"Faking (prefixing) this root with the test name in test works here too :)","commit_id":"bfd3d6384f279cb0b10887231e2da7e1f8d8e51e"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"72717e19212170760ddda2694969bf66bbfdb428","unresolved":false,"context_lines":[{"line_number":725,"context_line":"    CONFIG_MAX_SIZE \u003d 1024 * 1024 - 100 * 1024"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"    def _getZuulNodePath(self, *args: str) -\u003e str:"},{"line_number":728,"context_line":"        return \"/\".join(filter(lambda s: s is not None and s !\u003d \u0027\u0027,"},{"line_number":729,"context_line":"                               [self.CONFIG_ROOT] + list(args)))"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_6458a5f9","line":728,"updated":"2020-06-22 21:40:22.000000000","message":"Should this be an os.path.join instead? Then it is an error if you input non string types and \u0027\u0027 is handled automatically.","commit_id":"bfd3d6384f279cb0b10887231e2da7e1f8d8e51e"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"678518701e93b81c0348a430f77ce17c636cd201","unresolved":false,"context_lines":[{"line_number":725,"context_line":"    CONFIG_MAX_SIZE \u003d 1024 * 1024 - 100 * 1024"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"    def _getZuulNodePath(self, *args: str) -\u003e str:"},{"line_number":728,"context_line":"        return \"/\".join(filter(lambda s: s is not None and s !\u003d \u0027\u0027,"},{"line_number":729,"context_line":"                               [self.CONFIG_ROOT] + list(args)))"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9f560f44_f51def62","line":728,"in_reply_to":"bf51134e_6458a5f9","updated":"2020-09-30 16:18:46.000000000","message":"You are right. But this part was removed now.","commit_id":"bfd3d6384f279cb0b10887231e2da7e1f8d8e51e"}],"zuul/zk/__init__.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"c838ba0b6d587715ef21e69d2b65983157c04b5e","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    testing only ZooKeeper interactions."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self, enable_cache: bool \u003d True):"},{"line_number":34,"context_line":"        \"\"\""},{"line_number":35,"context_line":"        Initialize the ZooKeeper object."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"9f560f44_db6af1e9","line":33,"updated":"2020-10-13 07:04:18.000000000","message":"As I understood this is the representation of the zk god class we have now. I think we should remove that completely since that will cause some problems. Not all components will need all zk data models. Further individual caches should be enabled per use case to restrict load on the zk cluster.\n\nI think the architecture would be cleaner if we have one zk model class per use case (nodepool, executor, pipeline state, etc) and use them individually (they still can all share the same kazoo client in the end). That makes it possible to tune them more fine granular (like fingergw needs executor data, but no caching, etc).","commit_id":"1dffaab477a4a2aae6501f75753fb808c9c23bc1"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"a1d895bd398838a658645be7c009c1604bb7b68f","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    testing only ZooKeeper interactions."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self, enable_cache: bool \u003d True):"},{"line_number":34,"context_line":"        \"\"\""},{"line_number":35,"context_line":"        Initialize the ZooKeeper object."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":47,"id":"9f560f44_ded01a56","line":33,"in_reply_to":"9f560f44_db6af1e9","updated":"2020-10-13 11:19:35.000000000","message":"Done. Yes, this would be better. Original concern was larger code change. But if thats ok then I will update it in the next patch.","commit_id":"1dffaab477a4a2aae6501f75753fb808c9c23bc1"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"f4b15ddbf4086b358f8d7ece1fcb8fad9dcd3171","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        self.on_connect_listeners: List[Callable[[], None]] \u003d []"},{"line_number":37,"context_line":"        self.on_disconnect_listeners: List[Callable[[], None]] \u003d []"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def _connectionListener(self, state):"},{"line_number":40,"context_line":"        \"\"\""},{"line_number":41,"context_line":"        Listener method for Kazoo connection state changes."},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":48,"id":"7f6b1bfe_5f39873f","line":39,"updated":"2020-10-13 14:59:43.000000000","message":"Off topic: In the future we might want to call connection state listeners here as well (depending on the use case).","commit_id":"0b71dc97066138d250cb740498dcacca5d7bcbac"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"f5ea553caea854ae6cc2169fafdd3f629c13f21b","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            self.client.set_hosts(hosts\u003dhosts)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"class ZooKeeperBase(metaclass\u003dABCMeta):"},{"line_number":136,"context_line":"    def __init__(self, client: ZooKeeperClient):"},{"line_number":137,"context_line":"        self.client \u003d client"},{"line_number":138,"context_line":"        self.client.on_connect_listeners.append(self._onConnect)"}],"source_content_type":"text/x-python","patch_set":59,"id":"0740aa3d_3f365bf4","line":135,"updated":"2021-02-05 17:56:18.000000000","message":"nit: Might be worth noting this is a base client for classes that implement application logic on ZK.","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"61d942dcb32416942e02257912753c456da95cc4","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            self.client.set_hosts(hosts\u003dhosts)"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"class ZooKeeperBase(metaclass\u003dABCMeta):"},{"line_number":136,"context_line":"    def __init__(self, client: ZooKeeperClient):"},{"line_number":137,"context_line":"        self.client \u003d client"},{"line_number":138,"context_line":"        self.client.on_connect_listeners.append(self._onConnect)"}],"source_content_type":"text/x-python","patch_set":59,"id":"18ec1ff0_762591d5","line":135,"in_reply_to":"0740aa3d_3f365bf4","updated":"2021-02-09 09:38:21.000000000","message":"Done","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"}],"zuul/zk/base.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"88ce4a4929a39226fca31b4f1e5a1e433e04cc51","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    @property"},{"line_number":54,"context_line":"    def connected(self):"},{"line_number":55,"context_line":"        return self.client.state \u003d\u003d KazooState.CONNECTED"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @property"},{"line_number":58,"context_line":"    def suspended(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"9f560f44_e699c76d","line":55,"updated":"2020-09-30 20:38:49.000000000","message":"Do these properties need to check if self.client is not None? Before we connect for the first time they would be None.\n\nPerhaps:\n\n  return self.client is not None and self.client.state \u003d\u003d CONNECTED","commit_id":"e50793d5defe4d196790e439454cc038638f573e"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"595385a8aaf7fd96ed70d5e8c4c30eba25700a91","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    @property"},{"line_number":54,"context_line":"    def connected(self):"},{"line_number":55,"context_line":"        return self.client.state \u003d\u003d KazooState.CONNECTED"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    @property"},{"line_number":58,"context_line":"    def suspended(self):"}],"source_content_type":"text/x-python","patch_set":41,"id":"9f560f44_cbec87bd","line":55,"in_reply_to":"9f560f44_e699c76d","updated":"2020-10-01 06:26:19.000000000","message":"Yes, good catch.","commit_id":"e50793d5defe4d196790e439454cc038638f573e"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"6c7c1f682dba701fc54d9f84c385496ac154b1ee","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return self._became_lost"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def resetLostFlag(self):"},{"line_number":70,"context_line":"        self._became_lost \u003d False"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def logConnectionRetryEvent(self):"},{"line_number":73,"context_line":"        now \u003d time.monotonic()"}],"source_content_type":"text/x-python","patch_set":43,"id":"9f560f44_ce8bf5bd","line":70,"updated":"2020-10-01 08:42:35.000000000","message":"You could remove the `didLoseConnection` property, `resetLostFlag()` method and maybe event the `_became_lost` flag as this is not used anywhere.","commit_id":"24bfb3e9ebd27f87b0441ef1e4d60a048c1bc1d5"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"11cafd8f1365320e8e0018e7b080ca227559815a","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return self._became_lost"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def resetLostFlag(self):"},{"line_number":70,"context_line":"        self._became_lost \u003d False"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def logConnectionRetryEvent(self):"},{"line_number":73,"context_line":"        now \u003d time.monotonic()"}],"source_content_type":"text/x-python","patch_set":43,"id":"9f560f44_d97c5db8","line":70,"in_reply_to":"9f560f44_ce8bf5bd","updated":"2020-10-01 09:42:01.000000000","message":"Done","commit_id":"24bfb3e9ebd27f87b0441ef1e4d60a048c1bc1d5"}],"zuul/zk/launcher.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"829115edf23674e6de142cc0e9cd879319f615a7","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"class Launcher:"},{"line_number":3,"context_line":"    \"\"\""},{"line_number":4,"context_line":"    Class to describe a nodepool launcher."},{"line_number":5,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9507ce82","line":2,"updated":"2020-08-05 11:44:25.000000000","message":"I think this might still fit fine into the nodepool file as it\u0027s only used there?","commit_id":"051768dd3ec170dd1fbd643e207346fc60b80613"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"18e8bdcae2e00744b88d584b3d3398b83af2d8ff","unresolved":false,"context_lines":[{"line_number":1,"context_line":""},{"line_number":2,"context_line":"class Launcher:"},{"line_number":3,"context_line":"    \"\"\""},{"line_number":4,"context_line":"    Class to describe a nodepool launcher."},{"line_number":5,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_354a42ab","line":2,"in_reply_to":"9f560f44_9507ce82","updated":"2020-08-05 11:56:19.000000000","message":"Done","commit_id":"051768dd3ec170dd1fbd643e207346fc60b80613"}],"zuul/zk/nodepool.py":[{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"c50fc12a6e446a6bf6878158c1e8d0447d511122","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    NODEPOOL_REQUEST_LOCK_ROOT \u003d \"/nodepool/requests-lock\""},{"line_number":32,"context_line":"    HOLD_REQUEST_ROOT \u003d \u0027/zuul/hold-requests\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":35,"context_line":"        super().__init__(*args, **kwargs)"},{"line_number":36,"context_line":"        # The caching model we use is designed around handing out model"},{"line_number":37,"context_line":"        # data as objects. To do this, we use two caches: one is a TreeCache"}],"source_content_type":"text/x-python","patch_set":29,"id":"9f560f44_fdd6d226","line":34,"updated":"2020-09-10 12:34:58.000000000","message":"It seems like this code is defined twice.","commit_id":"b27684784b4c84089719c264b97eb00b3e42b894"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"335589469fef373a3ed6cbe8d0d20685f5185a66","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            self._hold_request_tree \u003d None"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def __bytesToDict(self, data):"},{"line_number":68,"context_line":"        return json.loads(data.decode(\u0027utf8\u0027))"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def __launcherPath(self, launcher):"},{"line_number":71,"context_line":"        return \"%s/%s\" % (self.LAUNCHER_ROOT, launcher)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_47d35ad4","line":68,"updated":"2020-09-30 11:29:09.000000000","message":"That function can be static, and perhaps moved to the base class.","commit_id":"91a4faaa0c0a936efba60bcc0691b9cad3ed5861"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"678518701e93b81c0348a430f77ce17c636cd201","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            self._hold_request_tree \u003d None"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    def __bytesToDict(self, data):"},{"line_number":68,"context_line":"        return json.loads(data.decode(\u0027utf8\u0027))"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def __launcherPath(self, launcher):"},{"line_number":71,"context_line":"        return \"%s/%s\" % (self.LAUNCHER_ROOT, launcher)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_d5f38b08","line":68,"in_reply_to":"9f560f44_47d35ad4","updated":"2020-09-30 16:18:46.000000000","message":"I would rather remove it completely if you don\u0027t mind. Not a big fan of wrapping such simple one-liners into methods/functions.","commit_id":"91a4faaa0c0a936efba60bcc0691b9cad3ed5861"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"335589469fef373a3ed6cbe8d0d20685f5185a66","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __cacheFaultListener(self, e):"},{"line_number":77,"context_line":"        self.log.exception(e)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def _dictToStr(self, data):"},{"line_number":80,"context_line":"        return json.dumps(data).encode(\u0027utf8\u0027)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _strToDict(self, data):"},{"line_number":83,"context_line":"        return json.loads(data.decode(\u0027utf8\u0027))"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def getRegisteredLaunchers(self):"},{"line_number":86,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_87dd72df","line":83,"range":{"start_line":79,"start_character":0,"end_line":83,"end_character":46},"updated":"2020-09-30 11:29:09.000000000","message":"dito, it seems like useful to have for the other module.","commit_id":"91a4faaa0c0a936efba60bcc0691b9cad3ed5861"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"678518701e93b81c0348a430f77ce17c636cd201","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def __cacheFaultListener(self, e):"},{"line_number":77,"context_line":"        self.log.exception(e)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def _dictToStr(self, data):"},{"line_number":80,"context_line":"        return json.dumps(data).encode(\u0027utf8\u0027)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _strToDict(self, data):"},{"line_number":83,"context_line":"        return json.loads(data.decode(\u0027utf8\u0027))"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def getRegisteredLaunchers(self):"},{"line_number":86,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":40,"id":"9f560f44_f555efc3","line":83,"range":{"start_line":79,"start_character":0,"end_line":83,"end_character":46},"in_reply_to":"9f560f44_87dd72df","updated":"2020-09-30 16:18:46.000000000","message":"same here, the _strToDict anyway duplicates the __bytesToDict.","commit_id":"91a4faaa0c0a936efba60bcc0691b9cad3ed5861"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"6c7c1f682dba701fc54d9f84c385496ac154b1ee","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        return \"%s/%s\" % (self.NODES_ROOT, node)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def __cacheFaultListener(self, e):"},{"line_number":74,"context_line":"        self.log.exception(e)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def getRegisteredLaunchers(self):"},{"line_number":77,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":43,"id":"9f560f44_591e8d5f","line":74,"updated":"2020-10-01 08:42:35.000000000","message":"Is there any reason  to use name mangling for this and the above methods (e.g. name clashes in subclasses)? Otherwise the convention is to just use one underscore for internal methods/attributes.","commit_id":"24bfb3e9ebd27f87b0441ef1e4d60a048c1bc1d5"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"11cafd8f1365320e8e0018e7b080ca227559815a","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        return \"%s/%s\" % (self.NODES_ROOT, node)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def __cacheFaultListener(self, e):"},{"line_number":74,"context_line":"        self.log.exception(e)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def getRegisteredLaunchers(self):"},{"line_number":77,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":43,"id":"9f560f44_19489588","line":74,"in_reply_to":"9f560f44_591e8d5f","updated":"2020-10-01 09:42:01.000000000","message":"Done","commit_id":"24bfb3e9ebd27f87b0441ef1e4d60a048c1bc1d5"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"561cb9fd2ffdc722d0974dcfa75672fbbf05804e","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        except NoNodeError:"},{"line_number":110,"context_line":"            return []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _getNode(self, node):"},{"line_number":113,"context_line":"        \"\"\""},{"line_number":114,"context_line":"        Get the data for a specific node."},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_6912cdb3","line":112,"updated":"2020-10-21 13:20:57.000000000","message":"nit: Should we rename the parameter to `node_id` (like the comment says) just to make things a little clearer?","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"49b2e7d6f3e760cad9fb19c6202a880f420ae46c","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        except NoNodeError:"},{"line_number":110,"context_line":"            return []"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def _getNode(self, node):"},{"line_number":113,"context_line":"        \"\"\""},{"line_number":114,"context_line":"        Get the data for a specific node."},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_e7d26834","line":112,"in_reply_to":"3f65232a_6912cdb3","updated":"2020-10-23 07:13:33.000000000","message":"I guess this can be renamed. Original: https://review.opendev.org/c/717269/53/zuul/zk.py.\n\nIn that case I would do it in separate change, same way as typing improvements.","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"561cb9fd2ffdc722d0974dcfa75672fbbf05804e","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        request.lock.release()"},{"line_number":310,"context_line":"        request.lock \u003d None"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    def _holdRequestCacheListener(self, event):"},{"line_number":313,"context_line":"        \"\"\""},{"line_number":314,"context_line":"        Keep the hold request object cache in sync with the TreeCache."},{"line_number":315,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_1b027cb5","line":312,"updated":"2020-10-21 13:20:57.000000000","message":"Just for clarification: This method is called whenever a hold request is updated in ZooKeeper to also update the corresponding object in the cache?","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"49b2e7d6f3e760cad9fb19c6202a880f420ae46c","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        request.lock.release()"},{"line_number":310,"context_line":"        request.lock \u003d None"},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    def _holdRequestCacheListener(self, event):"},{"line_number":313,"context_line":"        \"\"\""},{"line_number":314,"context_line":"        Keep the hold request object cache in sync with the TreeCache."},{"line_number":315,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_87745437","line":312,"in_reply_to":"3f65232a_1b027cb5","updated":"2020-10-23 07:13:33.000000000","message":"Yes (line: 60)","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"561cb9fd2ffdc722d0974dcfa75672fbbf05804e","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                return"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"            path \u003d event.event_data.path"},{"line_number":329,"context_line":"            request_id \u003d path.rsplit(\u0027/\u0027, 1)[1]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            if event.event_type in ("},{"line_number":332,"context_line":"                    TreeEvent.NODE_ADDED, TreeEvent.NODE_UPDATED):"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_5b835457","line":329,"updated":"2020-10-21 13:20:57.000000000","message":"This line might fail if the event_data doesn\u0027t provide a path attribute (like checked in line 317). To avoid this and to simplify things a little we could store the path in a variable once and return if it\u0027s None (or raise an Exception if that\u0027s appropriate). We could then also remove line 328.\n\nSomething like this:\n\npath \u003d None\nif hasattr(event.event_data, \u0027path\u0027):\n    # Ignore root node\n    path \u003d event.event_data.path\n\nif path \u003d\u003d self.HOLD_REQUEST_ROOT:\n    return\n\nif event.event_type not in...\n    return\n\nif path is None:\n    return\n\n...","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"49b2e7d6f3e760cad9fb19c6202a880f420ae46c","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                return"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"            path \u003d event.event_data.path"},{"line_number":329,"context_line":"            request_id \u003d path.rsplit(\u0027/\u0027, 1)[1]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            if event.event_type in ("},{"line_number":332,"context_line":"                    TreeEvent.NODE_ADDED, TreeEvent.NODE_UPDATED):"}],"source_content_type":"text/x-python","patch_set":53,"id":"3f65232a_07896422","line":329,"in_reply_to":"3f65232a_5b835457","updated":"2020-10-23 07:13:33.000000000","message":"I guess event types \"NODE_ADDED\", \"NODE_UPDATED\" and \"NODE_REMOVED\" always provide a path. The original implementation is the same. The only change is that the methods \"holdRequestCacheListener\" and \"_holdRequestCacheListener\" were merged together. Check https://review.opendev.org/c/717269/53/zuul/zk.py\n\nAlthough this file was renamed from \"zk.py\" and git is showing that, gerrit is marking it as \"new\" and \"zk.py\" as deleted.","commit_id":"b8f24e227705f60d89e82435b126c81b674d2f11"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"f5ea553caea854ae6cc2169fafdd3f629c13f21b","unresolved":false,"context_lines":[{"line_number":360,"context_line":"                \"Exception in hold request cache update for event: %s\", event)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"    def submitNodeRequest(self, node_request, watcher):"},{"line_number":363,"context_line":"        \"\"\""},{"line_number":364,"context_line":"        Submit a request for nodes to Nodepool."},{"line_number":365,"context_line":""},{"line_number":366,"context_line":"        :param NodeRequest node_request: A NodeRequest with the"}],"source_content_type":"text/x-python","patch_set":59,"id":"68357f52_81273abd","line":363,"updated":"2021-02-05 17:56:18.000000000","message":"nit: Probably didn\u0027t need to change all of the quotes in this change.  It\u0027s got enough going on already.  :)","commit_id":"b0739b528a1057e698adbd190886a9bd70812e83"}],"zuul/zk/zuul.py":[{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"1c75eb2a0a3fd7bafed963f8683b791a2a1876d8","unresolved":false,"context_lines":[{"line_number":23,"context_line":"                               [self.ZUUL_CONFIG_ROOT] + list(args)))"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"},{"line_number":26,"context_line":"        if TYPE_CHECKING:  # IDE type checking support"},{"line_number":27,"context_line":"            from zuul.zk import ZooKeeper"},{"line_number":28,"context_line":"            assert isinstance(self, ZooKeeper)"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9f560f44_1803fd11","line":26,"updated":"2020-09-08 19:51:22.000000000","message":"I\u0027m not a huge fan of adding such code for IDE type checking support. Perhaps this is a symptom of using a Mixin, would it be more appropriate to use functions instead and replace self by a zuul.zk.ZooKeeper instance?","commit_id":"1af48464ac1cbde986efce3d092b975a48232ab0"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"a9275436c1edcb178d6f02a827eef5073024a978","unresolved":false,"context_lines":[{"line_number":23,"context_line":"                               [self.ZUUL_CONFIG_ROOT] + list(args)))"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"},{"line_number":26,"context_line":"        if TYPE_CHECKING:  # IDE type checking support"},{"line_number":27,"context_line":"            from zuul.zk import ZooKeeper"},{"line_number":28,"context_line":"            assert isinstance(self, ZooKeeper)"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9f560f44_fbbb82c0","line":26,"in_reply_to":"9f560f44_1803fd11","updated":"2020-09-10 08:07:50.000000000","message":"Yes, thats because of the mixin approach. I was trying the function approach too. But did not like the result either. I am open to suggestions. The goal here was to break up the ZooKeeper God-Class. This will become even a larger one due to the focus on Zookeeper as a central component","commit_id":"1af48464ac1cbde986efce3d092b975a48232ab0"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"87fbcff52c80facc233b01490a26e8e9c84f43d2","unresolved":false,"context_lines":[{"line_number":23,"context_line":"                               [self.ZUUL_CONFIG_ROOT] + list(args)))"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"},{"line_number":26,"context_line":"        if TYPE_CHECKING:  # IDE type checking support"},{"line_number":27,"context_line":"            from zuul.zk import ZooKeeper"},{"line_number":28,"context_line":"            assert isinstance(self, ZooKeeper)"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9f560f44_fda412bb","line":26,"in_reply_to":"9f560f44_fbbb82c0","updated":"2020-09-10 12:46:17.000000000","message":"IIUC PS28, the current design is:\n\n\n               BaseClient\n                 /    \\\n                /      \\\n             Nodepool  Zuul\n                \\       /\n                 \\     /\n                   Zk\n\n\nI find it difficult to follow the state where all the methods and internal state are shared amongs these object. As an improvement, I suggest this design:\n\n    Zk:\n      client \u003d BaseClient()\n      nodepool \u003d Nodepool(client)\n      zuul \u003d Zuul(client)\n\nWhere the client provides a higher level interface of the kazooclient with its internal state hidden from Nodepool and Zuul. The downside is that call site needs to be changed from `zk.getHoldRequest` to `zk.nodepool.getHoldRequest`.","commit_id":"1af48464ac1cbde986efce3d092b975a48232ab0"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"ed2cc5251efac1c86667decb8d8bd7c21897b18d","unresolved":false,"context_lines":[{"line_number":23,"context_line":"                               [self.ZUUL_CONFIG_ROOT] + list(args)))"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def _getConfigPartContent(self, parent, child) -\u003e str:"},{"line_number":26,"context_line":"        if TYPE_CHECKING:  # IDE type checking support"},{"line_number":27,"context_line":"            from zuul.zk import ZooKeeper"},{"line_number":28,"context_line":"            assert isinstance(self, ZooKeeper)"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9f560f44_072322a9","line":26,"in_reply_to":"9f560f44_fda412bb","updated":"2020-09-30 10:18:04.000000000","message":"I like your suggesstion also more: I changed that.","commit_id":"1af48464ac1cbde986efce3d092b975a48232ab0"}]}