)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f70b9087c42fbfeea696e57ddcf3ae7b7ddb221b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Add typing information"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As with the \u0027nova.virt.hardware\u0027 module, add typing information here now"},{"line_number":10,"context_line":"so that we can use it during development of later features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Part of blueprint use-pcpu-and-vcpu-in-one-instance"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1f493fa4_061da072","line":10,"updated":"2020-05-07 18:55:24.000000000","message":"I guess my gripe would be, you\u0027re not *just* adding type information, you\u0027re also refactoring certain bits of code to avoid confusing mypy.","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"38fe2523226e4bab53dbf7b52f21d134315ad062","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Add typing information"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As with the \u0027nova.virt.hardware\u0027 module, add typing information here now"},{"line_number":10,"context_line":"so that we can use it during development of later features."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Part of blueprint use-pcpu-and-vcpu-in-one-instance"},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1f493fa4_2517e953","line":10,"in_reply_to":"1f493fa4_061da072","updated":"2020-05-15 12:59:55.000000000","message":"Done","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"30532ec84edbc0595e84e8fa65c92528263a5612","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"As with the \u0027nova.virt.hardware\u0027 module, add typing information here now"},{"line_number":10,"context_line":"so that we can use it during development of later features. This"},{"line_number":11,"context_line":"requires some minor tweaks of code that mypy found confusing."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Part of blueprint use-pcpu-and-vcpu-in-one-instance"},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1f493fa4_d2a364b0","line":11,"updated":"2020-05-08 13:34:46.000000000","message":"\\o/","commit_id":"34e6041ba9df551d552cb60e90285ff893fa1026"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d6c3cdfbafa0c15447fb6f815705667e9f937c95","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"            raise exception.LiveMigrationURINotAvailable(virt_type\u003dvirt_type)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"        str_format \u003d (dest,)"},{"line_number":1043,"context_line":"        if virt_type in (\u0027kvm\u0027, \u0027qemu\u0027):"},{"line_number":1044,"context_line":"            scheme \u003d CONF.libvirt.live_migration_scheme or \u0027tcp\u0027"},{"line_number":1045,"context_line":"            str_format \u003d (scheme, dest)"},{"line_number":1046,"context_line":"        return uris.get(virt_type) % str_format"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_672bd82f","side":"PARENT","line":1043,"range":{"start_line":1043,"start_character":8,"end_line":1043,"end_character":40},"updated":"2020-03-26 02:11:49.000000000","message":"ok...that is safe. but it isn\u0027t about typing...","commit_id":"4cb49a8a70fe49d8e918fe3d08923364c1d27f7d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"c599003347add853fbd0971b744e8be9d9b2e664","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"            raise exception.LiveMigrationURINotAvailable(virt_type\u003dvirt_type)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"        str_format \u003d (dest,)"},{"line_number":1043,"context_line":"        if virt_type in (\u0027kvm\u0027, \u0027qemu\u0027):"},{"line_number":1044,"context_line":"            scheme \u003d CONF.libvirt.live_migration_scheme or \u0027tcp\u0027"},{"line_number":1045,"context_line":"            str_format \u003d (scheme, dest)"},{"line_number":1046,"context_line":"        return uris.get(virt_type) % str_format"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_d2028e09","side":"PARENT","line":1043,"range":{"start_line":1043,"start_character":8,"end_line":1043,"end_character":40},"in_reply_to":"df33271e_5b9cad5c","updated":"2020-04-06 07:53:31.000000000","message":"thanks, I see the case now","commit_id":"4cb49a8a70fe49d8e918fe3d08923364c1d27f7d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"30c02a01c50f8956e386f0dfeedab0bc41cbb094","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"            raise exception.LiveMigrationURINotAvailable(virt_type\u003dvirt_type)"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"        str_format \u003d (dest,)"},{"line_number":1043,"context_line":"        if virt_type in (\u0027kvm\u0027, \u0027qemu\u0027):"},{"line_number":1044,"context_line":"            scheme \u003d CONF.libvirt.live_migration_scheme or \u0027tcp\u0027"},{"line_number":1045,"context_line":"            str_format \u003d (scheme, dest)"},{"line_number":1046,"context_line":"        return uris.get(virt_type) % str_format"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_5b9cad5c","side":"PARENT","line":1043,"range":{"start_line":1043,"start_character":8,"end_line":1043,"end_character":40},"in_reply_to":"df33271e_672bd82f","updated":"2020-03-26 10:08:45.000000000","message":"It actually is:\n\n\tnova/virt/libvirt/driver.py: note: In member \"_live_migration_uri\" of class \"LibvirtDriver\":\n\tnova/virt/libvirt/driver.py:1049:27: error: Incompatible types in assignment (expression has type \"Tuple[Union[Any, str], Any]\", variable has type \"Tuple[Any]\")\n\tnova/virt/libvirt/driver.py:1051:16: error: Unsupported left operand type for % (\"None\")\n\tnova/virt/libvirt/driver.py:1051:38: note: Left operand is of type \"Optional[str]\"\n\tFound 2 errors in 1 file (checked 2 source files)\n\nmypy is a bit dumb and takes the first declaration of a variable to be its type. I can manually override it but this seemed clearer.","commit_id":"4cb49a8a70fe49d8e918fe3d08923364c1d27f7d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d6c3cdfbafa0c15447fb6f815705667e9f937c95","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1385,"context_line":"        for path in ["},{"line_number":1386,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1387,"context_line":"        ]:"},{"line_number":1388,"context_line":"            dmcrypt.delete_volume(path)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"    def _get_serial_ports_from_guest(self, guest, mode\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_e7c40862","line":1387,"updated":"2020-03-26 02:11:49.000000000","message":"this isn\u0027t about typing...","commit_id":"0737fa77df4e8c1913d9da767bde270b40d1354c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"30c02a01c50f8956e386f0dfeedab0bc41cbb094","unresolved":false,"context_lines":[{"line_number":1384,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1385,"context_line":"        for path in ["},{"line_number":1386,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1387,"context_line":"        ]:"},{"line_number":1388,"context_line":"            dmcrypt.delete_volume(path)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"    def _get_serial_ports_from_guest(self, guest, mode\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_7bb251e8","line":1387,"in_reply_to":"df33271e_e7c40862","updated":"2020-03-26 10:08:45.000000000","message":"Afraid it is. Unchanged, this is:\n\n  nova/virt/libvirt/driver.py: note: In member \"_detach_encrypted_volumes\" of class \"LibvirtDriver\":\n  nova/virt/libvirt/driver.py:1385:29: error: Need type annotation for \u0027encrypted_volumes\u0027\n  Found 1 error in 1 file (checked 2 source files)\n\nBy changing this, I don\u0027t need those annotations and it\u0027s clearer.","commit_id":"0737fa77df4e8c1913d9da767bde270b40d1354c"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"d6c3cdfbafa0c15447fb6f815705667e9f937c95","unresolved":false,"context_lines":[{"line_number":2905,"context_line":"                   % volume_id)"},{"line_number":2906,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2907,"context_line":""},{"line_number":2908,"context_line":"        if my_dev is None or (active_disk is None and active_protocol is None):"},{"line_number":2909,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2910,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2911,"context_line":"                   % volume_id)"},{"line_number":2912,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2913,"context_line":""},{"line_number":2914,"context_line":"        LOG.debug(\"found device at %s\", my_dev, instance\u003dinstance)"},{"line_number":2915,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_07cacc6b","line":2912,"range":{"start_line":2908,"start_character":8,"end_line":2912,"end_character":46},"updated":"2020-03-26 02:11:49.000000000","message":"so you want to remove this?","commit_id":"0737fa77df4e8c1913d9da767bde270b40d1354c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"30c02a01c50f8956e386f0dfeedab0bc41cbb094","unresolved":false,"context_lines":[{"line_number":2905,"context_line":"                   % volume_id)"},{"line_number":2906,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2907,"context_line":""},{"line_number":2908,"context_line":"        if my_dev is None or (active_disk is None and active_protocol is None):"},{"line_number":2909,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2910,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2911,"context_line":"                   % volume_id)"},{"line_number":2912,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2913,"context_line":""},{"line_number":2914,"context_line":"        LOG.debug(\"found device at %s\", my_dev, instance\u003dinstance)"},{"line_number":2915,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df33271e_7be7f1e5","line":2912,"range":{"start_line":2908,"start_character":8,"end_line":2912,"end_character":46},"in_reply_to":"df33271e_07cacc6b","updated":"2020-03-26 10:08:45.000000000","message":"I don\u0027t think so. I think it\u0027s possible that I could find a disk whose serial \u003d\u003d volume_id but that doesn\u0027t have source_path or source_protocol configured, right?","commit_id":"0737fa77df4e8c1913d9da767bde270b40d1354c"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"28a135b0ef37cb44f20048bb16d8ada42681e8e4","unresolved":false,"context_lines":[{"line_number":10495,"context_line":"            block_device_obj, mapping\u003dinstance_info[\u0027mapping\u0027])"},{"line_number":10496,"context_line":"        return block_device.prepend_dev(disk_info[\u0027dev\u0027])"},{"line_number":10497,"context_line":""},{"line_number":10498,"context_line":"    def is_supported_fs_format(self, fs_type) -\u003e bool:"},{"line_number":10499,"context_line":"        return fs_type in [nova.privsep.fs.FS_FORMAT_EXT2,"},{"line_number":10500,"context_line":"                           nova.privsep.fs.FS_FORMAT_EXT3,"},{"line_number":10501,"context_line":"                           nova.privsep.fs.FS_FORMAT_EXT4,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_37a60712","line":10498,"updated":"2020-03-27 11:19:16.000000000","message":"The type hint has side effect for polluting the \u0027__annotation__\u0027 feild of function. And we have at least one test,the \u0027test_public_api_signatures\u0027, checks these feilds. \nThis test checks the consistence of the common/derived methods between this \u0027LibvirtDriver\u0027 class and its base class, the \u0027driver.ComputeDriver\u0027 class.\n\nMaybe you need also add these kinf of type hints for the same functions in ComputeDriver, and maybe for all classes based on ComputeDriver.\n\nnova.tests.unit.virt.libvirt.test_driver.LibvirtConnTestCase.test_public_api_signatures currently reports a failure.","commit_id":"6dd1505af3a51dc5da1aef315346220b1b1a0f9c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"38fe2523226e4bab53dbf7b52f21d134315ad062","unresolved":false,"context_lines":[{"line_number":10495,"context_line":"            block_device_obj, mapping\u003dinstance_info[\u0027mapping\u0027])"},{"line_number":10496,"context_line":"        return block_device.prepend_dev(disk_info[\u0027dev\u0027])"},{"line_number":10497,"context_line":""},{"line_number":10498,"context_line":"    def is_supported_fs_format(self, fs_type) -\u003e bool:"},{"line_number":10499,"context_line":"        return fs_type in [nova.privsep.fs.FS_FORMAT_EXT2,"},{"line_number":10500,"context_line":"                           nova.privsep.fs.FS_FORMAT_EXT3,"},{"line_number":10501,"context_line":"                           nova.privsep.fs.FS_FORMAT_EXT4,"}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_127791a2","line":10498,"in_reply_to":"df33271e_37a60712","updated":"2020-05-15 12:59:55.000000000","message":"Done","commit_id":"6dd1505af3a51dc5da1aef315346220b1b1a0f9c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f70b9087c42fbfeea696e57ddcf3ae7b7ddb221b","unresolved":false,"context_lines":[{"line_number":1043,"context_line":"            raise exception.LiveMigrationURINotAvailable(virt_type\u003dvirt_type)"},{"line_number":1044,"context_line":""},{"line_number":1045,"context_line":"        str_format \u003d {"},{"line_number":1046,"context_line":"            \u0027dest\u0027: dest,"},{"line_number":1047,"context_line":"            \u0027scheme\u0027: CONF.libvirt.live_migration_scheme or \u0027tcp\u0027,"},{"line_number":1048,"context_line":"        }"},{"line_number":1049,"context_line":"        return uri % str_format"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f493fa4_c68d78d4","line":1046,"updated":"2020-05-07 18:55:24.000000000","message":"Note: despite looking unrelated, this is actually needed to avoid confusing mypy (and is cleaner anyways)","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f70b9087c42fbfeea696e57ddcf3ae7b7ddb221b","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"    def _detach_encrypted_volumes(self, instance, block_device_info):"},{"line_number":1382,"context_line":"        \"\"\"Detaches encrypted volumes attached to instance.\"\"\""},{"line_number":1383,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1384,"context_line":"        for path in ["},{"line_number":1385,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1386,"context_line":"        ]:"},{"line_number":1387,"context_line":"            dmcrypt.delete_volume(path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f493fa4_a624acc7","line":1384,"updated":"2020-05-07 18:55:24.000000000","message":"This doesn\u0027t look related either at first glance, but I\u0027m going to trust Stephen and not verify every single one of these.","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"38fe2523226e4bab53dbf7b52f21d134315ad062","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"    def _detach_encrypted_volumes(self, instance, block_device_info):"},{"line_number":1382,"context_line":"        \"\"\"Detaches encrypted volumes attached to instance.\"\"\""},{"line_number":1383,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1384,"context_line":"        for path in ["},{"line_number":1385,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1386,"context_line":"        ]:"},{"line_number":1387,"context_line":"            dmcrypt.delete_volume(path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f493fa4_253ec9d9","line":1384,"in_reply_to":"1f493fa4_a624acc7","updated":"2020-05-15 12:59:55.000000000","message":"Yeah, mypy didn\u0027t like filter","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"f70b9087c42fbfeea696e57ddcf3ae7b7ddb221b","unresolved":false,"context_lines":[{"line_number":2912,"context_line":"                active_protocol \u003d guest_disk.source_protocol"},{"line_number":2913,"context_line":"                active_disk_object \u003d guest_disk"},{"line_number":2914,"context_line":"                break"},{"line_number":2915,"context_line":"        else:"},{"line_number":2916,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2917,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2918,"context_line":"                   % volume_id)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f493fa4_112b1882","line":2915,"updated":"2020-05-07 18:55:24.000000000","message":"Huh, TIL for/else is a thing. Perhaps a bit counter-intuitively, the else gets hit if no `break` triggered during the loop, which makes sense here.","commit_id":"ac1d58bedeedfed22c4c07d5423b5b2cc657d174"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                    MIN_LIBVIRT_PMEM_SUPPORT)})"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # vpmem keyed by name {name: objects.LibvirtVPMEMDevice,...}"},{"line_number":463,"context_line":"        vpmems_by_name: ty.Dict[str, \u0027objects.LibvirtVPMEMDevice\u0027] \u003d {}"},{"line_number":464,"context_line":"        # vpmem list keyed by resource class"},{"line_number":465,"context_line":"        # {\u0027RC_0\u0027: [objects.LibvirtVPMEMDevice, ...], \u0027RC_1\u0027: [...]}"},{"line_number":466,"context_line":"        vpmems_by_rc: ty.Dict[str, ty.List[\u0027objects.LibvirtVPMEMDevice\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_342134b6","line":463,"updated":"2020-06-16 12:38:03.000000000","message":"I\u0027m curious. How does conformance to \u0027objects.LibvirtVPMEMDevice\u0027 is validated by mypy? I tried to change this to foo and mypy caught it, but I then changed it to objects.RequestSpec and mypy accepted that.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7335222a7b6027373759949885481ff8e6ba79","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                    MIN_LIBVIRT_PMEM_SUPPORT)})"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # vpmem keyed by name {name: objects.LibvirtVPMEMDevice,...}"},{"line_number":463,"context_line":"        vpmems_by_name: ty.Dict[str, \u0027objects.LibvirtVPMEMDevice\u0027] \u003d {}"},{"line_number":464,"context_line":"        # vpmem list keyed by resource class"},{"line_number":465,"context_line":"        # {\u0027RC_0\u0027: [objects.LibvirtVPMEMDevice, ...], \u0027RC_1\u0027: [...]}"},{"line_number":466,"context_line":"        vpmems_by_rc: ty.Dict[str, ty.List[\u0027objects.LibvirtVPMEMDevice\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_67b8a0e4","line":463,"in_reply_to":"bf51134e_12d4b51f","updated":"2020-06-23 10:48:40.000000000","message":"OK that explains it. Sorry for being picky here. I just wanted get more understanding what mypy enforces.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                    MIN_LIBVIRT_PMEM_SUPPORT)})"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # vpmem keyed by name {name: objects.LibvirtVPMEMDevice,...}"},{"line_number":463,"context_line":"        vpmems_by_name: ty.Dict[str, \u0027objects.LibvirtVPMEMDevice\u0027] \u003d {}"},{"line_number":464,"context_line":"        # vpmem list keyed by resource class"},{"line_number":465,"context_line":"        # {\u0027RC_0\u0027: [objects.LibvirtVPMEMDevice, ...], \u0027RC_1\u0027: [...]}"},{"line_number":466,"context_line":"        vpmems_by_rc: ty.Dict[str, ty.List[\u0027objects.LibvirtVPMEMDevice\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_db1bb118","line":463,"in_reply_to":"bf51134e_342134b6","updated":"2020-06-17 10:56:33.000000000","message":"The reason for the string is to delay interpolation. The \u0027nova.objects\u0027 module doesn\u0027t have a \u0027LibvirtVPMEMDevice\u0027 attribute initially. This is only set on the module when you call \u0027nova.objects.register_all()\u0027. If we don\u0027t quote the string, it will try to import the module which will fail. You\u0027d see a similar thing if you tried to import the module from the REPL. We don\u0027t see this in tests or production code because we call \u0027register_all()\u0027 early in the code (e.g. [1], [2], [3], ...). mypy doesn\u0027t actually run the code so it\u0027s not possible to do this (believe me, I\u0027ve tried a lot of approaches)\n\n[1] https://github.com/openstack/nova/blob/21.0.0/nova/tests/unit/__init__.py#L27-L30\n[2] https://github.com/openstack/nova/blob/21.0.0/nova/cmd/compute.py#L46\n[3] https://github.com/openstack/nova/blob/21.0.0/nova/cmd/conductor.py#L37","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7c7333a19e3e78d5f6b2f11d2fcc0f3007411a0","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                    MIN_LIBVIRT_PMEM_SUPPORT)})"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # vpmem keyed by name {name: objects.LibvirtVPMEMDevice,...}"},{"line_number":463,"context_line":"        vpmems_by_name: ty.Dict[str, \u0027objects.LibvirtVPMEMDevice\u0027] \u003d {}"},{"line_number":464,"context_line":"        # vpmem list keyed by resource class"},{"line_number":465,"context_line":"        # {\u0027RC_0\u0027: [objects.LibvirtVPMEMDevice, ...], \u0027RC_1\u0027: [...]}"},{"line_number":466,"context_line":"        vpmems_by_rc: ty.Dict[str, ty.List[\u0027objects.LibvirtVPMEMDevice\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_12d4b51f","line":463,"in_reply_to":"bf51134e_a0bb2270","updated":"2020-06-19 09:43:58.000000000","message":"Ah, that\u0027s because we don\u0027t have type checking enabled for the \u0027nova.objects\u0027 module. Once we do (which will admittedly take time, due to the dynamic nature of that module) this will be resolved. It\u0027s not ideal, but for now think of these as mostly hints.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"745876bce0addec9b84f87825b05c45b44f4d239","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                    MIN_LIBVIRT_PMEM_SUPPORT)})"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # vpmem keyed by name {name: objects.LibvirtVPMEMDevice,...}"},{"line_number":463,"context_line":"        vpmems_by_name: ty.Dict[str, \u0027objects.LibvirtVPMEMDevice\u0027] \u003d {}"},{"line_number":464,"context_line":"        # vpmem list keyed by resource class"},{"line_number":465,"context_line":"        # {\u0027RC_0\u0027: [objects.LibvirtVPMEMDevice, ...], \u0027RC_1\u0027: [...]}"},{"line_number":466,"context_line":"        vpmems_by_rc: ty.Dict[str, ty.List[\u0027objects.LibvirtVPMEMDevice\u0027]] \u003d ("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_a0bb2270","line":463,"in_reply_to":"bf51134e_db1bb118","updated":"2020-06-17 16:32:48.000000000","message":"Ahh got it. The ovos are dynamically put into the objects namespace. \n\nStill something does not add up. I can modify this type definition here and mypy does not cry. I can even remove quotes around it and mypy does not fail that the type is not importable. It accepts event objects.foo instead of \u0027objects.LibvirtVPMEMDevice\u0027.\n\nI even put a \n\n  a: int \u003d \"help\"\n\nhere and mypy did not caught the mismatch running the tox -e mypy target. Is is something with my env?\n\n// Later\n\nThere is a .mypy_cache dir that needs to be deleted between runs as otherwise mypy does not detect the changes in the source files. But still it only detects the a:int\u003d\"help\" error does not detect if I say objects.foo in the vpmems_by_name declaration.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        str_format \u003d {"},{"line_number":1083,"context_line":"            \u0027dest\u0027: dest,"},{"line_number":1084,"context_line":"            \u0027scheme\u0027: CONF.libvirt.live_migration_scheme or \u0027tcp\u0027,"},{"line_number":1085,"context_line":"        }"},{"line_number":1086,"context_line":"        return uri % str_format"},{"line_number":1087,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_373816d2","line":1084,"updated":"2020-06-16 12:38:03.000000000","message":"scheme will be unused in case of xen and parallels, but that does not cause any error.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        str_format \u003d {"},{"line_number":1083,"context_line":"            \u0027dest\u0027: dest,"},{"line_number":1084,"context_line":"            \u0027scheme\u0027: CONF.libvirt.live_migration_scheme or \u0027tcp\u0027,"},{"line_number":1085,"context_line":"        }"},{"line_number":1086,"context_line":"        return uri % str_format"},{"line_number":1087,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_fbd79522","line":1084,"in_reply_to":"bf51134e_373816d2","updated":"2020-06-17 10:56:33.000000000","message":"Correct. This seemed the cleanest way to do this. mypy complained otherwise that scheme had two different types","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"745876bce0addec9b84f87825b05c45b44f4d239","unresolved":false,"context_lines":[{"line_number":1081,"context_line":""},{"line_number":1082,"context_line":"        str_format \u003d {"},{"line_number":1083,"context_line":"            \u0027dest\u0027: dest,"},{"line_number":1084,"context_line":"            \u0027scheme\u0027: CONF.libvirt.live_migration_scheme or \u0027tcp\u0027,"},{"line_number":1085,"context_line":"        }"},{"line_number":1086,"context_line":"        return uri % str_format"},{"line_number":1087,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_60d58a43","line":1084,"in_reply_to":"bf51134e_fbd79522","updated":"2020-06-17 16:32:48.000000000","message":"Your solution looks OK to me.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":1419,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1420,"context_line":"        for path in ["},{"line_number":1421,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1422,"context_line":"        ]:"},{"line_number":1423,"context_line":"            dmcrypt.delete_volume(path)"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"    def _get_serial_ports_from_guest(self, guest, mode\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_376656ef","line":1422,"updated":"2020-06-16 12:38:03.000000000","message":"I guess Dan will not be happy with this formatting","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":2952,"context_line":"                active_protocol \u003d guest_disk.source_protocol"},{"line_number":2953,"context_line":"                active_disk_object \u003d guest_disk"},{"line_number":2954,"context_line":"                break"},{"line_number":2955,"context_line":"        else:"},{"line_number":2956,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2957,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2958,"context_line":"                   % volume_id)"},{"line_number":2959,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2960,"context_line":""},{"line_number":2961,"context_line":"        if my_dev is None or (active_disk is None and active_protocol is None):"},{"line_number":2962,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2963,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2964,"context_line":"                   % volume_id)"},{"line_number":2965,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2966,"context_line":""},{"line_number":2967,"context_line":"        LOG.debug(\"found device at %s\", my_dev, instance\u003dinstance)"},{"line_number":2968,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_f7133e0c","line":2965,"range":{"start_line":2955,"start_character":0,"end_line":2965,"end_character":46},"updated":"2020-06-16 12:38:03.000000000","message":"I don\u0027t like this duplication. :/\n\nAs far as I understand mypy fails to recognize that if my_dev remains None (e.g. if the loop was noop), then we exit the function with an exception at L2965 so if we reach L2988 then my_dev is not None. \n\nMaybe if we pull out a function that does the search for my_dev that would help mypy out?","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7c7333a19e3e78d5f6b2f11d2fcc0f3007411a0","unresolved":false,"context_lines":[{"line_number":2952,"context_line":"                active_protocol \u003d guest_disk.source_protocol"},{"line_number":2953,"context_line":"                active_disk_object \u003d guest_disk"},{"line_number":2954,"context_line":"                break"},{"line_number":2955,"context_line":"        else:"},{"line_number":2956,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2957,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2958,"context_line":"                   % volume_id)"},{"line_number":2959,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2960,"context_line":""},{"line_number":2961,"context_line":"        if my_dev is None or (active_disk is None and active_protocol is None):"},{"line_number":2962,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2963,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2964,"context_line":"                   % volume_id)"},{"line_number":2965,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2966,"context_line":""},{"line_number":2967,"context_line":"        LOG.debug(\"found device at %s\", my_dev, instance\u003dinstance)"},{"line_number":2968,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_f22e61f8","line":2965,"range":{"start_line":2955,"start_character":0,"end_line":2965,"end_character":46},"in_reply_to":"bf51134e_2df8b4e3","updated":"2020-06-19 09:43:58.000000000","message":"Okay, moved the active_disk/active_protocol check inside the \u0027guest_disk.serial \u003d\u003d volume_id\u0027 conditional block. That should dedupe this again. Sorry for the noise","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":2952,"context_line":"                active_protocol \u003d guest_disk.source_protocol"},{"line_number":2953,"context_line":"                active_disk_object \u003d guest_disk"},{"line_number":2954,"context_line":"                break"},{"line_number":2955,"context_line":"        else:"},{"line_number":2956,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2957,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2958,"context_line":"                   % volume_id)"},{"line_number":2959,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2960,"context_line":""},{"line_number":2961,"context_line":"        if my_dev is None or (active_disk is None and active_protocol is None):"},{"line_number":2962,"context_line":"            LOG.debug(\u0027Domain XML: %s\u0027, xml, instance\u003dinstance)"},{"line_number":2963,"context_line":"            msg \u003d (_(\"Disk with id \u0027%s\u0027 not found attached to instance.\")"},{"line_number":2964,"context_line":"                   % volume_id)"},{"line_number":2965,"context_line":"            raise exception.InternalError(msg)"},{"line_number":2966,"context_line":""},{"line_number":2967,"context_line":"        LOG.debug(\"found device at %s\", my_dev, instance\u003dinstance)"},{"line_number":2968,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_2df8b4e3","line":2965,"range":{"start_line":2955,"start_character":0,"end_line":2965,"end_character":46},"in_reply_to":"bf51134e_f7133e0c","updated":"2020-06-17 10:56:33.000000000","message":"This one I\u0027m not so sure about. In theory, I shouldn\u0027t need the original block and the else block should catch everything for me, i.e. if we never trigger the \u0027guest_disk.serial \u003d\u003d volume_id\u0027 condition. However, I wasn\u0027t able to determine if we\u0027d ever hit a condition where \u0027guest_disk.serial\u0027 would equal \u0027volume_id\u0027, but \u0027guest_disk.target_dev\u0027, \u0027guest_disk.source_path\u0027 and \u0027guest_disk.source_protocol\u0027 would all be unset, so I left this here. If you\u0027ve suggestions for how to figure this out, I\u0027ll happily drop the original block in favor of the for-else block","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":3835,"context_line":"            target_partition \u003d None"},{"line_number":3836,"context_line":""},{"line_number":3837,"context_line":"        # Handles the key injection."},{"line_number":3838,"context_line":"        key \u003d None"},{"line_number":3839,"context_line":"        if CONF.libvirt.inject_key and instance.get(\u0027key_data\u0027):"},{"line_number":3840,"context_line":"            key \u003d str(instance.key_data)"},{"line_number":3841,"context_line":""},{"line_number":3842,"context_line":"        # Handles the admin password injection."},{"line_number":3843,"context_line":"        admin_pass \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_ba0a4d87","line":3840,"range":{"start_line":3838,"start_character":0,"end_line":3840,"end_character":40},"updated":"2020-06-16 12:38:03.000000000","message":"I don\u0027t like that mypy forces such style changes. From the python code perspective both version is correct and the key variable has the same type in both cases. It is just mypy that infers str type of a variable based on the type of the first assignment in the old code and does not allow None later. Still in the new code it allows both None and str for some reason.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"745876bce0addec9b84f87825b05c45b44f4d239","unresolved":false,"context_lines":[{"line_number":3835,"context_line":"            target_partition \u003d None"},{"line_number":3836,"context_line":""},{"line_number":3837,"context_line":"        # Handles the key injection."},{"line_number":3838,"context_line":"        key \u003d None"},{"line_number":3839,"context_line":"        if CONF.libvirt.inject_key and instance.get(\u0027key_data\u0027):"},{"line_number":3840,"context_line":"            key \u003d str(instance.key_data)"},{"line_number":3841,"context_line":""},{"line_number":3842,"context_line":"        # Handles the admin password injection."},{"line_number":3843,"context_line":"        admin_pass \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_00e76e14","line":3840,"range":{"start_line":3838,"start_character":0,"end_line":3840,"end_character":40},"in_reply_to":"bf51134e_9b949946","updated":"2020-06-17 16:32:48.000000000","message":"Yeah, the proposed code is correct and and equivalent with the old one.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":3835,"context_line":"            target_partition \u003d None"},{"line_number":3836,"context_line":""},{"line_number":3837,"context_line":"        # Handles the key injection."},{"line_number":3838,"context_line":"        key \u003d None"},{"line_number":3839,"context_line":"        if CONF.libvirt.inject_key and instance.get(\u0027key_data\u0027):"},{"line_number":3840,"context_line":"            key \u003d str(instance.key_data)"},{"line_number":3841,"context_line":""},{"line_number":3842,"context_line":"        # Handles the admin password injection."},{"line_number":3843,"context_line":"        admin_pass \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_9b949946","line":3840,"range":{"start_line":3838,"start_character":0,"end_line":3840,"end_character":40},"in_reply_to":"bf51134e_ba0a4d87","updated":"2020-06-17 10:56:33.000000000","message":"It seems okay with None -\u003e (not None) but not the other way around. With that said:\n\n  x \u003d \u003cdefault value\u003e\n  if \u003cconditional\u003e:\n      x \u003d \u003cother value\u003e\n\nis a reasonably common Python idiom. It\u0027s unfortunate that we\u0027ve to do it but it\u0027s not wrong","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":3840,"context_line":"            key \u003d str(instance.key_data)"},{"line_number":3841,"context_line":""},{"line_number":3842,"context_line":"        # Handles the admin password injection."},{"line_number":3843,"context_line":"        admin_pass \u003d None"},{"line_number":3844,"context_line":"        if CONF.libvirt.inject_password:"},{"line_number":3845,"context_line":"            admin_pass \u003d injection_info.admin_pass"},{"line_number":3846,"context_line":""},{"line_number":3847,"context_line":"        # Handles the network injection."},{"line_number":3848,"context_line":"        net \u003d netutils.get_injected_network_template("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_da13c1b7","line":3845,"range":{"start_line":3843,"start_character":0,"end_line":3845,"end_character":50},"updated":"2020-06-16 12:38:03.000000000","message":"I guess you change this just to be symmetric with the above change as mypy allows the old form too. I guess the type inference allows extending the type of a variable from None with something specific (e.g. str) but not the other way around.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":3840,"context_line":"            key \u003d str(instance.key_data)"},{"line_number":3841,"context_line":""},{"line_number":3842,"context_line":"        # Handles the admin password injection."},{"line_number":3843,"context_line":"        admin_pass \u003d None"},{"line_number":3844,"context_line":"        if CONF.libvirt.inject_password:"},{"line_number":3845,"context_line":"            admin_pass \u003d injection_info.admin_pass"},{"line_number":3846,"context_line":""},{"line_number":3847,"context_line":"        # Handles the network injection."},{"line_number":3848,"context_line":"        net \u003d netutils.get_injected_network_template("}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_db9e1162","line":3845,"range":{"start_line":3843,"start_character":0,"end_line":3845,"end_character":50},"in_reply_to":"bf51134e_da13c1b7","updated":"2020-06-17 10:56:33.000000000","message":"Yes, exactly.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":4796,"context_line":""},{"line_number":4797,"context_line":"    def _get_guest_idmaps(self):"},{"line_number":4798,"context_line":"        id_maps: ty.List[ty.Union["},{"line_number":4799,"context_line":"            vconfig.LibvirtConfigGuestUIDMap,"},{"line_number":4800,"context_line":"            vconfig.LibvirtConfigGuestGIDMap,"},{"line_number":4801,"context_line":"        ]] \u003d []"},{"line_number":4802,"context_line":"        if CONF.libvirt.virt_type \u003d\u003d \u0027lxc\u0027 and CONF.libvirt.uid_maps:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_da61013c","line":4799,"range":{"start_line":4799,"start_character":12,"end_line":4799,"end_character":44},"updated":"2020-06-16 12:38:03.000000000","message":"Related to my first question in this file what is the difference listing the type directly or as a string?","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":4796,"context_line":""},{"line_number":4797,"context_line":"    def _get_guest_idmaps(self):"},{"line_number":4798,"context_line":"        id_maps: ty.List[ty.Union["},{"line_number":4799,"context_line":"            vconfig.LibvirtConfigGuestUIDMap,"},{"line_number":4800,"context_line":"            vconfig.LibvirtConfigGuestGIDMap,"},{"line_number":4801,"context_line":"        ]] \u003d []"},{"line_number":4802,"context_line":"        if CONF.libvirt.virt_type \u003d\u003d \u0027lxc\u0027 and CONF.libvirt.uid_maps:"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_fb8e752c","line":4799,"range":{"start_line":4799,"start_character":12,"end_line":4799,"end_character":44},"in_reply_to":"bf51134e_da61013c","updated":"2020-06-17 10:56:33.000000000","message":"Unlike the above, \u0027LibvirtConfigGuestUIDMap\u0027 is an actual attribute of \u0027nova.virt.libvirt.config\u0027 and not something we register dynamically. As a result, we can use this without the string.","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":4797,"context_line":"    def _get_guest_idmaps(self):"},{"line_number":4798,"context_line":"        id_maps: ty.List[ty.Union["},{"line_number":4799,"context_line":"            vconfig.LibvirtConfigGuestUIDMap,"},{"line_number":4800,"context_line":"            vconfig.LibvirtConfigGuestGIDMap,"},{"line_number":4801,"context_line":"        ]] \u003d []"},{"line_number":4802,"context_line":"        if CONF.libvirt.virt_type \u003d\u003d \u0027lxc\u0027 and CONF.libvirt.uid_maps:"},{"line_number":4803,"context_line":"            uid_maps \u003d self._create_idmaps(vconfig.LibvirtConfigGuestUIDMap,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_3a52bd68","line":4800,"updated":"2020-06-16 12:38:03.000000000","message":"Why it is defined as an Union of two types that has a common ancestor? I would use vconfig.LibvirtConfigGuestIDMap directly as a type","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7c7333a19e3e78d5f6b2f11d2fcc0f3007411a0","unresolved":false,"context_lines":[{"line_number":4797,"context_line":"    def _get_guest_idmaps(self):"},{"line_number":4798,"context_line":"        id_maps: ty.List[ty.Union["},{"line_number":4799,"context_line":"            vconfig.LibvirtConfigGuestUIDMap,"},{"line_number":4800,"context_line":"            vconfig.LibvirtConfigGuestGIDMap,"},{"line_number":4801,"context_line":"        ]] \u003d []"},{"line_number":4802,"context_line":"        if CONF.libvirt.virt_type \u003d\u003d \u0027lxc\u0027 and CONF.libvirt.uid_maps:"},{"line_number":4803,"context_line":"            uid_maps \u003d self._create_idmaps(vconfig.LibvirtConfigGuestUIDMap,"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_b244e9b8","line":4800,"in_reply_to":"bf51134e_3a52bd68","updated":"2020-06-19 09:43:58.000000000","message":"Oh, I never even thought of that. Done","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"15873d23ed1f6054caf25bbd7f5298f377bdeb86","unresolved":false,"context_lines":[{"line_number":7350,"context_line":"                set(x) for x in set("},{"line_number":7351,"context_line":"                    tuple(cpu.siblings) or () for cpu in cell.cpus"},{"line_number":7352,"context_line":"                )"},{"line_number":7353,"context_line":"            )"},{"line_number":7354,"context_line":""},{"line_number":7355,"context_line":"            cpus \u0026\u003d available_shared_cpus | available_dedicated_cpus"},{"line_number":7356,"context_line":"            siblings \u003d [sib \u0026 cpus for sib in siblings]"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_7a56f52b","line":7353,"updated":"2020-06-16 12:38:03.000000000","message":"My brain run out of stack processing this. \n\nIs it just me or the above equivalent to\n\nsiblings \u003d sorted(\n    for x in set(set(cpu.siblings) or set() \n                 for cpu in cell.cpus))","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c7c7333a19e3e78d5f6b2f11d2fcc0f3007411a0","unresolved":false,"context_lines":[{"line_number":7350,"context_line":"                set(x) for x in set("},{"line_number":7351,"context_line":"                    tuple(cpu.siblings) or () for cpu in cell.cpus"},{"line_number":7352,"context_line":"                )"},{"line_number":7353,"context_line":"            )"},{"line_number":7354,"context_line":""},{"line_number":7355,"context_line":"            cpus \u0026\u003d available_shared_cpus | available_dedicated_cpus"},{"line_number":7356,"context_line":"            siblings \u003d [sib \u0026 cpus for sib in siblings]"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_32d579ba","line":7353,"in_reply_to":"bf51134e_60f3ca74","updated":"2020-06-19 09:43:58.000000000","message":"Done","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99af125b631b6c0b15db733544d02bd054eddd30","unresolved":false,"context_lines":[{"line_number":7350,"context_line":"                set(x) for x in set("},{"line_number":7351,"context_line":"                    tuple(cpu.siblings) or () for cpu in cell.cpus"},{"line_number":7352,"context_line":"                )"},{"line_number":7353,"context_line":"            )"},{"line_number":7354,"context_line":""},{"line_number":7355,"context_line":"            cpus \u0026\u003d available_shared_cpus | available_dedicated_cpus"},{"line_number":7356,"context_line":"            siblings \u003d [sib \u0026 cpus for sib in siblings]"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_fb1e756e","line":7353,"in_reply_to":"bf51134e_7a56f52b","updated":"2020-06-17 10:56:33.000000000","message":"This took me aaaaages to figure out too :D I was hoping this was easier to understand, but clearly it isn\u0027t. What we\u0027re aiming to get is a list of all unique sets of siblings (note: list is important since it has to be sorted, and set is important since we don\u0027t want duplicates). It\u0027s probably easiest to visualize with some code. Here\u0027s an example using the XML from my own laptop:\n\n  \u003e\u003e\u003e from nova.virt.libvirt import config\n  \u003e\u003e\u003e xml \u003d \"\"\"\n  ... \u003ccell id\u003d\u00270\u0027\u003e\n  ...   \u003cmemory unit\u003d\u0027KiB\u0027\u003e19885704\u003c/memory\u003e\n  ...   \u003cpages unit\u003d\u0027KiB\u0027 size\u003d\u00274\u0027\u003e4971426\u003c/pages\u003e\n  ...   \u003cpages unit\u003d\u0027KiB\u0027 size\u003d\u00272048\u0027\u003e0\u003c/pages\u003e\n  ...   \u003cpages unit\u003d\u0027KiB\u0027 size\u003d\u00271048576\u0027\u003e0\u003c/pages\u003e\n  ...   \u003cdistances\u003e\n  ...     \u003csibling id\u003d\u00270\u0027 value\u003d\u002710\u0027/\u003e\n  ...   \u003c/distances\u003e\n  ...   \u003ccpus num\u003d\u00274\u0027\u003e\n  ...     \u003ccpu id\u003d\u00270\u0027 socket_id\u003d\u00270\u0027 core_id\u003d\u00270\u0027 siblings\u003d\u00270,2\u0027/\u003e\n  ...     \u003ccpu id\u003d\u00271\u0027 socket_id\u003d\u00270\u0027 core_id\u003d\u00271\u0027 siblings\u003d\u00271,3\u0027/\u003e\n  ...     \u003ccpu id\u003d\u00272\u0027 socket_id\u003d\u00270\u0027 core_id\u003d\u00270\u0027 siblings\u003d\u00270,2\u0027/\u003e\n  ...     \u003ccpu id\u003d\u00273\u0027 socket_id\u003d\u00270\u0027 core_id\u003d\u00271\u0027 siblings\u003d\u00271,3\u0027/\u003e\n  ...   \u003c/cpus\u003e\n  ... \u003c/cell\u003e\n  ... \"\"\"\n  \u003e\u003e\u003e cell \u003d config.LibvirtConfigCapsNUMACell()\n  \u003e\u003e\u003e cell.parse_str(xml)\n  \u003e\u003e\u003e sorted(set(x) for x in set(tuple(cpu.siblings) or () for cpu in cell.cpus))\n  [{1, 3}, {0, 2}]\n\nWe have to use tuple(cpu.siblings) and not set since sets aren\u0027t immutable and therefore aren\u0027t hashable.\n\n  \u003e\u003e\u003e sorted(set(x) for x in set(set(cpu.siblings) or () for cpu in cell.cpus))\n  Traceback (most recent call last):\n    File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  TypeError: unhashable type: \u0027set\u0027\n\nPerhaps it would be better if I unrolled this list comprehension and just use bog-standard for loops?","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"745876bce0addec9b84f87825b05c45b44f4d239","unresolved":false,"context_lines":[{"line_number":7350,"context_line":"                set(x) for x in set("},{"line_number":7351,"context_line":"                    tuple(cpu.siblings) or () for cpu in cell.cpus"},{"line_number":7352,"context_line":"                )"},{"line_number":7353,"context_line":"            )"},{"line_number":7354,"context_line":""},{"line_number":7355,"context_line":"            cpus \u0026\u003d available_shared_cpus | available_dedicated_cpus"},{"line_number":7356,"context_line":"            siblings \u003d [sib \u0026 cpus for sib in siblings]"}],"source_content_type":"text/x-python","patch_set":9,"id":"bf51134e_60f3ca74","line":7353,"in_reply_to":"bf51134e_fb1e756e","updated":"2020-06-17 16:32:48.000000000","message":"Thanks got it now (and learned a new word bog-standard).\n\nSo cpu.siblings is a set but we want to deduplicate sibling sets by putting them in a set, but that needs a hashable type, like tuple. But at the end we want to covert back the sibling tuples to sibling sets. Maybe it worth a code comment describing the intention of this code. :)","commit_id":"f95b9cead0f6e4acf8ce8b7e434c794482768864"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"54cd1dd68b2c7c461871ed2bb8de49b3633c270d","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"    def _detach_encrypted_volumes(self, instance, block_device_info):"},{"line_number":1467,"context_line":"        \"\"\"Detaches encrypted volumes attached to instance.\"\"\""},{"line_number":1468,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1469,"context_line":"        for path in ["},{"line_number":1470,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1471,"context_line":"        ]:"},{"line_number":1472,"context_line":"            dmcrypt.delete_volume(path)"},{"line_number":1473,"context_line":""},{"line_number":1474,"context_line":"    def _get_serial_ports_from_guest(self, guest, mode\u003dNone):"},{"line_number":1475,"context_line":"        \"\"\"Returns an iterator over serial port(s) configured on guest."}],"source_content_type":"text/x-python","patch_set":10,"id":"bf51134e_b7004203","line":1472,"range":{"start_line":1469,"start_character":8,"end_line":1472,"end_character":39},"updated":"2020-06-21 10:11:26.000000000","message":"This is two levels loop. cases.\n\nIt can be:\n\nfor d in disks:\n    if dmcrypt.is_encrypted(d[\u0027path\u0027])\n       dmcrypt.delete_volume(path)\n\nbut yes, it is small list in usual","commit_id":"6a71981e47d3c92fa52010df783ef0c79c577139"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f0fcef5894b7734ec451268af25dcc8fb6af284e","unresolved":false,"context_lines":[{"line_number":1466,"context_line":"    def _detach_encrypted_volumes(self, instance, block_device_info):"},{"line_number":1467,"context_line":"        \"\"\"Detaches encrypted volumes attached to instance.\"\"\""},{"line_number":1468,"context_line":"        disks \u003d self._get_instance_disk_info(instance, block_device_info)"},{"line_number":1469,"context_line":"        for path in ["},{"line_number":1470,"context_line":"            d[\u0027path\u0027] for d in disks if dmcrypt.is_encrypted(d[\u0027path\u0027])"},{"line_number":1471,"context_line":"        ]:"},{"line_number":1472,"context_line":"            dmcrypt.delete_volume(path)"},{"line_number":1473,"context_line":""},{"line_number":1474,"context_line":"    def _get_serial_ports_from_guest(self, guest, mode\u003dNone):"},{"line_number":1475,"context_line":"        \"\"\"Returns an iterator over serial port(s) configured on guest."}],"source_content_type":"text/x-python","patch_set":10,"id":"bf51134e_05fe156a","line":1472,"range":{"start_line":1469,"start_character":8,"end_line":1472,"end_character":39},"in_reply_to":"bf51134e_b7004203","updated":"2020-06-22 09:21:58.000000000","message":"Oh yeah, I could have also just done:\n\n  for disk in disks if dmcrypt.is_encrypted(disk[\u0027path\u0027]):\n      dmcrypt.delete_volume(disk[\u0027path\u0027])\n\nCan rework later","commit_id":"6a71981e47d3c92fa52010df783ef0c79c577139"}]}