)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"56189be7_417fb78c","updated":"2022-03-16 18:26:06.000000000","message":"+2 because I\u0027m sure this change works, but I did have questions to help me understand some of the API and usage expectations here.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"}],"doc/source/aws.rst":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":324,"context_line":"         ``ansible_python_interpreter``.  The special value ``auto`` will"},{"line_number":325,"context_line":"         direct Zuul to use inbuilt Ansible logic to select the"},{"line_number":326,"context_line":"         interpreter on Ansible \u003e\u003d2.8, and default to"},{"line_number":327,"context_line":"         ``/usr/bin/python2`` for earlier versions."},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"      .. attr:: shell-type"},{"line_number":330,"context_line":"         :type: str"}],"source_content_type":"text/x-rst","patch_set":2,"id":"4058c716_1e6526de","line":327,"updated":"2022-03-16 18:50:25.000000000","message":"Oh yep.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"617fc7f741e7d1e7b0efcab8936dc282ecf2eea9","unresolved":true,"context_lines":[{"line_number":324,"context_line":"         ``ansible_python_interpreter``.  The special value ``auto`` will"},{"line_number":325,"context_line":"         direct Zuul to use inbuilt Ansible logic to select the"},{"line_number":326,"context_line":"         interpreter on Ansible \u003e\u003d2.8, and default to"},{"line_number":327,"context_line":"         ``/usr/bin/python2`` for earlier versions."},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"      .. attr:: shell-type"},{"line_number":330,"context_line":"         :type: str"}],"source_content_type":"text/x-rst","patch_set":2,"id":"25b030bf_7d121024","line":327,"updated":"2022-02-23 23:02:32.000000000","message":"nit: Ansible 2.7 has been removed in Zuul 4.0 so we might not need to mention this case anymore.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"}],"nodepool/driver/aws/__init__.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from nodepool.driver.statemachine import StateMachineDriver"},{"line_number":19,"context_line":"from nodepool.driver.aws.config import AwsProviderConfig"},{"line_number":20,"context_line":"from nodepool.driver.aws.adapter import AwsAdapter"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AwsDriver(StateMachineDriver):"}],"source_content_type":"text/x-python","patch_set":2,"id":"567fccc4_86c67731","line":20,"updated":"2022-03-16 18:26:06.000000000","message":"It seems the old provider.py file was not removed even though it is no longer used here.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"}],"nodepool/driver/aws/adapter.py":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    def listResources(self):"},{"line_number":198,"context_line":"        self._tagAmis()"},{"line_number":199,"context_line":"        self._tagSnapshots()"},{"line_number":200,"context_line":"        for instance in self._listInstances():"},{"line_number":201,"context_line":"            if instance.state[\"Name\"].lower() \u003d\u003d \"terminated\":"},{"line_number":202,"context_line":"                continue"},{"line_number":203,"context_line":"            yield AwsResource(tag_list_to_dict(instance.tags),"}],"source_content_type":"text/x-python","patch_set":2,"id":"780c78db_d5335778","line":200,"updated":"2022-03-16 18:26:06.000000000","message":"Nit could call listInstances() here.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def listResources(self):"},{"line_number":198,"context_line":"        self._tagAmis()"},{"line_number":199,"context_line":"        self._tagSnapshots()"},{"line_number":200,"context_line":"        for instance in self._listInstances():"},{"line_number":201,"context_line":"            if instance.state[\"Name\"].lower() \u003d\u003d \"terminated\":"},{"line_number":202,"context_line":"                continue"},{"line_number":203,"context_line":"            yield AwsResource(tag_list_to_dict(instance.tags),"}],"source_content_type":"text/x-python","patch_set":2,"id":"b453e922_ab0fb5cb","line":200,"updated":"2022-03-16 18:50:25.000000000","message":"Well, that creates an AwsInstance object instead of an AwsResource; which is probably fine, but it may have some extra requirements which I wouldn\u0027t want to guarantee we could fulfill here, since we may be dealing with leaked instances so all the requirements may not be present.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"617fc7f741e7d1e7b0efcab8936dc282ecf2eea9","unresolved":true,"context_lines":[{"line_number":251,"context_line":"                ServiceCode\u003d\u0027ec2\u0027,"},{"line_number":252,"context_line":"                QuotaCode\u003d\u0027L-1216C47A\u0027"},{"line_number":253,"context_line":"            )"},{"line_number":254,"context_line":"            cores \u003d response[\u0027Quota\u0027][\u0027Value\u0027]"},{"line_number":255,"context_line":"        return QuotaInformation(cores\u003dcores,"},{"line_number":256,"context_line":"                                default\u003dmath.inf)"},{"line_number":257,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ffa6e079_83d9ef19","line":254,"updated":"2022-02-23 23:02:32.000000000","message":"One thought about quotas. AWS has separated quotas for some instance families. For the start I think this quota code is fine andcovers the most used instance types.\n\nI wonder if we could improve that later by mapping the quotas to codes-standard for the standard images and cores-p for p instances and so on. This way we could generically cover the individual quotas for the currently 8 different instance quotas.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":251,"context_line":"                ServiceCode\u003d\u0027ec2\u0027,"},{"line_number":252,"context_line":"                QuotaCode\u003d\u0027L-1216C47A\u0027"},{"line_number":253,"context_line":"            )"},{"line_number":254,"context_line":"            cores \u003d response[\u0027Quota\u0027][\u0027Value\u0027]"},{"line_number":255,"context_line":"        return QuotaInformation(cores\u003dcores,"},{"line_number":256,"context_line":"                                default\u003dmath.inf)"},{"line_number":257,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2913b7a0_cb5cf63f","line":254,"updated":"2022-03-16 18:50:25.000000000","message":"Yeah, the quota system is pretty flexible (thanks!), I think that might be a nice improvement.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        except Exception:"},{"line_number":323,"context_line":"            self.log.exception(\"Error tagging AMI:\")"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"        # Tag the snapshot"},{"line_number":326,"context_line":"        try:"},{"line_number":327,"context_line":"            with self.non_mutating_rate_limiter:"},{"line_number":328,"context_line":"                snap \u003d self.ec2.Snapshot("}],"source_content_type":"text/x-python","patch_set":2,"id":"1e5948eb_0c3bae95","line":325,"updated":"2022-03-16 18:50:25.000000000","message":"Internal AWS implementation detail: AFAICT, this is actually implemented in AWS by booting an instance from this image, then they run some stuff on it, and take a snapshot.\n\nAn interesting side effect of this is that you can\u0027t really upload an image that doesn\u0027t closely correspond to an image they already support natively.  Especially the kernel must be a supported version.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        except Exception:"},{"line_number":323,"context_line":"            self.log.exception(\"Error tagging AMI:\")"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"        # Tag the snapshot"},{"line_number":326,"context_line":"        try:"},{"line_number":327,"context_line":"            with self.non_mutating_rate_limiter:"},{"line_number":328,"context_line":"                snap \u003d self.ec2.Snapshot("}],"source_content_type":"text/x-python","patch_set":2,"id":"1921f417_ccf7236a","line":325,"updated":"2022-03-16 18:26:06.000000000","message":"Why is there a snapshot if we are directly uploading an AMI? Is this intentional due to some api oddity or is this something that should be removed?","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    # Local implementation below"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def _tagAmis(self):"},{"line_number":345,"context_line":"        # There is no way to tag imported AMIs, so this routine"},{"line_number":346,"context_line":"        # \"eventually\" tags them.  We look for any AMIs without tags"},{"line_number":347,"context_line":"        # which correspond to import tasks, and we copy the tags from"}],"source_content_type":"text/x-python","patch_set":2,"id":"8cb253dd_dd44152b","line":344,"updated":"2022-03-16 18:50:25.000000000","message":"I thought about that, but the tagging action in uploadImage is so straightforward (because we actually have the IDs in that case) and this is so complex (because we have to guess them!) that it seemed better to have different routines, even if they are slightly redundant.  Hopefully these are noops 99% of the time.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    # Local implementation below"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def _tagAmis(self):"},{"line_number":345,"context_line":"        # There is no way to tag imported AMIs, so this routine"},{"line_number":346,"context_line":"        # \"eventually\" tags them.  We look for any AMIs without tags"},{"line_number":347,"context_line":"        # which correspond to import tasks, and we copy the tags from"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f77cda3_967cbfa5","line":344,"updated":"2022-03-16 18:26:06.000000000","message":"This method and the next one seem a bit at odds with what the uploadImage does. It applies tags to both AMIs and snapshots. Should we maybe call these methods from uploadImage instead to brute for this a bit more?","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":366,"context_line":"                else:"},{"line_number":367,"context_line":"                    self.not_our_images.add(ami.id)"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    def _tagSnapshots(self):"},{"line_number":370,"context_line":"        # See comments for _tagAmis"},{"line_number":371,"context_line":"        for snap in self._listSnapshots():"},{"line_number":372,"context_line":"            if (\u0027import-ami-\u0027 in snap.description and"}],"source_content_type":"text/x-python","patch_set":2,"id":"ef4dc492_361bac73","line":369,"updated":"2022-03-16 18:26:06.000000000","message":"See above.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        return image_id"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"    @cachetools.func.lru_cache(maxsize\u003dNone)"},{"line_number":501,"context_line":"    def _getImage(self, image_id):"},{"line_number":502,"context_line":"        with self.non_mutating_rate_limiter:"},{"line_number":503,"context_line":"            return self.ec2.Image(image_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a82291a9_c63e5343","line":500,"updated":"2022-03-16 18:50:25.000000000","message":"I guess we could.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        return image_id"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"    @cachetools.func.lru_cache(maxsize\u003dNone)"},{"line_number":501,"context_line":"    def _getImage(self, image_id):"},{"line_number":502,"context_line":"        with self.non_mutating_rate_limiter:"},{"line_number":503,"context_line":"            return self.ec2.Image(image_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"2f9a679e_d4e41490","line":500,"range":{"start_line":500,"start_character":31,"end_line":500,"end_character":43},"updated":"2022-03-16 18:26:06.000000000","message":"Why not limit this to a few thousand entries?","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":573,"context_line":"                    mapping[\u0027Ebs\u0027][\u0027VolumeType\u0027] \u003d label.volume_type"},{"line_number":574,"context_line":"                # If the AMI is a snapshot, we cannot supply an \"encrypted\""},{"line_number":575,"context_line":"                # parameter"},{"line_number":576,"context_line":"                if \u0027Encrypted\u0027 in mapping[\u0027Ebs\u0027]:"},{"line_number":577,"context_line":"                    del mapping[\u0027Ebs\u0027][\u0027Encrypted\u0027]"},{"line_number":578,"context_line":"                args[\u0027BlockDeviceMappings\u0027] \u003d [mapping]"},{"line_number":579,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d40d4f9d_51a88218","line":576,"updated":"2022-03-16 18:26:06.000000000","message":"Should we check if it is a snapshot before removing the flag?","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"a225229ca076b26012cd0c015aee6076b470c764","unresolved":false,"context_lines":[{"line_number":573,"context_line":"                    mapping[\u0027Ebs\u0027][\u0027VolumeType\u0027] \u003d label.volume_type"},{"line_number":574,"context_line":"                # If the AMI is a snapshot, we cannot supply an \"encrypted\""},{"line_number":575,"context_line":"                # parameter"},{"line_number":576,"context_line":"                if \u0027Encrypted\u0027 in mapping[\u0027Ebs\u0027]:"},{"line_number":577,"context_line":"                    del mapping[\u0027Ebs\u0027][\u0027Encrypted\u0027]"},{"line_number":578,"context_line":"                args[\u0027BlockDeviceMappings\u0027] \u003d [mapping]"},{"line_number":579,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"be1078a9_e9d420d1","line":576,"updated":"2022-03-16 18:50:25.000000000","message":"This bit was copy-pasta from the old driver.  That does sound like a good improvement.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"}],"nodepool/tests/fixtures/aws-bad-config-images.yaml":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"d97bea17eb39754fa84f5651002c8f95166bf2d5","unresolved":true,"context_lines":[{"line_number":16,"context_line":"    region-name: us-west-2"},{"line_number":17,"context_line":"    cloud-images:"},{"line_number":18,"context_line":"      - name: ubuntu1404-bad-config"},{"line_number":19,"context_line":"        image-id: ami-1e749f67"},{"line_number":20,"context_line":"        image-filters:"},{"line_number":21,"context_line":"          - name: name"},{"line_number":22,"context_line":"            values:"},{"line_number":23,"context_line":"              - ubuntu*"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"2b7dc530_a2adcb30","line":20,"range":{"start_line":19,"start_character":1,"end_line":20,"end_character":22},"updated":"2022-03-16 18:26:06.000000000","message":"These two are exclusive options and generate the error.","commit_id":"43678bf4c1a4fe507c7fe03b7ac285f5325ea0b6"}]}