)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"2df1600a9f35eba5cb44650a983c3ae3c6f7a3d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7e35def8_d00f9c76","updated":"2023-07-10 05:42:48.000000000","message":"recheck","commit_id":"9ad24b84510bfba3fc3bd6921c098441e75530c5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f0b770fd_e058276f","updated":"2023-07-25 02:15:10.000000000","message":"I think at least some of the inline comments will need to be addressed (particularly the need for .get() in some places and the pagination reset fix).","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"}],"nodepool/driver/openstack/adapter.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":119,"context_line":"    def __init__(self, provider, server, quota):"},{"line_number":120,"context_line":"        super().__init__()"},{"line_number":121,"context_line":"        self.external_id \u003d server[\u0027id\u0027]"},{"line_number":122,"context_line":"        self.metadata \u003d server[\u0027metadata\u0027]"},{"line_number":123,"context_line":"        self.private_ipv4 \u003d server.get(\u0027private_v4\u0027)"},{"line_number":124,"context_line":"        self.private_ipv6 \u003d None"},{"line_number":125,"context_line":"        self.public_ipv4 \u003d server.get(\u0027public_v4\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"56e278d6_a628cb1c","line":122,"updated":"2023-07-25 02:15:10.000000000","message":"I think metadata isn\u0027t required by the api so this may not exist? Might be safer to use .get().","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    def __init__(self, provider, server, quota):"},{"line_number":120,"context_line":"        super().__init__()"},{"line_number":121,"context_line":"        self.external_id \u003d server[\u0027id\u0027]"},{"line_number":122,"context_line":"        self.metadata \u003d server[\u0027metadata\u0027]"},{"line_number":123,"context_line":"        self.private_ipv4 \u003d server.get(\u0027private_v4\u0027)"},{"line_number":124,"context_line":"        self.private_ipv6 \u003d None"},{"line_number":125,"context_line":"        self.public_ipv4 \u003d server.get(\u0027public_v4\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"52b8bf90_d7f8c09f","line":122,"in_reply_to":"56e278d6_a628cb1c","updated":"2023-07-25 18:29:37.000000000","message":"If it isn\u0027t, nodepool will be very unhappy for other reasons, but we can be defensive here.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":123,"context_line":"        self.private_ipv4 \u003d server.get(\u0027private_v4\u0027)"},{"line_number":124,"context_line":"        self.private_ipv6 \u003d None"},{"line_number":125,"context_line":"        self.public_ipv4 \u003d server.get(\u0027public_v4\u0027)"},{"line_number":126,"context_line":"        self.public_ipv6 \u003d server.get(\u0027public_v6\u0027)"},{"line_number":127,"context_line":"        self.host_id \u003d server[\u0027hostId\u0027]"},{"line_number":128,"context_line":"        self.cloud \u003d provider.cloud_config.name"},{"line_number":129,"context_line":"        self.region \u003d provider.region_name"}],"source_content_type":"text/x-python","patch_set":6,"id":"c36a7a0f_1d26e971","line":126,"updated":"2023-07-25 02:15:10.000000000","message":"Should IP addresses be .get() values since some clouds may be ipv4 only or ipv6 only?","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        self.private_ipv4 \u003d server.get(\u0027private_v4\u0027)"},{"line_number":124,"context_line":"        self.private_ipv6 \u003d None"},{"line_number":125,"context_line":"        self.public_ipv4 \u003d server.get(\u0027public_v4\u0027)"},{"line_number":126,"context_line":"        self.public_ipv6 \u003d server.get(\u0027public_v6\u0027)"},{"line_number":127,"context_line":"        self.host_id \u003d server[\u0027hostId\u0027]"},{"line_number":128,"context_line":"        self.cloud \u003d provider.cloud_config.name"},{"line_number":129,"context_line":"        self.region \u003d provider.region_name"}],"source_content_type":"text/x-python","patch_set":6,"id":"4826773b_1d1b3726","line":126,"in_reply_to":"c36a7a0f_1d26e971","updated":"2023-07-25 18:29:37.000000000","message":"I\u0027m interpreting this question as \"Why did you use get here?\"  and it\u0027s actually because the ip addrs are filled in by a separate method (add_server_interfaces) and we don\u0027t call that in the cleanup leaked resources code path.  The other thing about v4 or v6 only may also be true.  :)","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            self.server \u003d self.adapter._getServer(self.external_id)"},{"line_number":168,"context_line":"            if (self.server and"},{"line_number":169,"context_line":"                self.adapter._hasFloatingIps() and"},{"line_number":170,"context_line":"                self.server[\u0027addresses\u0027]):"},{"line_number":171,"context_line":"                self.floating_ips \u003d self.adapter._getFloatingIps(self.server)"},{"line_number":172,"context_line":"                for fip in self.floating_ips:"},{"line_number":173,"context_line":"                    self.adapter._deleteFloatingIp(fip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c5de9326_47eef5df","line":170,"updated":"2023-07-25 02:15:10.000000000","message":"I would use .get() here since we are testing the truthyness of this value and using element access may raise an exception if None.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            self.server \u003d self.adapter._getServer(self.external_id)"},{"line_number":168,"context_line":"            if (self.server and"},{"line_number":169,"context_line":"                self.adapter._hasFloatingIps() and"},{"line_number":170,"context_line":"                self.server[\u0027addresses\u0027]):"},{"line_number":171,"context_line":"                self.floating_ips \u003d self.adapter._getFloatingIps(self.server)"},{"line_number":172,"context_line":"                for fip in self.floating_ips:"},{"line_number":173,"context_line":"                    self.adapter._deleteFloatingIp(fip)"}],"source_content_type":"text/x-python","patch_set":6,"id":"8cd66548_961acc28","line":170,"in_reply_to":"c5de9326_47eef5df","updated":"2023-07-25 18:29:37.000000000","message":"Done.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":455,"context_line":"        for server in self._listServers():"},{"line_number":456,"context_line":"            if server[\u0027status\u0027].lower() \u003d\u003d \u0027deleted\u0027:"},{"line_number":457,"context_line":"                continue"},{"line_number":458,"context_line":"            yield OpenStackResource(server[\u0027metadata\u0027],"},{"line_number":459,"context_line":"                                    OpenStackResource.TYPE_INSTANCE,"},{"line_number":460,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":461,"context_line":"        # Floating IP and port leakage can\u0027t be handled by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"56b61b2a_7c3f1f53","line":458,"updated":"2023-07-25 02:15:10.000000000","message":"If you decide to .get() metadata above you should update this line too.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":455,"context_line":"        for server in self._listServers():"},{"line_number":456,"context_line":"            if server[\u0027status\u0027].lower() \u003d\u003d \u0027deleted\u0027:"},{"line_number":457,"context_line":"                continue"},{"line_number":458,"context_line":"            yield OpenStackResource(server[\u0027metadata\u0027],"},{"line_number":459,"context_line":"                                    OpenStackResource.TYPE_INSTANCE,"},{"line_number":460,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":461,"context_line":"        # Floating IP and port leakage can\u0027t be handled by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"c6325cca_a7e5ee17","line":458,"in_reply_to":"56b61b2a_7c3f1f53","updated":"2023-07-25 18:29:37.000000000","message":"Done","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":734,"context_line":"            last_marker \u003d query_params.pop(\u0027marker\u0027, None)"},{"line_number":735,"context_line":"            query_params.pop(\u0027limit\u0027, None)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"            resources \u003d data[\u0027servers\u0027]"},{"line_number":738,"context_line":"            if not isinstance(resources, list):"},{"line_number":739,"context_line":"                resources \u003d [resources]"},{"line_number":740,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"222abb32_39bfa9ad","line":737,"updated":"2023-07-25 02:15:10.000000000","message":"This overwrites the previous request\u0027s resource list rather than appending to it. I think we may end up losing resources if we end up doing paged requests.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":734,"context_line":"            last_marker \u003d query_params.pop(\u0027marker\u0027, None)"},{"line_number":735,"context_line":"            query_params.pop(\u0027limit\u0027, None)"},{"line_number":736,"context_line":""},{"line_number":737,"context_line":"            resources \u003d data[\u0027servers\u0027]"},{"line_number":738,"context_line":"            if not isinstance(resources, list):"},{"line_number":739,"context_line":"                resources \u003d [resources]"},{"line_number":740,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"e22143ad_a90064a1","line":737,"in_reply_to":"222abb32_39bfa9ad","updated":"2023-07-25 18:29:37.000000000","message":"Oops.  Fixed.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":789,"context_line":"            parts \u003d urllib.parse.urlparse(next_link)"},{"line_number":790,"context_line":"            query_params \u003d urllib.parse.parse_qs(parts.query)"},{"line_number":791,"context_line":"            params.update(query_params)"},{"line_number":792,"context_line":"            next_link \u003d urllib.parse.urljoin(next_link, parts.path)"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"        # If we still have no link, and limit was given and is non-zero,"},{"line_number":795,"context_line":"        # and the number of records yielded equals the limit, then the user"}],"source_content_type":"text/x-python","patch_set":6,"id":"10bdb520_1abc4b61","line":792,"updated":"2023-07-25 02:15:10.000000000","message":"I guess parts.path is fully rooted so it ends up replacing the old url path which included the query parameters.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":789,"context_line":"            parts \u003d urllib.parse.urlparse(next_link)"},{"line_number":790,"context_line":"            query_params \u003d urllib.parse.parse_qs(parts.query)"},{"line_number":791,"context_line":"            params.update(query_params)"},{"line_number":792,"context_line":"            next_link \u003d urllib.parse.urljoin(next_link, parts.path)"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"        # If we still have no link, and limit was given and is non-zero,"},{"line_number":795,"context_line":"        # and the number of records yielded equals the limit, then the user"}],"source_content_type":"text/x-python","patch_set":6,"id":"2de3faa1_10de8830","line":792,"in_reply_to":"10bdb520_1abc4b61","updated":"2023-07-25 18:29:37.000000000","message":"Yep.  This part is copied from sdk.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"}],"nodepool/tests/unit/test_launcher.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"877c87693606cad49bb29830838af28ca580dcb5","unresolved":true,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        # Get fake cloud record and set status to DELETING"},{"line_number":1113,"context_line":"        manager \u003d pool.getProviderManager(\u0027fake-provider\u0027)"},{"line_number":1114,"context_line":"        for instance in manager.adapter._client._server_list:"},{"line_number":1115,"context_line":"            if instance.id \u003d\u003d nodes[0].external_id:"},{"line_number":1116,"context_line":"                instance[\u0027status\u0027] \u003d \u0027DELETED\u0027"},{"line_number":1117,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":6,"id":"edc54767_a61afcc7","line":1114,"updated":"2023-07-25 02:15:10.000000000","message":"In https://review.opendev.org/c/zuul/nodepool/+/887907/6/nodepool/tests/__init__.py we continue to use manager.adapter._listServers(). Is this update necessary?","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"26275971441a1cdc62c9924e9ae748976aae868e","unresolved":false,"context_lines":[{"line_number":1111,"context_line":""},{"line_number":1112,"context_line":"        # Get fake cloud record and set status to DELETING"},{"line_number":1113,"context_line":"        manager \u003d pool.getProviderManager(\u0027fake-provider\u0027)"},{"line_number":1114,"context_line":"        for instance in manager.adapter._client._server_list:"},{"line_number":1115,"context_line":"            if instance.id \u003d\u003d nodes[0].external_id:"},{"line_number":1116,"context_line":"                instance[\u0027status\u0027] \u003d \u0027DELETED\u0027"},{"line_number":1117,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":6,"id":"e2d6c608_753e37b8","line":1114,"in_reply_to":"edc54767_a61afcc7","updated":"2023-07-25 18:29:37.000000000","message":"Yes.  The old code was relying on the fact that the openstack server object returned by _listServers was the *actual* fake openstack server object that our fake openstack cloud kept in its server list (to put that another way, it\u0027s like the object in the client\u0027s memory was shared with the object in the server\u0027s memory).  The intent here is to manipulate the server-side view of the instance, and then verify the client-side update is correct.\n\nAnyway, since we now return something other than the actual server-side fake object: the fake json response object, this is updated to mutate the server-side data directly.","commit_id":"86a923aa57dbf328fff93341ead694002aa7d3a5"}]}