)]}'
{"nodepool/driver/gce/adapter.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"53612b3bcd79b4c72adb182d59187d83f8a22305","unresolved":true,"context_lines":[{"line_number":252,"context_line":"            result \u003d q.execute()"},{"line_number":253,"context_line":"        return result.get(\u0027items\u0027, [])"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    @cachetools.func.lru_cache(maxsize\u003dNone)"},{"line_number":256,"context_line":"    def _getImageId(self, cloud_image):"},{"line_number":257,"context_line":"        image_id \u003d cloud_image.image_id"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"4785a561_72ab3b1a","line":255,"updated":"2022-08-25 09:52:45.000000000","message":"Isn\u0027t this leaking memory over time? AFAIK the cache is not instance scoped and since the args also include the self reference we will leak all instances that we create.","commit_id":"803a6bc00e97a510fd06d973cd1ed31956a16eff"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"0c068d3e1c0487ceecfed6eedbc1fc37fa4eaa2d","unresolved":false,"context_lines":[{"line_number":252,"context_line":"            result \u003d q.execute()"},{"line_number":253,"context_line":"        return result.get(\u0027items\u0027, [])"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    @cachetools.func.lru_cache(maxsize\u003dNone)"},{"line_number":256,"context_line":"    def _getImageId(self, cloud_image):"},{"line_number":257,"context_line":"        image_id \u003d cloud_image.image_id"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"91faf867_663bd944","line":255,"in_reply_to":"4785a561_72ab3b1a","updated":"2022-08-25 14:42:45.000000000","message":"Yep.  Fixed in a followup at https://review.opendev.org/854632 since it affects more than just this driver.","commit_id":"803a6bc00e97a510fd06d973cd1ed31956a16eff"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"47ca7ff1c52bce7d738adbbee9d44eda2113c186","unresolved":false,"context_lines":[{"line_number":164,"context_line":"            machine_type \u003d instance[\u0027machineType\u0027].split(\u0027/\u0027)[-1]"},{"line_number":165,"context_line":"            quota \u003d self._getQuotaForMachineType(machine_type)"},{"line_number":166,"context_line":"            instances.append(GceInstance(instance, quota))"},{"line_number":167,"context_line":"        return instances"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def listResources(self):"},{"line_number":170,"context_line":"        for instance in self._listInstances():"}],"source_content_type":"text/x-python","patch_set":7,"id":"c14cd2db_d0c98ed1","line":167,"updated":"2022-08-29 20:00:01.000000000","message":"There are multiple code paths that call _listInstances, not all of which return the higher level objects (to be clear, we\u0027re not modifying the result, we\u0027re augmenting it), so it\u0027s best to cache at the lowest level so they all benefit.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":164,"context_line":"            machine_type \u003d instance[\u0027machineType\u0027].split(\u0027/\u0027)[-1]"},{"line_number":165,"context_line":"            quota \u003d self._getQuotaForMachineType(machine_type)"},{"line_number":166,"context_line":"            instances.append(GceInstance(instance, quota))"},{"line_number":167,"context_line":"        return instances"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def listResources(self):"},{"line_number":170,"context_line":"        for instance in self._listInstances():"}],"source_content_type":"text/x-python","patch_set":7,"id":"7a6ee847_e12d8fa9","line":167,"updated":"2022-08-29 19:41:01.000000000","message":"We are caching the underlying request responses but then we perform additional modification here. Would it make sense to cache this layer instead and then cut down on the quota and such modifications?\n\nOr do we need this layer to be more mutable?","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":171,"context_line":"            if instance[\u0027status\u0027] \u003d\u003d \u0027TERMINATED\u0027:"},{"line_number":172,"context_line":"                continue"},{"line_number":173,"context_line":"            metadata \u003d gce_metadata_to_dict(instance.get(\u0027metadata\u0027))"},{"line_number":174,"context_line":"            yield GceResource(metadata, \u0027instance\u0027, instance[\u0027name\u0027])"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    def deleteResource(self, resource):"},{"line_number":177,"context_line":"        self.log.info(f\"Deleting leaked {resource.type}: {resource.id}\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"802d79e4_ba1825a3","line":174,"updated":"2022-08-29 19:41:01.000000000","message":"See above.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"}],"nodepool/driver/kubernetes/handler.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":82,"context_line":"                attempts +\u003d 1"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"class KubernetesNodeRequestHandler(NodeRequestHandler):"},{"line_number":86,"context_line":"    log \u003d logging.getLogger(\"nodepool.driver.kubernetes.\""},{"line_number":87,"context_line":"                            \"KubernetesNodeRequestHandler\")"},{"line_number":88,"context_line":"    launcher \u003d K8SLauncher"}],"source_content_type":"text/x-python","patch_set":7,"id":"7c35a25c_4b8ba84a","line":85,"updated":"2022-08-29 19:41:01.000000000","message":"The commit message indicates that the GCE driver was the only \"simple\" one but it appears k8s was using it too. If a new patchset is needed might want to update the commit message to match.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"47ca7ff1c52bce7d738adbbee9d44eda2113c186","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                attempts +\u003d 1"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"class KubernetesNodeRequestHandler(NodeRequestHandler):"},{"line_number":86,"context_line":"    log \u003d logging.getLogger(\"nodepool.driver.kubernetes.\""},{"line_number":87,"context_line":"                            \"KubernetesNodeRequestHandler\")"},{"line_number":88,"context_line":"    launcher \u003d K8SLauncher"}],"source_content_type":"text/x-python","patch_set":7,"id":"8e2fc1a2_0e35dad4","line":85,"updated":"2022-08-29 20:00:01.000000000","message":"Yes, this use of just part of the simple driver was... unexpected.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":127,"context_line":"                cloud_quota.subtract(needed_quota)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"                if not cloud_quota.non_negative():"},{"line_number":130,"context_line":"                    return False"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        # Now calculate pool specific quota. Values indicating no quota default"},{"line_number":133,"context_line":"        # to math.inf representing infinity that can be calculated with."}],"source_content_type":"text/x-python","patch_set":7,"id":"7b1d47dd_aec0a46b","line":130,"updated":"2022-08-29 19:41:01.000000000","message":"I don\u0027t know why but this condition had my brain all confused for a minute.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        label \u003d self.pool.labels[node.type[0]]"},{"line_number":205,"context_line":"        thd \u003d self.launcher(self, node, self.provider, label)"},{"line_number":206,"context_line":"        thd.start()"},{"line_number":207,"context_line":"        self._threads.append(thd)"}],"source_content_type":"text/x-python","patch_set":7,"id":"a16dabde_5c1030b2","line":207,"updated":"2022-08-29 19:41:01.000000000","message":"I think there may be a minor race here where the thread can be running but it isn\u0027t in the thread list. You may want to consider swapping the order of lines 206 and 207 to address that.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"47ca7ff1c52bce7d738adbbee9d44eda2113c186","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        label \u003d self.pool.labels[node.type[0]]"},{"line_number":205,"context_line":"        thd \u003d self.launcher(self, node, self.provider, label)"},{"line_number":206,"context_line":"        thd.start()"},{"line_number":207,"context_line":"        self._threads.append(thd)"}],"source_content_type":"text/x-python","patch_set":7,"id":"d3cb61a7_6af355b2","line":207,"updated":"2022-08-29 20:00:01.000000000","message":"This is all a copy of the existing code.  I don\u0027t know off-hand the ramifications of changing that (what you say makes sense, yet it\u0027s possible the opposite could be true -- another thread may attempt to operate on a non-started thread).  I\u0027d like to declare this out of scope for this change.  :)","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"75ef87dbbd7f213e4f9c90f3d6aba1a2b24cb799","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        label \u003d self.pool.labels[node.type[0]]"},{"line_number":205,"context_line":"        thd \u003d self.launcher(self, node, self.provider, label)"},{"line_number":206,"context_line":"        thd.start()"},{"line_number":207,"context_line":"        self._threads.append(thd)"}],"source_content_type":"text/x-python","patch_set":7,"id":"e1dbf79e_5825f341","line":207,"in_reply_to":"d3cb61a7_6af355b2","updated":"2022-08-29 20:02:07.000000000","message":"Fair enough. Might be worth looking at in a followup. Though I suspect the race is very small here and unlikely to be hit in most real world situations due to how long provider api requests take.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"}],"nodepool/driver/statemachine.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"47ca7ff1c52bce7d738adbbee9d44eda2113c186","unresolved":false,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"class StateMachineHandler(NodeRequestHandler):"},{"line_number":349,"context_line":"    log \u003d logging.getLogger(\"nodepool.StateMachineHandler\")"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def __init__(self, pw, request):"},{"line_number":352,"context_line":"        super().__init__(pw, request)"}],"source_content_type":"text/x-python","patch_set":7,"id":"afbeb654_a87a8884","line":349,"updated":"2022-08-29 20:00:01.000000000","message":"I think it makes a lot more sense for the logger names to be short and sensible for users.  I think we have some weird old copy-pasta in our current logger names.","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"fe3a1fc4e71dffa9fb1ed27e9e3420c407c4e8bf","unresolved":true,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"class StateMachineHandler(NodeRequestHandler):"},{"line_number":349,"context_line":"    log \u003d logging.getLogger(\"nodepool.StateMachineHandler\")"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def __init__(self, pw, request):"},{"line_number":352,"context_line":"        super().__init__(pw, request)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5b133911_be08d40f","line":349,"updated":"2022-08-29 19:41:01.000000000","message":"Nit should this be nodepool.driver.StateMachineHandler to match the class path?","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"75ef87dbbd7f213e4f9c90f3d6aba1a2b24cb799","unresolved":false,"context_lines":[{"line_number":346,"context_line":""},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"class StateMachineHandler(NodeRequestHandler):"},{"line_number":349,"context_line":"    log \u003d logging.getLogger(\"nodepool.StateMachineHandler\")"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def __init__(self, pw, request):"},{"line_number":352,"context_line":"        super().__init__(pw, request)"}],"source_content_type":"text/x-python","patch_set":7,"id":"14dc3f0d_f7973f43","line":349,"in_reply_to":"afbeb654_a87a8884","updated":"2022-08-29 20:02:07.000000000","message":"ACK","commit_id":"adab6eeb0daf1cdedecd241b3f0d01c3fbeac658"}]}