)]}'
{"nodepool/driver/aws/adapter.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ad7739cac4b3a0f662614e35ddb983c5dc1a315a","unresolved":true,"context_lines":[{"line_number":223,"context_line":""},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"class AwsInstance(statemachine.Instance):"},{"line_number":226,"context_line":"    def __init__(self, provider, instance, host, quota, label\u003dNone):"},{"line_number":227,"context_line":"        super().__init__()"},{"line_number":228,"context_line":"        self.external_id \u003d dict()"},{"line_number":229,"context_line":"        if instance:"}],"source_content_type":"text/x-python","patch_set":2,"id":"28d2e57e_80c74bc0","line":226,"updated":"2024-09-16 13:58:24.000000000","message":"nit: Can we make that a mandatory parameter?","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"775d63ef19c7eea82ac0a73eb0f793b70ddee88e","unresolved":false,"context_lines":[{"line_number":223,"context_line":""},{"line_number":224,"context_line":""},{"line_number":225,"context_line":"class AwsInstance(statemachine.Instance):"},{"line_number":226,"context_line":"    def __init__(self, provider, instance, host, quota, label\u003dNone):"},{"line_number":227,"context_line":"        super().__init__()"},{"line_number":228,"context_line":"        self.external_id \u003d dict()"},{"line_number":229,"context_line":"        if instance:"}],"source_content_type":"text/x-python","patch_set":2,"id":"95bf8f04_318405b2","line":226,"in_reply_to":"28d2e57e_80c74bc0","updated":"2024-09-16 19:45:09.000000000","message":"Done","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"}],"nodepool/driver/statemachine.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ad7739cac4b3a0f662614e35ddb983c5dc1a315a","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"        # TODO: Figure out better naming for container?"},{"line_number":1057,"context_line":"        # Holds flags coming from label(s) that modify the node request,"},{"line_number":1058,"context_line":"        # such as `spot` instance for AWS, `fleet` API or a metastatic node"},{"line_number":1059,"context_line":"        self.label_mods \u003d {}"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def __repr__(self):"},{"line_number":1062,"context_line":"        state \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"309f51e2_22c8eee5","line":1059,"range":{"start_line":1059,"start_character":13,"end_line":1059,"end_character":23},"updated":"2024-09-16 13:58:24.000000000","message":"Maybe `instance_properties`?","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"775d63ef19c7eea82ac0a73eb0f793b70ddee88e","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"        # TODO: Figure out better naming for container?"},{"line_number":1057,"context_line":"        # Holds flags coming from label(s) that modify the node request,"},{"line_number":1058,"context_line":"        # such as `spot` instance for AWS, `fleet` API or a metastatic node"},{"line_number":1059,"context_line":"        self.label_mods \u003d {}"},{"line_number":1060,"context_line":""},{"line_number":1061,"context_line":"    def __repr__(self):"},{"line_number":1062,"context_line":"        state \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"7d0d11b4_b5cddbab","line":1059,"range":{"start_line":1059,"start_character":13,"end_line":1059,"end_character":23},"in_reply_to":"309f51e2_22c8eee5","updated":"2024-09-16 19:45:09.000000000","message":"done.","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"2e78b205cbabf2f70884090d9a8003cd7519de47","unresolved":true,"context_lines":[{"line_number":1053,"context_line":"        self.metadata \u003d {}"},{"line_number":1054,"context_line":"        self.driver_data \u003d None"},{"line_number":1055,"context_line":"        self.slot \u003d None"},{"line_number":1056,"context_line":"        # TODO: Figure out better naming for container?"},{"line_number":1057,"context_line":"        # Holds flags coming from label(s) that modify the node request,"},{"line_number":1058,"context_line":"        # such as `spot` instance for AWS, `fleet` API or a metastatic node"},{"line_number":1059,"context_line":"        self.instance_properties \u003d {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"421c657b_04cc0501","line":1056,"updated":"2024-09-30 16:39:50.000000000","message":"I don\u0027t think we need this todo?  Either call it instance_properties, or node_properties or just properties.  But we don\u0027t need to change it in nodepool in the future.  It\u0027s going to be this name until the end of nodepool.  :)\n\nBut when we get to Zuul, I will probably ask that this be exposed via a variable called nodepool.node_properties.","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"9bb7c29c116ac2d7fe0910e167773a610fb14bd9","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"        self.metadata \u003d {}"},{"line_number":1054,"context_line":"        self.driver_data \u003d None"},{"line_number":1055,"context_line":"        self.slot \u003d None"},{"line_number":1056,"context_line":"        # TODO: Figure out better naming for container?"},{"line_number":1057,"context_line":"        # Holds flags coming from label(s) that modify the node request,"},{"line_number":1058,"context_line":"        # such as `spot` instance for AWS, `fleet` API or a metastatic node"},{"line_number":1059,"context_line":"        self.instance_properties \u003d {}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9d15c683_921e4b5a","line":1056,"in_reply_to":"421c657b_04cc0501","updated":"2024-10-04 08:49:10.000000000","message":"I will rename it to node_properties, to be consistent with the zuul side.","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"}],"nodepool/tests/unit/test_driver_aws.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"2e78b205cbabf2f70884090d9a8003cd7519de47","unresolved":true,"context_lines":[{"line_number":764,"context_line":""},{"line_number":765,"context_line":"        awsi \u003d AwsInstance(provider, instance, None, None, label)"},{"line_number":766,"context_line":"        self.assertEqual(awsi.instance_properties[\"spot\"], True)"},{"line_number":767,"context_line":"        self.assertEqual(awsi.instance_properties[\"fleet\"], \"fleet_flag\")"},{"line_number":768,"context_line":""},{"line_number":769,"context_line":"    def test_aws_tags(self):"},{"line_number":770,"context_line":"        req \u003d self.requestNode(\u0027aws/aws.yaml\u0027, \u0027ubuntu1404-with-tags\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"e4a6adc0_d4432175","line":767,"updated":"2024-09-30 16:39:50.000000000","message":"You will notice that most tests in nodepool do not follow this pattern.  Instead, we prefer functional tests that fully exercise the code.  Unit tests like this often don\u0027t do much more than verify that you typed the same thing twice, and then when you need to change some internal attribute, you then need to change it twice.\n\nIt\u0027s not important that the AwsInstance have an instance_properties dict, and it\u0027s not important what its values are.  Instead, what\u0027s important is this: if a label has \"use_spot\" set to true, then the node record in ZK should have its instance_properties set.  It\u0027s the end-to-end from user configuration to consumer (zuul via the node record in zk in this case) that\u0027s important.\n\nI think you should remove this test, and instead, add an assertion to the test_aws_provisioning_spot_instances test method to check that the resulting ZK node has the spot instance_properties set.  If you do that, I think you will find that you forgot to implement the part where you copy the data from the AwsInstance to the Node.  Grep for \"updateNodeFromInstance\" to see where to do that.\n\nLikewise, add an assertion to check fleet.","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"9bb7c29c116ac2d7fe0910e167773a610fb14bd9","unresolved":false,"context_lines":[{"line_number":764,"context_line":""},{"line_number":765,"context_line":"        awsi \u003d AwsInstance(provider, instance, None, None, label)"},{"line_number":766,"context_line":"        self.assertEqual(awsi.instance_properties[\"spot\"], True)"},{"line_number":767,"context_line":"        self.assertEqual(awsi.instance_properties[\"fleet\"], \"fleet_flag\")"},{"line_number":768,"context_line":""},{"line_number":769,"context_line":"    def test_aws_tags(self):"},{"line_number":770,"context_line":"        req \u003d self.requestNode(\u0027aws/aws.yaml\u0027, \u0027ubuntu1404-with-tags\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"9e25c4e9_e844b103","line":767,"in_reply_to":"e4a6adc0_d4432175","updated":"2024-10-04 08:49:10.000000000","message":"Thank you for the suggestion. The path you pointed showed that I actually had forgotten to add the transfer of properties in one place. It\u0027s working as intended now.","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"2e78b205cbabf2f70884090d9a8003cd7519de47","unresolved":true,"context_lines":[{"line_number":1336,"context_line":"        # Test creating a spot instances instead of an on-demand on."},{"line_number":1337,"context_line":"        req \u003d self.requestNode(\u0027aws/aws-spot.yaml\u0027, \u0027ubuntu1404-spot\u0027)"},{"line_number":1338,"context_line":"        node \u003d self.assertSuccess(req)"},{"line_number":1339,"context_line":"        instance \u003d self.ec2.Instance(node.external_id[\u0027instance\u0027])"},{"line_number":1340,"context_line":"        self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)"},{"line_number":1341,"context_line":"        # moto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1342,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"89f43ae2_edecd920","line":1339,"updated":"2024-09-30 16:39:50.000000000","message":"Add something like:\n\n         self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"55233585b186f293c963fc4f20988c24ba6ad6d2","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"        # Test creating a spot instances instead of an on-demand on."},{"line_number":1337,"context_line":"        req \u003d self.requestNode(\u0027aws/aws-spot.yaml\u0027, \u0027ubuntu1404-spot\u0027)"},{"line_number":1338,"context_line":"        node \u003d self.assertSuccess(req)"},{"line_number":1339,"context_line":"        instance \u003d self.ec2.Instance(node.external_id[\u0027instance\u0027])"},{"line_number":1340,"context_line":"        self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)"},{"line_number":1341,"context_line":"        # moto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1342,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"88ef8ace_ef4add18","line":1339,"in_reply_to":"89f43ae2_edecd920","updated":"2024-10-04 08:55:41.000000000","message":"Done","commit_id":"f8ed42ae412b3b78c4860f3605c19c0acdc12c2b"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"320eb08cd981ee059a5614fb53bd830b6debb8dc","unresolved":true,"context_lines":[{"line_number":1321,"context_line":"        node \u003d self.assertSuccess(req)"},{"line_number":1322,"context_line":"        instance \u003d self.ec2.Instance(node.external_id[\u0027instance\u0027])"},{"line_number":1323,"context_line":"        self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)"},{"line_number":1324,"context_line":"        # boto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1325,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"        # `node` is the actual StateMachineInstance, in this case, AwsInstance"}],"source_content_type":"text/x-python","patch_set":10,"id":"88268ae6_2dd65bfd","line":1324,"updated":"2024-10-04 14:54:31.000000000","message":"I don\u0027t think this change is correct.\n\nHere is the documentation for boto: https://boto3.amazonaws.com/v1/documentation/api/1.34.107/reference/services/ec2/instance/spot_instance_request_id.html\n\nmoto is the mock we use for boto, and it sometimes doesn\u0027t quite match.  This comment notes a difference.","commit_id":"18dd3b719648ebd31ddbe823b19bdf6369899e37"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"3505f0a82b5ee51ec8488f279e534965cb7018c2","unresolved":true,"context_lines":[{"line_number":1321,"context_line":"        node \u003d self.assertSuccess(req)"},{"line_number":1322,"context_line":"        instance \u003d self.ec2.Instance(node.external_id[\u0027instance\u0027])"},{"line_number":1323,"context_line":"        self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)"},{"line_number":1324,"context_line":"        # boto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1325,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"        # `node` is the actual StateMachineInstance, in this case, AwsInstance"}],"source_content_type":"text/x-python","patch_set":10,"id":"f326ece3_2fd310ad","line":1324,"in_reply_to":"88268ae6_2dd65bfd","updated":"2024-10-04 15:28:29.000000000","message":"This looked like such a plausible typo that I had to lowkey fix :) apparently I was wrong, thank you for pointing out. Clever name - moto, the mock for boto!","commit_id":"18dd3b719648ebd31ddbe823b19bdf6369899e37"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"fdec60d3fad400b982f324ab63697f21741ae274","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"        node \u003d self.assertSuccess(req)"},{"line_number":1322,"context_line":"        instance \u003d self.ec2.Instance(node.external_id[\u0027instance\u0027])"},{"line_number":1323,"context_line":"        self.assertEqual(instance.instance_lifecycle, \u0027spot\u0027)"},{"line_number":1324,"context_line":"        # boto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1325,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"        # `node` is the actual StateMachineInstance, in this case, AwsInstance"}],"source_content_type":"text/x-python","patch_set":10,"id":"5e2e9cce_9304988a","line":1324,"in_reply_to":"f326ece3_2fd310ad","updated":"2024-10-07 11:15:43.000000000","message":"Done","commit_id":"18dd3b719648ebd31ddbe823b19bdf6369899e37"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"320eb08cd981ee059a5614fb53bd830b6debb8dc","unresolved":true,"context_lines":[{"line_number":1324,"context_line":"        # boto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1325,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"        # `node` is the actual StateMachineInstance, in this case, AwsInstance"},{"line_number":1328,"context_line":"        self.assertTrue(node.node_properties[\u0027spot\u0027])"},{"line_number":1329,"context_line":""},{"line_number":1330,"context_line":"    def test_aws_dedicated_host(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"721fc5d5_bca59cba","line":1327,"updated":"2024-10-04 14:54:31.000000000","message":"Node should be a nodepool.zookeeper.Node(), not a StateMachineInstance.","commit_id":"18dd3b719648ebd31ddbe823b19bdf6369899e37"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"3505f0a82b5ee51ec8488f279e534965cb7018c2","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"        # boto doesn\u0027t provide the spot_instance_request_id"},{"line_number":1325,"context_line":"        # self.assertIsNotNone(instance.spot_instance_request_id)"},{"line_number":1326,"context_line":""},{"line_number":1327,"context_line":"        # `node` is the actual StateMachineInstance, in this case, AwsInstance"},{"line_number":1328,"context_line":"        self.assertTrue(node.node_properties[\u0027spot\u0027])"},{"line_number":1329,"context_line":""},{"line_number":1330,"context_line":"    def test_aws_dedicated_host(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"b19b0f37_8754f782","line":1327,"in_reply_to":"721fc5d5_bca59cba","updated":"2024-10-04 15:28:29.000000000","message":"Acknowledged","commit_id":"18dd3b719648ebd31ddbe823b19bdf6369899e37"}],"nodepool/zk/zookeeper.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"ad7739cac4b3a0f662614e35ddb983c5dc1a315a","unresolved":true,"context_lines":[{"line_number":560,"context_line":"        self.tenant_name \u003d None"},{"line_number":561,"context_line":"        self.driver_data \u003d None"},{"line_number":562,"context_line":"        self.requestor \u003d None"},{"line_number":563,"context_line":"        self.label_mods \u003d {}"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    def __repr__(self):"},{"line_number":566,"context_line":"        d \u003d self.toDict()"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f4001f7_79bc8b20","line":563,"updated":"2024-09-16 13:58:24.000000000","message":"I think we should also add/extend a test to make sure this is correctly serialized to Zookeeper as this will be part of the Zuul-Nodepool API.","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"},{"author":{"_account_id":37071,"name":"Tudor-Stefan Tabacel-Manea","display_name":"Tudor Tabacel","email":"tudor-stefan.tabacel-manea@bmw.de","username":"ttabacel"},"change_message_id":"775d63ef19c7eea82ac0a73eb0f793b70ddee88e","unresolved":false,"context_lines":[{"line_number":560,"context_line":"        self.tenant_name \u003d None"},{"line_number":561,"context_line":"        self.driver_data \u003d None"},{"line_number":562,"context_line":"        self.requestor \u003d None"},{"line_number":563,"context_line":"        self.label_mods \u003d {}"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    def __repr__(self):"},{"line_number":566,"context_line":"        d \u003d self.toDict()"}],"source_content_type":"text/x-python","patch_set":2,"id":"2b8f73a3_c520bd9c","line":563,"in_reply_to":"9f4001f7_79bc8b20","updated":"2024-09-16 19:45:09.000000000","message":"Done","commit_id":"fa6a31a1cb62e1579c1064960c9e750a8a7fca5f"}]}
