)]}'
{"zuul/zk/components.py":[{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"4f7c017d80924d125ea81d1ff17efb976d9242bf","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    PAUSED \u003d 2"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class ZooKeeperComponentReadOnly(object):"},{"line_number":32,"context_line":"    def __init__(self, client: ZooKeeperClient, content: Dict[str, Any]):"},{"line_number":33,"context_line":"        self._client \u003d client"},{"line_number":34,"context_line":"        self._content: Dict[str, Any] \u003d content"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_cd879cd7","line":31,"updated":"2020-10-22 11:13:25.000000000","message":"Do you have a use case in mind for a read-only ZooKeeperComponent or is this just here to make things easier extendable?","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"d5fc2b98932b2a056990fa003b729a0ea7cfcf25","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    PAUSED \u003d 2"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class ZooKeeperComponentReadOnly(object):"},{"line_number":32,"context_line":"    def __init__(self, client: ZooKeeperClient, content: Dict[str, Any]):"},{"line_number":33,"context_line":"        self._client \u003d client"},{"line_number":34,"context_line":"        self._content: Dict[str, Any] \u003d content"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_94e62b16","line":31,"in_reply_to":"3f65232a_cd879cd7","updated":"2020-10-25 20:12:57.000000000","message":"Yes, the ZooKeeperComponentRegistry#all will return ZooKeeperComponentReadOnly. The reason is that components can read other components but only the component which called the ZooKeeperComponentRegistry#register method will get a r/w access. This makes it easier to cache values (one does not have to check what is the state in ZK) and enables better prevention that a component changes attributes which it does not own.","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"4f7c017d80924d125ea81d1ff17efb976d9242bf","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self._path \u003d self._register(content)"},{"line_number":53,"context_line":"        self._set_lock: threading.Lock \u003d threading.Lock()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def __setitem__(self, key: str, value: Any) -\u003e None:"},{"line_number":56,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, int):"},{"line_number":57,"context_line":"            value \u003d ZooKeeperComponentState(value).name"},{"line_number":58,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, ZooKeeperComponentState):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_2d3e1029","line":55,"updated":"2020-10-22 11:13:25.000000000","message":"Not sure if I like the usage of __setitem__ here. It makes using the ZooKeeperComponent a little misleading IMHO since it looks like a dict but doesn\u0027t work like a dict in all occasions (e.g. when using len() or an iterator).\n\nI like the idea of directly updating the value of the znode when an attribute changes, but I think this could also be achieved by defining custom methods like `get()` and `set()` which don\u0027t imply any dictionary magic. IMHO this would make the usage of this class a little more explicit.\n\nI don\u0027t see this as a blocker and leave this comment up for discussion.","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"3ee9e6091494e1b913ea89bd1a6f7865ef24bbee","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self._path \u003d self._register(content)"},{"line_number":53,"context_line":"        self._set_lock: threading.Lock \u003d threading.Lock()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def __setitem__(self, key: str, value: Any) -\u003e None:"},{"line_number":56,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, int):"},{"line_number":57,"context_line":"            value \u003d ZooKeeperComponentState(value).name"},{"line_number":58,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, ZooKeeperComponentState):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_47cd1c5c","line":55,"in_reply_to":"3f65232a_2d3e1029","updated":"2020-10-23 07:19:58.000000000","message":"Another idea would be to implement also other methods like __len__ and __iter__ to make it also work like a dict (see https://docs.python.org/3.8/library/collections.abc.html#collections-abstract-base-classes)","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"d5fc2b98932b2a056990fa003b729a0ea7cfcf25","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        self._path \u003d self._register(content)"},{"line_number":53,"context_line":"        self._set_lock: threading.Lock \u003d threading.Lock()"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def __setitem__(self, key: str, value: Any) -\u003e None:"},{"line_number":56,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, int):"},{"line_number":57,"context_line":"            value \u003d ZooKeeperComponentState(value).name"},{"line_number":58,"context_line":"        if key \u003d\u003d \u0027state\u0027 and isinstance(value, ZooKeeperComponentState):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_74e38f06","line":55,"in_reply_to":"3f65232a_47cd1c5c","updated":"2020-10-25 20:12:57.000000000","message":"Done. Changed to set/get methods.","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":27952,"name":"Felix Edel","email":"felix.edel@bmw.de","username":"felix.schmidt"},"change_message_id":"4f7c017d80924d125ea81d1ff17efb976d9242bf","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        \"\"\""},{"line_number":110,"context_line":"        return ZooKeeperComponent(self.client, component, hostname, dict("},{"line_number":111,"context_line":"            hostname\u003dhostname,"},{"line_number":112,"context_line":"            state\u003dZooKeeperComponentState.STOPPED.name,"},{"line_number":113,"context_line":"        ))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_cd04bc7e","line":112,"updated":"2020-10-22 11:13:25.000000000","message":"Can\u0027t we directly use the ZooKeeperComponenState.STOPPED here? I think if we get rid of the `.name` here we could also remove the if/else branch in the __setitem__ method checking if either the enum value or the name is provided.","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":30637,"name":"Jan Kubovy","email":"jan.kubovy@bmw.de","username":"kubovy"},"change_message_id":"d5fc2b98932b2a056990fa003b729a0ea7cfcf25","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        \"\"\""},{"line_number":110,"context_line":"        return ZooKeeperComponent(self.client, component, hostname, dict("},{"line_number":111,"context_line":"            hostname\u003dhostname,"},{"line_number":112,"context_line":"            state\u003dZooKeeperComponentState.STOPPED.name,"},{"line_number":113,"context_line":"        ))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f65232a_14bcdbda","line":112,"in_reply_to":"3f65232a_cd04bc7e","updated":"2020-10-25 20:12:57.000000000","message":"This is going to be serialized to json and store in the zk node. Thats why the \".name\" is needed here. The if/else in line 56 enables to use different representations of ZooKeeperComponentState. One may argue that we only need one but I left it open for the developer to decide, based on context which representation he/she wants to use.","commit_id":"5ddafda8346cf412013f3c66fad0f7728a024e05"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"0dc24c6bc076822a44dd1261972f0424534e23da","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            hostname,"},{"line_number":181,"context_line":"            dict("},{"line_number":182,"context_line":"                hostname\u003dhostname,"},{"line_number":183,"context_line":"                state\u003dZooKeeperComponentState.STOPPED.name,"},{"line_number":184,"context_line":"            ),"},{"line_number":185,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":23,"id":"37002a8a_e32896a0","line":183,"updated":"2021-03-10 19:03:45.000000000","message":"The thing about this is that the following code fails:\n\n  component.set(\u0027state\u0027, ZooKeeperComponentState.RUNNING)\n  assert component.get(\u0027state\u0027) \u003d\u003d ZooKeeperComponentState.RUNNING\n\nWhich is pretty counter-intuitive.  But if we retain the existing pattern that we use both for Zuul model objects as well as for ZK interaction with Nodepool, where we use string constants (improved by making them class attributes as, for example, in the Pipeline class), we can do the following:\n\n  component.set(\u0027state\u0027, component.STATE_RUNNING)\n  assert component.get(\u0027state\u0027) \u003d\u003d component.STATE_RUNNING\n\nThat saves extra import statements in all the files that use this.  To accomplish this, we only need to do this:\n\n  class ZooKeeperComponentReadOnly:\n    STATE_RUNNING \u003d \u0027running\u0027\n\nMy preference would be to do that -- it matches the existing code, and is shorter, simpler, and has all the safety of enums (you won\u0027t accidentally set state to \"RUNNNING\" without causing an error) plus the advantage of input\u003doutput.","commit_id":"35743f02159f2553c79a01d2d17c67fb278cd6b6"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"49e601d5187270811d3ed16bccf34cdd475649e4","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            hostname,"},{"line_number":181,"context_line":"            dict("},{"line_number":182,"context_line":"                hostname\u003dhostname,"},{"line_number":183,"context_line":"                state\u003dZooKeeperComponentState.STOPPED.name,"},{"line_number":184,"context_line":"            ),"},{"line_number":185,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":23,"id":"7bc730a6_4f7b6b78","line":183,"in_reply_to":"37002a8a_e32896a0","updated":"2021-03-11 10:09:50.000000000","message":"Done","commit_id":"35743f02159f2553c79a01d2d17c67fb278cd6b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"503cd514071001fcbb800677cdd2faf47c88b07f","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    STOPPED \u003d 0"},{"line_number":30,"context_line":"    RUNNING \u003d 1"},{"line_number":31,"context_line":"    PAUSED \u003d 2"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self, client: ZooKeeperClient, content: Dict[str, Any]):"},{"line_number":34,"context_line":"        self._client \u003d client"}],"source_content_type":"text/x-python","patch_set":25,"id":"42ed8fa1_d165af9f","line":31,"updated":"2021-03-12 21:44:23.000000000","message":"Why don\u0027t we set these values to words so it\u0027s easy to debug?\n\nSTOPPED \u003d \u0027stopped\u0027","commit_id":"a9bf0cb88f0ef17ffe0c2a3bdfbe0761b50d4397"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"947ac0039f08e26f65ca3b0e0378399af94c6813","unresolved":true,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        result \u003d []"},{"line_number":155,"context_line":"        path \u003d \"{}/{}\".format(self.ROOT, kind)"},{"line_number":156,"context_line":"        self.kazoo_client.ensure_path(path)"},{"line_number":157,"context_line":"        for node in self.kazoo_client.get_children(path):"},{"line_number":158,"context_line":"            path \u003d \"{}/{}/{}\".format(self.ROOT, kind, node)"},{"line_number":159,"context_line":"            data, _ \u003d self.kazoo_client.get(path)"}],"source_content_type":"text/x-python","patch_set":26,"id":"c7cc5e49_1e5f3486","line":156,"updated":"2021-03-13 12:13:47.000000000","message":"Instead of always doing a network round trip here I think we should just catch the NoNodeError here.","commit_id":"22935c1177b21a8fa0f8b068cdcbc4de2a608ba7"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"573ba89f91e61418ec6fc1a31463565fefc4f2f1","unresolved":true,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        result \u003d []"},{"line_number":155,"context_line":"        path \u003d \"{}/{}\".format(self.ROOT, kind)"},{"line_number":156,"context_line":"        self.kazoo_client.ensure_path(path)"},{"line_number":157,"context_line":"        for node in self.kazoo_client.get_children(path):"},{"line_number":158,"context_line":"            path \u003d \"{}/{}/{}\".format(self.ROOT, kind, node)"},{"line_number":159,"context_line":"            data, _ \u003d self.kazoo_client.get(path)"}],"source_content_type":"text/x-python","patch_set":26,"id":"bfb73587_7c8597fa","line":156,"in_reply_to":"c7cc5e49_1e5f3486","updated":"2021-03-13 12:18:20.000000000","message":"Implemented in followup: https://review.opendev.org/c/zuul/zuul/+/780412","commit_id":"22935c1177b21a8fa0f8b068cdcbc4de2a608ba7"}]}