)]}'
{"nodepool/builder.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"cdcb96fd4382f6398d0b8fbaa04cf458b19f8aaf","unresolved":true,"context_lines":[{"line_number":1405,"context_line":"            self.zk_client.connect()"},{"line_number":1406,"context_line":"            self.zk \u003d zk.ZooKeeper(self.zk_client, enable_cache\u003dFalse)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"            hostname \u003d socket.gethostname()"},{"line_number":1409,"context_line":"            self.component_info \u003d BuilderComponent("},{"line_number":1410,"context_line":"                self.zk_client, hostname,"},{"line_number":1411,"context_line":"                version\u003dget_version_string())"}],"source_content_type":"text/x-python","patch_set":5,"id":"7b15d2da_bdb419ef","line":1408,"updated":"2022-05-23 14:40:50.000000000","message":"I think we should put this into a method and call it here. There are other parts of the builder (BaseWorker) that also uses its hostname but use socket.getfqdn().","commit_id":"56d532364399893b846d45cad5cbbe47bd2e40b2"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"7122f0a79a7adfe7dc6844ee04395465152c8e26","unresolved":false,"context_lines":[{"line_number":1405,"context_line":"            self.zk_client.connect()"},{"line_number":1406,"context_line":"            self.zk \u003d zk.ZooKeeper(self.zk_client, enable_cache\u003dFalse)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"            hostname \u003d socket.gethostname()"},{"line_number":1409,"context_line":"            self.component_info \u003d BuilderComponent("},{"line_number":1410,"context_line":"                self.zk_client, hostname,"},{"line_number":1411,"context_line":"                version\u003dget_version_string())"}],"source_content_type":"text/x-python","patch_set":5,"id":"320428e8_37c310bf","line":1408,"updated":"2022-05-23 14:51:16.000000000","message":"I\u0027m trying to keep the hostname behavior we get with zuul, so the component registry behaves the same way.  So I\u0027d like to either keep this as is, or if we want to switch this to fqdn, then switch it in zuul as well.\n\nI think hostname (which may be appreviated, but isn\u0027t always) is okay for this use case.","commit_id":"56d532364399893b846d45cad5cbbe47bd2e40b2"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"23fe7cb0d12a1125bc2fa76fca1fcc537a197701","unresolved":true,"context_lines":[{"line_number":1405,"context_line":"            self.zk_client.connect()"},{"line_number":1406,"context_line":"            self.zk \u003d zk.ZooKeeper(self.zk_client, enable_cache\u003dFalse)"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"            hostname \u003d socket.gethostname()"},{"line_number":1409,"context_line":"            self.component_info \u003d BuilderComponent("},{"line_number":1410,"context_line":"                self.zk_client, hostname,"},{"line_number":1411,"context_line":"                version\u003dget_version_string())"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a893ff4_4f538c88","line":1408,"in_reply_to":"7b15d2da_bdb419ef","updated":"2022-05-23 14:50:12.000000000","message":"Discussed in chat: since this is not used as identity it\u0027s fine to keep it as is and cleanup this at a later point.","commit_id":"56d532364399893b846d45cad5cbbe47bd2e40b2"}],"nodepool/driver/__init__.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e5b5f888b745ab532cd298541e8b4a2782b44d99","unresolved":false,"context_lines":[{"line_number":641,"context_line":"        # want to make sure we don\u0027t continuously grow this array."},{"line_number":642,"context_line":"        if self.launcher_id not in self.request.declined_by:"},{"line_number":643,"context_line":"            self.request.declined_by.append(self.launcher_id)"},{"line_number":644,"context_line":"        launchers \u003d set([x.id for x in self.zk.getRegisteredPools()])"},{"line_number":645,"context_line":"        if launchers.issubset(set(self.request.declined_by)):"},{"line_number":646,"context_line":"            # All launchers have declined it"},{"line_number":647,"context_line":"            self.log.debug(\"Failing declined node request\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"a363ec57_3f9a1150","line":644,"updated":"2022-05-23 18:49:10.000000000","message":"I thought about that, but there is so much code that calls these things launchers that I think it\u0027s easier to go with this for now.\n\nIt would be easier to have just called the pools launches as the component, but I\u0027m trying to set us up for being able to rectify this eventually.\n\nAnyway, long story short -- yes, but I think I see that as a longer-term change.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":641,"context_line":"        # want to make sure we don\u0027t continuously grow this array."},{"line_number":642,"context_line":"        if self.launcher_id not in self.request.declined_by:"},{"line_number":643,"context_line":"            self.request.declined_by.append(self.launcher_id)"},{"line_number":644,"context_line":"        launchers \u003d set([x.id for x in self.zk.getRegisteredPools()])"},{"line_number":645,"context_line":"        if launchers.issubset(set(self.request.declined_by)):"},{"line_number":646,"context_line":"            # All launchers have declined it"},{"line_number":647,"context_line":"            self.log.debug(\"Failing declined node request\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"da091989_628d99b5","line":644,"updated":"2022-05-23 18:19:24.000000000","message":"Nit might want to call these pools now? There is a launcher component instead. It might be confusing to read this later as a result.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"}],"nodepool/launcher.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"cdcb96fd4382f6398d0b8fbaa04cf458b19f8aaf","unresolved":true,"context_lines":[{"line_number":351,"context_line":"        self.running \u003d True"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        # Make sure we\u0027re always registered with ZK"},{"line_number":354,"context_line":"        hostname \u003d socket.gethostname()"},{"line_number":355,"context_line":"        self.component_info \u003d PoolComponent("},{"line_number":356,"context_line":"            self.zk.zk_client, hostname,"},{"line_number":357,"context_line":"            version\u003dget_version_string())"}],"source_content_type":"text/x-python","patch_set":5,"id":"cb5a9974_06ce11ba","line":354,"updated":"2022-05-23 14:40:50.000000000","message":"Same here (hostname vs fqdn used by the launcher_id), but I\u0027m on the fence on this one.","commit_id":"56d532364399893b846d45cad5cbbe47bd2e40b2"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":110,"context_line":"        # become out of date as the loop progresses, but it should be"},{"line_number":111,"context_line":"        # good enough to determine whether we should process requests"},{"line_number":112,"context_line":"        # which express a preference for a specific provider."},{"line_number":113,"context_line":"        launchers \u003d self.zk.getRegisteredPools()"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        pm \u003d self.getProviderManager()"},{"line_number":116,"context_line":"        has_quota_support \u003d isinstance(pm, QuotaSupport)"}],"source_content_type":"text/x-python","patch_set":6,"id":"cf082faa_1a1e4c69","line":113,"updated":"2022-05-23 18:19:24.000000000","message":"Nit might want to call these pools now? There is a launcher component instead. It might be confusing to read this later as a result.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"}],"nodepool/stats.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        states \u003d {}"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        launchers \u003d zk_conn.getRegisteredPools()"},{"line_number":102,"context_line":"        labels \u003d set()"},{"line_number":103,"context_line":"        for launcher in launchers:"},{"line_number":104,"context_line":"            labels.update(launcher.supported_labels)"}],"source_content_type":"text/x-python","patch_set":6,"id":"edc98b04_8c0b1126","line":101,"updated":"2022-05-23 18:19:24.000000000","message":"See other commments about confusion over launchers vs pools.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"}],"nodepool/status.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":267,"context_line":"    # NOTE(ianw): maybe add to each entry a list of which"},{"line_number":268,"context_line":"    #             launchers support the label?"},{"line_number":269,"context_line":"    labels \u003d set()"},{"line_number":270,"context_line":"    launchers \u003d zk.getRegisteredPools()"},{"line_number":271,"context_line":"    for launcher in launchers:"},{"line_number":272,"context_line":"        labels.update(set(launcher.supported_labels))"},{"line_number":273,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"70d4a49e_e70b61fe","line":270,"updated":"2022-05-23 18:19:24.000000000","message":"And here.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"}],"nodepool/zk/components.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"class LauncherComponent(BaseComponent):"},{"line_number":200,"context_line":"    kind \u003d \"launcher\""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"class BuilderComponent(BaseComponent):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3d9bcd31_16c6bdc3","line":200,"updated":"2022-05-23 18:19:24.000000000","message":"As noted elsewhere in my review I think having pools treated as launchers when we have a separate launcher component (this one) is confusing. Is this component even used? We should probably try to be more consistent to avoid confusion later.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e5b5f888b745ab532cd298541e8b4a2782b44d99","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"class LauncherComponent(BaseComponent):"},{"line_number":200,"context_line":"    kind \u003d \"launcher\""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"class BuilderComponent(BaseComponent):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9969b6a9_046f54a9","line":200,"updated":"2022-05-23 18:49:10.000000000","message":"This isn\u0027t necessary for this change, nor is the builder.  I\u0027m trying to set us up for being able to actually have a view of the whole system like we do with Zuul.\n\nNodepool itself only needs to care about the pool workers (which have been misleadingly called \"launchers\" up to this point).  If we want to have all the procesess represented, then we need to have something registered for the \"nodepool-launcher\" command.  I think *that* should be called a launcher.  So yes, this change starts calling the pool workers within the launchers pools, but doesn\u0027t change all of the code in nodepool to use different variable names.\n\nI am trying to set us up for avoiding confusion later.  The best way I know to do that is to call launchers launchers, and pools pools.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e5b5f888b745ab532cd298541e8b4a2782b44d99","unresolved":false,"context_lines":[{"line_number":234,"context_line":"    disconnects from ZK, the node will disappear."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    Component objects returned by this class are read-only; updating"},{"line_number":237,"context_line":"    their attributes will not be reflected in ZooKeeper."},{"line_number":238,"context_line":"    \"\"\""},{"line_number":239,"context_line":"    log \u003d logging.getLogger(\"nodepool.ComponentRegistry\")"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"e6c0d097_cc22bc7e","line":237,"updated":"2022-05-23 18:49:10.000000000","message":"Line 327.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":234,"context_line":"    disconnects from ZK, the node will disappear."},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    Component objects returned by this class are read-only; updating"},{"line_number":237,"context_line":"    their attributes will not be reflected in ZooKeeper."},{"line_number":238,"context_line":"    \"\"\""},{"line_number":239,"context_line":"    log \u003d logging.getLogger(\"nodepool.ComponentRegistry\")"},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ece45462_e4a0af88","line":237,"updated":"2022-05-23 18:19:24.000000000","message":"What determines this? It seems that BaseComponent.__setattr__ doesn\u0027t check/prevent updates.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":294,"context_line":"            return self._onComponentUpdate(kind, hostname, data, stat, event)"},{"line_number":295,"context_line":"        return watch"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def _onComponentUpdate(self, kind, hostname, data, stat, event):"},{"line_number":298,"context_line":"        if event:"},{"line_number":299,"context_line":"            etype \u003d event.type"},{"line_number":300,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"a33ef1a0_b90aa290","line":297,"range":{"start_line":297,"start_character":39,"end_line":297,"end_character":47},"updated":"2022-05-23 18:19:24.000000000","message":"Does this include the sequence identifier? It must because we don\u0027t seem to handle a sequence otherwise.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e5b5f888b745ab532cd298541e8b4a2782b44d99","unresolved":false,"context_lines":[{"line_number":294,"context_line":"            return self._onComponentUpdate(kind, hostname, data, stat, event)"},{"line_number":295,"context_line":"        return watch"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def _onComponentUpdate(self, kind, hostname, data, stat, event):"},{"line_number":298,"context_line":"        if event:"},{"line_number":299,"context_line":"            etype \u003d event.type"},{"line_number":300,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa179047_6b9e55de","line":297,"updated":"2022-05-23 18:49:10.000000000","message":"The \"hostname\" is the ZK path component which includes the sequence id.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cdd0ee408110724af329687b97a5d52ca0400471","unresolved":true,"context_lines":[{"line_number":394,"context_line":"                sys.exit(1)"},{"line_number":395,"context_line":"        else:"},{"line_number":396,"context_line":"            if version \u003e self.model_api:"},{"line_number":397,"context_line":"                if version \u003e MODEL_API:"},{"line_number":398,"context_line":"                    self.log.info(\"The data model version of this component \""},{"line_number":399,"context_line":"                                  \"is older than other components in the \""},{"line_number":400,"context_line":"                                  \"system, so other components will operate \""}],"source_content_type":"text/x-python","patch_set":6,"id":"b10cc96d_408d0573","line":397,"updated":"2022-05-23 18:19:24.000000000","message":"I feel like we discussed this when zuul added model api versioning, but I find this particular case to be confusing. If on startup we hit this condition we exit(1) to prevent data corruption. But if it occurs after we\u0027ve set self.model_api we\u0027ll fast forward our local version ahead of our MODEL_API after we hit this case? That doesn\u0027t seem correct.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"e5b5f888b745ab532cd298541e8b4a2782b44d99","unresolved":false,"context_lines":[{"line_number":394,"context_line":"                sys.exit(1)"},{"line_number":395,"context_line":"        else:"},{"line_number":396,"context_line":"            if version \u003e self.model_api:"},{"line_number":397,"context_line":"                if version \u003e MODEL_API:"},{"line_number":398,"context_line":"                    self.log.info(\"The data model version of this component \""},{"line_number":399,"context_line":"                                  \"is older than other components in the \""},{"line_number":400,"context_line":"                                  \"system, so other components will operate \""}],"source_content_type":"text/x-python","patch_set":6,"id":"b409343c_55cd51d9","line":397,"updated":"2022-05-23 18:49:10.000000000","message":"This is copied from Zuul to achieve parity.\n\nThis could only happen if our registration was somehow dropped (since if our version is lower than the rest, we should be the ones setting the minimum).\n\nIf we\u0027re having trouble with ZK such that this happens, it\u0027s arguable whether it\u0027s better to just resume as before or exit because someone might have been tricked into doing an upgrade that needs to be rolled back.\n\nMy inclination is to avoid exiting here because it could lead to a minor ZK hiccup causing the whole system to exit.  But I don\u0027t think there\u0027s a clear right answer.","commit_id":"a612aa603c5c6935dc1c3617a18f81b3004d192d"}]}