)]}'
{"zuul/cmd/client.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b3612cc6419d139689cb1a6fc8d31525d9fdf8b9","unresolved":false,"context_lines":[{"line_number":965,"context_line":"        safe_pipeline \u003d urllib.parse.quote_plus(args.pipeline)"},{"line_number":966,"context_line":"        # Create a component registry to register it with the global"},{"line_number":967,"context_line":"        # singleton."},{"line_number":968,"context_line":"        ComponentRegistry(zk_client)"},{"line_number":969,"context_line":"        with tenant_write_lock(zk_client, args.tenant) as lock:"},{"line_number":970,"context_line":"            path \u003d f\u0027/zuul/tenant/{safe_tenant}/pipeline/{safe_pipeline}\u0027"},{"line_number":971,"context_line":"            layout_uuid \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"5e7c5731_c02ffba4","line":968,"updated":"2022-02-14 18:58:29.000000000","message":"I\u0027ve revised this to add a create method.","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"9db558dd52b7698d516cabba449275ce396ca3d3","unresolved":true,"context_lines":[{"line_number":965,"context_line":"        safe_pipeline \u003d urllib.parse.quote_plus(args.pipeline)"},{"line_number":966,"context_line":"        # Create a component registry to register it with the global"},{"line_number":967,"context_line":"        # singleton."},{"line_number":968,"context_line":"        ComponentRegistry(zk_client)"},{"line_number":969,"context_line":"        with tenant_write_lock(zk_client, args.tenant) as lock:"},{"line_number":970,"context_line":"            path \u003d f\u0027/zuul/tenant/{safe_tenant}/pipeline/{safe_pipeline}\u0027"},{"line_number":971,"context_line":"            layout_uuid \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"054f5cfa_6c80802e","line":968,"updated":"2022-02-10 23:28:56.000000000","message":"If there can only be one component registry, shouldn\u0027t we use a `COMPONENT_REGISTRY.create(zk_client)` method instead?","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4d41367b05c45924c79c48030b33df51ddf6f6d4","unresolved":false,"context_lines":[{"line_number":965,"context_line":"        safe_pipeline \u003d urllib.parse.quote_plus(args.pipeline)"},{"line_number":966,"context_line":"        # Create a component registry to register it with the global"},{"line_number":967,"context_line":"        # singleton."},{"line_number":968,"context_line":"        ComponentRegistry(zk_client)"},{"line_number":969,"context_line":"        with tenant_write_lock(zk_client, args.tenant) as lock:"},{"line_number":970,"context_line":"            path \u003d f\u0027/zuul/tenant/{safe_tenant}/pipeline/{safe_pipeline}\u0027"},{"line_number":971,"context_line":"            layout_uuid \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ca089a15_d567b488","line":968,"updated":"2022-02-11 21:57:46.000000000","message":"There can be more than one (and there may be more than one now).  It\u0027s harmless -- they\u0027ll see the same data.  With this change, there probably doesn\u0027t need to be, so I think we could enhance this to work as you suggest.","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"}],"zuul/zk/components.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4d41367b05c45924c79c48030b33df51ddf6f6d4","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setRegistry(self, registry):"},{"line_number":35,"context_line":"        if self.registry is None:"},{"line_number":36,"context_line":"            self.registry \u003d registry"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def clearRegistry(self, registry):"},{"line_number":39,"context_line":"        self.registry \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"b339408d_d6cd32f2","line":36,"updated":"2022-02-11 21:57:46.000000000","message":"As above, it\u0027s harmless to have more than one and the current code doesn\u0027t prohibit it.","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b3612cc6419d139689cb1a6fc8d31525d9fdf8b9","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setRegistry(self, registry):"},{"line_number":35,"context_line":"        if self.registry is None:"},{"line_number":36,"context_line":"            self.registry \u003d registry"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def clearRegistry(self, registry):"},{"line_number":39,"context_line":"        self.registry \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"c52cb55c_04d7c0a7","line":36,"updated":"2022-02-14 18:58:29.000000000","message":"I\u0027ve revised this to be a create method.  Normally that shouldn\u0027t be called more than once, but it will be in the tests because we run multiple components in the same process.  So it doesn\u0027t raise an exception if it\u0027s called twice (but it also doesn\u0027t create a second registry).","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"},{"author":{"_account_id":9311,"name":"Tristan Cacqueray","email":"tdecacqu@redhat.com","username":"tristanC"},"change_message_id":"9db558dd52b7698d516cabba449275ce396ca3d3","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def setRegistry(self, registry):"},{"line_number":35,"context_line":"        if self.registry is None:"},{"line_number":36,"context_line":"            self.registry \u003d registry"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def clearRegistry(self, registry):"},{"line_number":39,"context_line":"        self.registry \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"32c07f5a_5d1f8b94","line":36,"updated":"2022-02-10 23:28:56.000000000","message":"Shouldn\u0027t this raise an exception if there is already a registry registered?","commit_id":"1f6efed1247853c85ba92983dfa1abc4be88ee2b"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"80f7240c31be8315fd2bff28881cd7e88a1f22cb","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        return self.registry.model_api"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"COMPONENT_REGISTRY \u003d GlobalRegistry()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"class BaseComponent(ZooKeeperBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"f7441317_e31a4e1e","line":47,"updated":"2022-02-17 01:15:17.000000000","message":"I\u0027m intrigued by the idea because I also dislike module global variables.  But one of the goals here is to make this work even if we don\u0027t have a zk_client.  Example use case: https://review.opendev.org/827935  where if we were to use this, we would change the serialization of the changekey reference based on the cluster model api version.  But the ChangeKey class doesn\u0027t interact with zk directly.\n\nWe could decide that\u0027s a bad idea and drop that goal (and/or pass zk_client around to more objects, but that\u0027s about the same as passing around the zkcontext objects this removes).\n\nOr we could merge the two and use my approach with your class method.\n\nBut really at the end of the day, it\u0027s still a global variable no matter which way we go right?  It\u0027s just called COMPONENT_REGISTRY in one version and ComponentRegistry._registries in the other?  Or is there another advantage to storring the registry as a class attribute?\n\n(Also, I\u0027m assuming you meant \u0027cls._registries\u0027 in your example instead of \u0027self._registries\u0027.)","commit_id":"864a2b7701f47c82dfda218538fd58970896ec2a"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"75fce744adeefa821c036c1885fc267878d8ace3","unresolved":true,"context_lines":[{"line_number":44,"context_line":"        return self.registry.model_api"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"COMPONENT_REGISTRY \u003d GlobalRegistry()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"class BaseComponent(ZooKeeperBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa3a5fa6_8ac186f4","line":47,"updated":"2022-02-16 13:18:25.000000000","message":"Just wondering if instead of using a module global registry we could make the ComponentRegistry a singleton based on the passed zk client instance:\n\n  class ComponentRegistry:\n\n      _registries \u003d {}\n\n      @classmethod\n      def create(cls, zk_client):\n          zk_id \u003d id(zk_client)\n          if zk_id not in self._registries:\n              self._registries[zk_id] \u003d ComponentRegistry(zk_client)\n          return self._registries[zk_id]\n\nThen in other parts of the code we could get the component registry with:\n\n  component_registry \u003d ComponentRegistry.create(zk_client)\n\nIt\u0027s mainly my (unfounded?) distaste for module global objects that makes me suggest that, but I\u0027m also fine if you go ahead with this approach.","commit_id":"864a2b7701f47c82dfda218538fd58970896ec2a"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"20ae1e745aa03f1c20bcdada64d6cdd6d0619985","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        return self.registry.model_api"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"COMPONENT_REGISTRY \u003d GlobalRegistry()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"class BaseComponent(ZooKeeperBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"81dbc83c_be300ee9","line":47,"in_reply_to":"f7441317_e31a4e1e","updated":"2022-02-17 17:56:12.000000000","message":"Yes, it would still be a global state, so fine for me to go ahead with this approach. Yep, that should be `cls._registries`.","commit_id":"864a2b7701f47c82dfda218538fd58970896ec2a"}]}