)]}'
{"nodepool/driver/example/config.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        self.instance_type \u003d None"},{"line_number":55,"context_line":"        # The ProviderPool object that owns this label."},{"line_number":56,"context_line":"        self.pool \u003d None"},{"line_number":57,"context_line":"        self.host_key_checking \u003d self.pool.host_key_checking"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def __eq__(self, other):"},{"line_number":60,"context_line":"        if isinstance(other, ProviderLabel):"}],"source_content_type":"text/x-python","patch_set":7,"id":"81d2f81f_01356327","line":57,"updated":"2022-11-21 23:39:23.000000000","message":"I know this is an exmaple but self.pool is None per the line above and does not have a host_key_checking attribute. Perhaps we should set self.pool to something a bit more valid here to make a better example?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.instance_type \u003d None"},{"line_number":55,"context_line":"        # The ProviderPool object that owns this label."},{"line_number":56,"context_line":"        self.pool \u003d None"},{"line_number":57,"context_line":"        self.host_key_checking \u003d self.pool.host_key_checking"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def __eq__(self, other):"},{"line_number":60,"context_line":"        if isinstance(other, ProviderLabel):"}],"source_content_type":"text/x-python","patch_set":7,"id":"54cef6f0_cac536d3","line":57,"in_reply_to":"81d2f81f_01356327","updated":"2022-11-22 00:24:38.000000000","message":"Done","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"}],"nodepool/driver/fake/adapter.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":1,"context_line":"# Copyright 2022 Acme Gating, LLC"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"# not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":7,"id":"e4546e16_b535deb5","line":1,"updated":"2022-11-21 23:39:23.000000000","message":"I\u0027m not sure we should wholesale replace the previous (C) line? Seems like a fair bit of this file remains which would continue to make that valid?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2022 Acme Gating, LLC"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"# not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":7,"id":"ed984ffc_174e936a","line":1,"in_reply_to":"e4546e16_b535deb5","updated":"2022-11-22 00:24:38.000000000","message":"You\u0027re right.  This error occurred because I initially started with an empty file before realizing it made more sense for existing code to come over here.  Sorry!","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":295,"context_line":"                    floating_ip_address\u003d\u0027fake\u0027,"},{"line_number":296,"context_line":"                    status\u003d\u0027ACTIVE\u0027)"},{"line_number":297,"context_line":"        self._floating_ip_list.append(fip)"},{"line_number":298,"context_line":"        server.addresses"},{"line_number":299,"context_line":"        return fip"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def _needs_floating_ip(self, server, nat_destination):"}],"source_content_type":"text/x-python","patch_set":7,"id":"286ba92a_ac1608bd","line":298,"updated":"2022-11-21 23:39:23.000000000","message":"I\u0027m not sure how to interpret this line. Seems like it would need to be a function call to side effect. Is it missing ()s? But I don\u0027t think that is it either because Dummy.addresses from create_server above is just a dict.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":295,"context_line":"                    floating_ip_address\u003d\u0027fake\u0027,"},{"line_number":296,"context_line":"                    status\u003d\u0027ACTIVE\u0027)"},{"line_number":297,"context_line":"        self._floating_ip_list.append(fip)"},{"line_number":298,"context_line":"        server.addresses"},{"line_number":299,"context_line":"        return fip"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def _needs_floating_ip(self, server, nat_destination):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5faf927e_251fdd89","line":298,"in_reply_to":"286ba92a_ac1608bd","updated":"2022-11-22 00:24:38.000000000","message":"I think it\u0027s nothing.  Weird.  Seems like something pyflakes would have caught.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"}],"nodepool/driver/openstack/adapter.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":56,"context_line":"    def __init__(self, metadata, type, id):"},{"line_number":57,"context_line":"        super().__init__(metadata)"},{"line_number":58,"context_line":"        self.type \u003d type"},{"line_number":59,"context_line":"        self.id \u003d id"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class OpenStackDeleteStateMachine(statemachine.StateMachine):"}],"source_content_type":"text/x-python","patch_set":7,"id":"ef9b66bd_345179f7","line":59,"updated":"2022-11-21 23:39:23.000000000","message":"Both type and id are python reserved words. I half wonder if we should take this opportunity to use different names here to avoid unexpected collisions? This isn\u0027t a regression though, the old code used the same names and hadn\u0027t collided (yet) so is probably fine.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    def __init__(self, metadata, type, id):"},{"line_number":57,"context_line":"        super().__init__(metadata)"},{"line_number":58,"context_line":"        self.type \u003d type"},{"line_number":59,"context_line":"        self.id \u003d id"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"class OpenStackDeleteStateMachine(statemachine.StateMachine):"}],"source_content_type":"text/x-python","patch_set":7,"id":"7d15939d_61de1f94","line":59,"in_reply_to":"ef9b66bd_345179f7","updated":"2022-11-22 00:24:38.000000000","message":"I agree they are generally worth avoiding.  I think in simple data class constructors like this it\u0027s okay.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        if self.state \u003d\u003d self.FLOATING_IP_DELETING:"},{"line_number":89,"context_line":"            fips \u003d []"},{"line_number":90,"context_line":"            for fip in self.floating_ips:"},{"line_number":91,"context_line":"                print(\u0027check\u0027, fip)"},{"line_number":92,"context_line":"                fip \u003d self.adapter._refreshFloatingIpDelete(fip)"},{"line_number":93,"context_line":"                if not fip or fip[\u0027status\u0027] \u003d\u003d \u0027DOWN\u0027:"},{"line_number":94,"context_line":"                    fip \u003d None"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f0d66a9_24009127","line":91,"updated":"2022-11-22 00:24:38.000000000","message":"Oops.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":90,"context_line":"            for fip in self.floating_ips:"},{"line_number":91,"context_line":"                print(\u0027check\u0027, fip)"},{"line_number":92,"context_line":"                fip \u003d self.adapter._refreshFloatingIpDelete(fip)"},{"line_number":93,"context_line":"                if not fip or fip[\u0027status\u0027] \u003d\u003d \u0027DOWN\u0027:"},{"line_number":94,"context_line":"                    fip \u003d None"},{"line_number":95,"context_line":"                if fip:"},{"line_number":96,"context_line":"                    fips.append(fip)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dd0b54a9_3721f0a8","line":93,"range":{"start_line":93,"start_character":30,"end_line":93,"end_character":53},"updated":"2022-11-21 23:39:23.000000000","message":"The reason we proceed here if the port goes DOWN is that our leaked fip cleanup routine will cleanup DOWN ports so we don\u0027t need to wait further for instance deletion.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            for fip in self.floating_ips:"},{"line_number":91,"context_line":"                print(\u0027check\u0027, fip)"},{"line_number":92,"context_line":"                fip \u003d self.adapter._refreshFloatingIpDelete(fip)"},{"line_number":93,"context_line":"                if not fip or fip[\u0027status\u0027] \u003d\u003d \u0027DOWN\u0027:"},{"line_number":94,"context_line":"                    fip \u003d None"},{"line_number":95,"context_line":"                if fip:"},{"line_number":96,"context_line":"                    fips.append(fip)"}],"source_content_type":"text/x-python","patch_set":7,"id":"6dca7ccc_4678970a","line":93,"range":{"start_line":93,"start_character":30,"end_line":93,"end_character":53},"in_reply_to":"dd0b54a9_3721f0a8","updated":"2022-11-22 00:24:38.000000000","message":"Yep -- there\u0027s nothing else we can do with it at this point.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":180,"context_line":"        # merge in any provided properties"},{"line_number":181,"context_line":"        if props:"},{"line_number":182,"context_line":"            meta.update(props)"},{"line_number":183,"context_line":"        meta.update(metadata)"},{"line_number":184,"context_line":"        self.metadata \u003d meta"},{"line_number":185,"context_line":"        self.flavor \u003d self.adapter._findFlavor("},{"line_number":186,"context_line":"            flavor_name\u003dself.label.flavor_name,"}],"source_content_type":"text/x-python","patch_set":7,"id":"c9a9db6f_83b66038","line":183,"updated":"2022-11-21 23:39:23.000000000","message":"How are props and metadata different here? That might be worth a comment? (note I don\u0027t think this is a regression).","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":180,"context_line":"        # merge in any provided properties"},{"line_number":181,"context_line":"        if props:"},{"line_number":182,"context_line":"            meta.update(props)"},{"line_number":183,"context_line":"        meta.update(metadata)"},{"line_number":184,"context_line":"        self.metadata \u003d meta"},{"line_number":185,"context_line":"        self.flavor \u003d self.adapter._findFlavor("},{"line_number":186,"context_line":"            flavor_name\u003dself.label.flavor_name,"}],"source_content_type":"text/x-python","patch_set":7,"id":"557303dd_15955cfd","line":183,"in_reply_to":"c9a9db6f_83b66038","updated":"2022-11-22 00:24:38.000000000","message":"Will do.  These 2 variables are:\n* props: user-supplied instance properties from the config file\n* metadata: nodepool-supplied internal data (like node id)","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":259,"context_line":"                    self.state \u003d self.COMPLETE"},{"line_number":260,"context_line":"            elif self.server.status \u003d\u003d \u0027ERROR\u0027:"},{"line_number":261,"context_line":"                if (\u0027fault\u0027 in self.server and self.server[\u0027fault\u0027] is not None"},{"line_number":262,"context_line":"                    and \u0027message\u0027 in self.server[\u0027fault\u0027]):"},{"line_number":263,"context_line":"                    self.log.error("},{"line_number":264,"context_line":"                        \"Error in creating the server.\""},{"line_number":265,"context_line":"                        \" Compute service reports fault: {reason}\".format("}],"source_content_type":"text/x-python","patch_set":7,"id":"5f6d2552_f0739a76","line":262,"updated":"2022-11-21 23:39:23.000000000","message":"Do we need to check for quota messages here as we do above? I think openstack may actually check these things upfront so we don\u0027t need to.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":259,"context_line":"                    self.state \u003d self.COMPLETE"},{"line_number":260,"context_line":"            elif self.server.status \u003d\u003d \u0027ERROR\u0027:"},{"line_number":261,"context_line":"                if (\u0027fault\u0027 in self.server and self.server[\u0027fault\u0027] is not None"},{"line_number":262,"context_line":"                    and \u0027message\u0027 in self.server[\u0027fault\u0027]):"},{"line_number":263,"context_line":"                    self.log.error("},{"line_number":264,"context_line":"                        \"Error in creating the server.\""},{"line_number":265,"context_line":"                        \" Compute service reports fault: {reason}\".format("}],"source_content_type":"text/x-python","patch_set":7,"id":"ed533a69_3b554896","line":262,"in_reply_to":"5f6d2552_f0739a76","updated":"2022-11-22 00:24:38.000000000","message":"I believe that is correct.  If we don\u0027t have quota, we shouldn\u0027t have gotten this far.  These errors are more likely to be that openstack can\u0027t find an adequate compute host.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":298,"context_line":"            self.attempts +\u003d 1"},{"line_number":299,"context_line":"            if self.attempts \u003e\u003d self.retries:"},{"line_number":300,"context_line":"                raise Exception(\"Too many retries\")"},{"line_number":301,"context_line":"            if self.quota_exceeded:"},{"line_number":302,"context_line":"                # A quota exception is not directly recoverable so bail"},{"line_number":303,"context_line":"                # out immediately with a specific exception."},{"line_number":304,"context_line":"                self.log.info(\"Quota exceeded, invalidating quota cache\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"e0fac7b3_b9e7e4a0","line":301,"updated":"2022-11-21 23:39:23.000000000","message":"Should this check occur before the attempts value is incremented? Otherwise we may report an extra attempt when we log the attempts for this node that has failed.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"4c7878877b24fc05e991ac0ccaae7587360bdfb1","unresolved":false,"context_lines":[{"line_number":298,"context_line":"            self.attempts +\u003d 1"},{"line_number":299,"context_line":"            if self.attempts \u003e\u003d self.retries:"},{"line_number":300,"context_line":"                raise Exception(\"Too many retries\")"},{"line_number":301,"context_line":"            if self.quota_exceeded:"},{"line_number":302,"context_line":"                # A quota exception is not directly recoverable so bail"},{"line_number":303,"context_line":"                # out immediately with a specific exception."},{"line_number":304,"context_line":"                self.log.info(\"Quota exceeded, invalidating quota cache\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"607e4c0e_519be161","line":301,"in_reply_to":"93cdaaf0_b68c31b6","updated":"2022-11-22 00:31:04.000000000","message":"Ya I was mostly thinking from an accounting/stats standpoint ensuring this number is accurate may be useful. But I don\u0027t think it is critical because we are going to fail either way.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":true,"context_lines":[{"line_number":298,"context_line":"            self.attempts +\u003d 1"},{"line_number":299,"context_line":"            if self.attempts \u003e\u003d self.retries:"},{"line_number":300,"context_line":"                raise Exception(\"Too many retries\")"},{"line_number":301,"context_line":"            if self.quota_exceeded:"},{"line_number":302,"context_line":"                # A quota exception is not directly recoverable so bail"},{"line_number":303,"context_line":"                # out immediately with a specific exception."},{"line_number":304,"context_line":"                self.log.info(\"Quota exceeded, invalidating quota cache\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"93cdaaf0_b68c31b6","line":301,"in_reply_to":"e0fac7b3_b9e7e4a0","updated":"2022-11-22 00:24:38.000000000","message":"At this point we\u0027re going to exit the state machine altogether so the number of attempts doesn\u0027t matter.  We could move it anyway in order to future-proof it in case something changes, but I\u0027d rather keep the sequence around the retry count as-is and don\u0027t think it\u0027s worth the complexity to work around that.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":309,"context_line":"        if self.state \u003d\u003d self.FLOATING_IP_CREATING:"},{"line_number":310,"context_line":"            self.floating_ip \u003d self.adapter._refreshFloatingIp("},{"line_number":311,"context_line":"                self.floating_ip)"},{"line_number":312,"context_line":"            if self.floating_ip.get(\u0027port_id\u0027, None):"},{"line_number":313,"context_line":"                if self.floating_ip[\u0027status\u0027] \u003d\u003d \u0027ACTIVE\u0027:"},{"line_number":314,"context_line":"                    self.state \u003d self.FLOATING_IP_ATTACHING"},{"line_number":315,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ae47ac25_fd896905","line":312,"updated":"2022-11-21 23:39:23.000000000","message":"In this case we don\u0027t need an explicit attach because it is already associated with a port?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":309,"context_line":"        if self.state \u003d\u003d self.FLOATING_IP_CREATING:"},{"line_number":310,"context_line":"            self.floating_ip \u003d self.adapter._refreshFloatingIp("},{"line_number":311,"context_line":"                self.floating_ip)"},{"line_number":312,"context_line":"            if self.floating_ip.get(\u0027port_id\u0027, None):"},{"line_number":313,"context_line":"                if self.floating_ip[\u0027status\u0027] \u003d\u003d \u0027ACTIVE\u0027:"},{"line_number":314,"context_line":"                    self.state \u003d self.FLOATING_IP_ATTACHING"},{"line_number":315,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"417c1945_f0104897","line":312,"in_reply_to":"ae47ac25_fd896905","updated":"2022-11-22 00:24:38.000000000","message":"Yep.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":384,"context_line":"                                    \u0027server\u0027, server.id)"},{"line_number":385,"context_line":"        # Floating IP and port leakage can\u0027t be handled by the"},{"line_number":386,"context_line":"        # automatic resource cleanup due to lack of metadata, so we"},{"line_number":387,"context_line":"        # call the cleanup methods here."},{"line_number":388,"context_line":"        if self.provider.port_cleanup_interval:"},{"line_number":389,"context_line":"            self._cleanupLeakedPorts()"},{"line_number":390,"context_line":"        if self.provider.clean_floating_ips:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ad4195ce_b104db73","line":387,"updated":"2022-11-21 23:39:23.000000000","message":"I\u0027m not sure I understand this comment. We\u0027re calling the methods without the extra metadata either way.\n\nIs it that we don\u0027t have enough info to call it from cleanupLeakedResources()?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":384,"context_line":"                                    \u0027server\u0027, server.id)"},{"line_number":385,"context_line":"        # Floating IP and port leakage can\u0027t be handled by the"},{"line_number":386,"context_line":"        # automatic resource cleanup due to lack of metadata, so we"},{"line_number":387,"context_line":"        # call the cleanup methods here."},{"line_number":388,"context_line":"        if self.provider.port_cleanup_interval:"},{"line_number":389,"context_line":"            self._cleanupLeakedPorts()"},{"line_number":390,"context_line":"        if self.provider.clean_floating_ips:"}],"source_content_type":"text/x-python","patch_set":7,"id":"a4ee2077_ac92cbcb","line":387,"in_reply_to":"ad4195ce_b104db73","updated":"2022-11-22 00:24:38.000000000","message":"Yes that\u0027s the issue -- openstack doesn\u0027t store metadata on those objects, so cleanupLeakedResources (which is basically a method that deletes resources if their metadata doesn\u0027t match nodepool\u0027s state) can\u0027t perform its delta calculation on them.  I\u0027ll change the wording on the method to make it more clear.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            self._cleanupFloatingIps()"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def deleteResource(self, resource):"},{"line_number":394,"context_line":"        self.log.info(f\"Deleting leaked {resource.type}: {resource.id}\")"},{"line_number":395,"context_line":"        if resource.type \u003d\u003d \u0027server\u0027:"},{"line_number":396,"context_line":"            self._deleteServer(resource.id)"},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"710e19c4_d0e71f1b","line":394,"updated":"2022-11-21 23:39:23.000000000","message":"Note that this method\u0027s doc string says it should be called to delete the resources not necessarily due to leakage. However, we seem to only call this method from cleanupLeakedResources().","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":391,"context_line":"            self._cleanupFloatingIps()"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def deleteResource(self, resource):"},{"line_number":394,"context_line":"        self.log.info(f\"Deleting leaked {resource.type}: {resource.id}\")"},{"line_number":395,"context_line":"        if resource.type \u003d\u003d \u0027server\u0027:"},{"line_number":396,"context_line":"            self._deleteServer(resource.id)"},{"line_number":397,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"02d70fb7_470395e6","line":394,"in_reply_to":"710e19c4_d0e71f1b","updated":"2022-11-22 00:24:38.000000000","message":"The doc string in statemachine.py does actually mention that it\u0027s for a leaked resource.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"b65a67fbfeb8c6a43de812c3cf84b362a4334e1d","unresolved":true,"context_lines":[{"line_number":670,"context_line":"                if fip.status \u003d\u003d \u0027DOWN\u0027:"},{"line_number":671,"context_line":"                    return None"},{"line_number":672,"context_line":"                return fip"},{"line_number":673,"context_line":"        return obj"},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"    def _needsFloatingIp(self, server):"},{"line_number":676,"context_line":"        return self._client._needs_floating_ip("}],"source_content_type":"text/x-python","patch_set":13,"id":"df87471b_97009900","line":673,"range":{"start_line":673,"start_character":15,"end_line":673,"end_character":18},"updated":"2023-02-21 09:37:16.000000000","message":"shouldn\u0027t this be \"return None\"? else how would a successful deletion be noticed?","commit_id":"be3edd3e17eb1f262c77a5adf6ca3f9c5214886e"}],"nodepool/driver/statemachine.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":466,"context_line":"        zone."},{"line_number":467,"context_line":"        For this to work properly, the provider entry in the nodepool"},{"line_number":468,"context_line":"        config must list the availability zones. Otherwise, new nodes will be"},{"line_number":469,"context_line":"        put in random AZs at nova\u0027s whim. The exception being if there is an"},{"line_number":470,"context_line":"        existing node in the READY state that we can select for this node set."},{"line_number":471,"context_line":"        Its AZ will then be used for new nodes, as well as any other READY"},{"line_number":472,"context_line":"        nodes."}],"source_content_type":"text/x-python","patch_set":7,"id":"2940728a_84ea3555","line":469,"range":{"start_line":469,"start_character":15,"end_line":469,"end_character":41},"updated":"2022-11-21 23:39:23.000000000","message":"Nit this comment is super openstack specific but should probably be made more generic now that it is in the generic statemachine driver?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":466,"context_line":"        zone."},{"line_number":467,"context_line":"        For this to work properly, the provider entry in the nodepool"},{"line_number":468,"context_line":"        config must list the availability zones. Otherwise, new nodes will be"},{"line_number":469,"context_line":"        put in random AZs at nova\u0027s whim. The exception being if there is an"},{"line_number":470,"context_line":"        existing node in the READY state that we can select for this node set."},{"line_number":471,"context_line":"        Its AZ will then be used for new nodes, as well as any other READY"},{"line_number":472,"context_line":"        nodes."}],"source_content_type":"text/x-python","patch_set":7,"id":"8cc8311f_fd0f2e94","line":469,"range":{"start_line":469,"start_character":15,"end_line":469,"end_character":41},"in_reply_to":"2940728a_84ea3555","updated":"2022-11-22 00:24:38.000000000","message":"Done.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":540,"context_line":"        self.keyscan_worker \u003d ThreadPoolExecutor()"},{"line_number":541,"context_line":"        self.state_machine_thread \u003d threading.Thread("},{"line_number":542,"context_line":"            target\u003dself._runStateMachines,"},{"line_number":543,"context_line":"            daemon\u003dTrue)"},{"line_number":544,"context_line":"        self.state_machine_thread.start()"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"    def stop(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"d2fbc38e_b3676fc6","line":543,"updated":"2022-11-21 23:39:23.000000000","message":"I\u0027m guessing that testing caught this because we started a bunch of non daemon threads that never exited causing tests to never exit?","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        self.keyscan_worker \u003d ThreadPoolExecutor()"},{"line_number":541,"context_line":"        self.state_machine_thread \u003d threading.Thread("},{"line_number":542,"context_line":"            target\u003dself._runStateMachines,"},{"line_number":543,"context_line":"            daemon\u003dTrue)"},{"line_number":544,"context_line":"        self.state_machine_thread.start()"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"    def stop(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"4914453d_96257316","line":543,"in_reply_to":"d2fbc38e_b3676fc6","updated":"2022-11-22 00:24:38.000000000","message":"That sounds right (but my memory is a little fuzzy).","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"}],"nodepool/tests/unit/test_launcher.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"78bc494d97dbc0a46b5d8db0cd3fb0d97a1bba90","unresolved":true,"context_lines":[{"line_number":1254,"context_line":"        time.sleep(5)"},{"line_number":1255,"context_line":"        # Make sure it shows up as leaked"},{"line_number":1256,"context_line":"        manager \u003d pool.getProviderManager(\u0027fake-provider\u0027)"},{"line_number":1257,"context_line":"        instances \u003d list(manager.adapter.listInstances())"},{"line_number":1258,"context_line":"        self.assertEqual(1, len(instances))"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def test_leaked_node(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfa148ea_ac893e0c","line":1257,"updated":"2022-11-21 23:39:23.000000000","message":"In most places we seem to use the adapter private methods like _listServers(). Here we use listInstances(). Is there a reason to prefer one over the other? Maybe we should be using public interfaces which call into private methods for maximum coverage?\n\nReading more it appears the listInstances gives us an incomplete view so may not be appropriate for all test needs.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"5d780c892ac5b1d03bb250aba02f2251d384bd6c","unresolved":false,"context_lines":[{"line_number":1254,"context_line":"        time.sleep(5)"},{"line_number":1255,"context_line":"        # Make sure it shows up as leaked"},{"line_number":1256,"context_line":"        manager \u003d pool.getProviderManager(\u0027fake-provider\u0027)"},{"line_number":1257,"context_line":"        instances \u003d list(manager.adapter.listInstances())"},{"line_number":1258,"context_line":"        self.assertEqual(1, len(instances))"},{"line_number":1259,"context_line":""},{"line_number":1260,"context_line":"    def test_leaked_node(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"d45ec034_7bcaa189","line":1257,"in_reply_to":"bfa148ea_ac893e0c","updated":"2022-11-22 00:24:38.000000000","message":"I tried to keep the tests as equivalent as possible.  In this case, this seemed to be the best way to determine that we still had a non-deleting instance.  In other words, in this case we can use the higher-level API.\n\nWe probably could have used the lower-level API here, but it wasn\u0027t necessary.  I think in other places we may need to use the low-level API in order to see deleting servers.","commit_id":"28adec688f322ba85681107ac47ed7d96201e9ec"}]}