)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e64494aeadd68f571042a09c3a936e5259e1d2d3","unresolved":true,"context_lines":[{"line_number":9,"context_line":"NOTE(lyarwood): Still WIP as I\u0027d like to get some approval of the"},{"line_number":10,"context_line":"approach in machine_type_utils._get_instances_without_mtype of filtering"},{"line_number":11,"context_line":"in python instead of a DB query. I still have the code for the latter to"},{"line_number":12,"context_line":"hand so I\u0027m fine either way."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change adds a machine_type command to list all instance UUIDs with"},{"line_number":15,"context_line":"hw_machine_type unset in their image metadata. This will be useful to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5fac0eb3_93e4fca3","line":12,"updated":"2021-02-16 13:05:43.000000000","message":"Is there any pro-cons for python based vs DB based filtering? I guess DB is cheaper in execution time and memory. Would be nice to know what happens when this query runs in a big deployment without any cell id filtering.","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6079840fecd8f6ba751d930da5aba2382554750e","unresolved":true,"context_lines":[{"line_number":9,"context_line":"NOTE(lyarwood): Still WIP as I\u0027d like to get some approval of the"},{"line_number":10,"context_line":"approach in machine_type_utils._get_instances_without_mtype of filtering"},{"line_number":11,"context_line":"in python instead of a DB query. I still have the code for the latter to"},{"line_number":12,"context_line":"hand so I\u0027m fine either way."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This change adds a machine_type command to list all instance UUIDs with"},{"line_number":15,"context_line":"hw_machine_type unset in their image metadata. This will be useful to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"5f196d61_7d76e2a5","line":12,"in_reply_to":"5fac0eb3_93e4fca3","updated":"2021-02-24 09:45:52.000000000","message":"If we don\u0027t have information about the performance then go with the less complex solution and let CERN know that it might be an issue. Then if this is an issue we can change the solution later in a bugfix.","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"}],"nova/cmd/manage.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e64494aeadd68f571042a09c3a936e5259e1d2d3","unresolved":true,"context_lines":[{"line_number":2858,"context_line":"        * 0: Command completed successfully."},{"line_number":2859,"context_line":"        * 1: An unexpected error happened."},{"line_number":2860,"context_line":"        * 2: Unable to find cell mapping."},{"line_number":2861,"context_line":"        * 3: No instances found without image_hw_machine_type set."},{"line_number":2862,"context_line":"        \"\"\""},{"line_number":2863,"context_line":"        try:"},{"line_number":2864,"context_line":"            instance_list \u003d machine_type_utils.get_instances_without_type("}],"source_content_type":"text/x-python","patch_set":4,"id":"49717d7d_40860185","line":2861,"updated":"2021-02-16 13:05:43.000000000","message":"wondering if this is the error condition and not case when there are instances without machine_type set","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"90bba2f68d726524a251f6a1687fcc4f5a71c7e0","unresolved":true,"context_lines":[{"line_number":2739,"context_line":"                return 0"},{"line_number":2740,"context_line":"            else:"},{"line_number":2741,"context_line":"                print(\u0027No instances found without hw_machine_type set.\u0027)"},{"line_number":2742,"context_line":"                return 3"},{"line_number":2743,"context_line":"        except exception.CellMappingNotFound as e:"},{"line_number":2744,"context_line":"            print(_(str(e)))"},{"line_number":2745,"context_line":"            return 2"}],"source_content_type":"text/x-python","patch_set":5,"id":"7357f7fd_8da5aace","line":2742,"updated":"2021-02-19 18:01:21.000000000","message":"Is this backwards? The success case for this would be..no instances returned, surely?","commit_id":"38f0f05f1065ed4360fa1ac30495777fbe2979c7"}],"nova/virt/libvirt/machine_type_utils.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"760a3426c6fd351124ac8f7659ce4960770aca27","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        )"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    results \u003d nova_context.scatter_gather_skip_cell0("},{"line_number":224,"context_line":"            context,"},{"line_number":225,"context_line":"            _get_instances_without_mtype"},{"line_number":226,"context_line":"    )"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"113e8c3f_b6c4b08d","line":224,"range":{"start_line":224,"start_character":8,"end_line":224,"end_character":12},"updated":"2021-03-01 17:35:34.000000000","message":"whoops","commit_id":"3f33af93db590051586df77fd0c19c5946d1fb68"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2f9010c45ffdb9a734fca028e4e53c42caa3aa9c","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        )"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    results \u003d nova_context.scatter_gather_skip_cell0("},{"line_number":224,"context_line":"            context,"},{"line_number":225,"context_line":"            _get_instances_without_mtype"},{"line_number":226,"context_line":"    )"},{"line_number":227,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"2af613ce_87349739","line":224,"range":{"start_line":224,"start_character":8,"end_line":224,"end_character":12},"in_reply_to":"113e8c3f_b6c4b08d","updated":"2021-03-02 10:45:21.000000000","message":"Done","commit_id":"3f33af93db590051586df77fd0c19c5946d1fb68"}],"nova/virt/machine_type_utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e64494aeadd68f571042a09c3a936e5259e1d2d3","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"def _get_instances_without_mtype("},{"line_number":184,"context_line":"    context: \u0027nova_context.RequestContext\u0027,"},{"line_number":185,"context_line":") -\u003e ty.List[objects.Instance]:"},{"line_number":186,"context_line":"    \"\"\"Fetch a list of instance UUIDs from the DB without hw_machine_type set"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    :param meta: \u0027sqlalchemy.MetaData\u0027 pointing to a given cell DB"}],"source_content_type":"text/x-python","patch_set":4,"id":"9b4acfb0_1be22329","line":185,"range":{"start_line":185,"start_character":13,"end_line":185,"end_character":29},"updated":"2021-02-16 13:05:43.000000000","message":"@Stephen: how on Earth mypy now capable to resolve this (objects.Instances is dynamically generated) but not in other files?","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3ee865b7eaf9aa80f6b54a769fdb7478e3486f52","unresolved":true,"context_lines":[{"line_number":182,"context_line":""},{"line_number":183,"context_line":"def _get_instances_without_mtype("},{"line_number":184,"context_line":"    context: \u0027nova_context.RequestContext\u0027,"},{"line_number":185,"context_line":") -\u003e ty.List[objects.Instance]:"},{"line_number":186,"context_line":"    \"\"\"Fetch a list of instance UUIDs from the DB without hw_machine_type set"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    :param meta: \u0027sqlalchemy.MetaData\u0027 pointing to a given cell DB"}],"source_content_type":"text/x-python","patch_set":4,"id":"9bb501d7_4fd89d9f","line":185,"range":{"start_line":185,"start_character":13,"end_line":185,"end_character":29},"in_reply_to":"9b4acfb0_1be22329","updated":"2021-02-19 17:57:56.000000000","message":"iirc, normally this isn\u0027t a failure of mypy (it\u0027s still ignoring it for now because \u0027nova.objects\u0027 isn\u0027t typed yet) but rather a failure of unit tests. Python evaluates these things as soon as it loads the code but \u0027nova.objects\u0027 doesn\u0027t have an \u0027Instance\u0027 attribute until \u0027nova.objects.register_all\u0027 is called.\n\nSo yeah, I\u0027ve no idea how this passing 😊","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d6d19dbd4118fc4a8d69ad32c81d396b58690e88","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        if instance.deleted \u003d\u003d 0 and instance.vm_state !\u003d vm_states.BUILDING:"},{"line_number":196,"context_line":"            if instance.image_meta.properties.get(\u0027hw_machine_type\u0027) is None:"},{"line_number":197,"context_line":"                instances_without.append(instance)"},{"line_number":198,"context_line":"    return instances_without"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"def _get_cell_mappings("}],"source_content_type":"text/x-python","patch_set":4,"id":"22ca70a6_354a5240","line":198,"updated":"2021-02-16 10:11:18.000000000","message":"fwiw, I think this seems reasonable though it\u0027s kind of hard to judge without seeing it run at scale. It\u0027s perhaps worth noting, however, that we do have experience with similar problems (populating an instance-related field) in the form of the \u0027nova-manage db null_instance_uuid_scan\u0027 command and it\u0027s underlying implementation of \u0027nova.db.sqlalchemy.migration.db_null_instance_uuid_scan\u0027. I assume you\u0027ve looked at doing something like that here and it didn\u0027t work? If not, I wonder would a similar SQLAlchemy model-based approach work for the above commands and allow us to move all of this to \u0027nova.db(.sqlalchemy).api\u0027?","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"85f0dd9c6e8666b51191244d7645ac575c1a8d99","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        if instance.deleted \u003d\u003d 0 and instance.vm_state !\u003d vm_states.BUILDING:"},{"line_number":196,"context_line":"            if instance.image_meta.properties.get(\u0027hw_machine_type\u0027) is None:"},{"line_number":197,"context_line":"                instances_without.append(instance)"},{"line_number":198,"context_line":"    return instances_without"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"def _get_cell_mappings("}],"source_content_type":"text/x-python","patch_set":4,"id":"29a1fe54_6a7c9e7e","line":198,"in_reply_to":"22ca70a6_354a5240","updated":"2021-02-16 10:51:06.000000000","message":"Yeah I actually started by looking at the _check_ironic_flavor_migration status upgrade check and essentially copying the SQLAlchemy approach but it felt awkward and not worth the ultimate maintenance burden compared to the above. Even if that approach would ultimately out preform this the fact that this is for a single shot command not a long running periodic etc made me think this would be acceptable.\n\nThe code I had for this was in the nova-status change below FWIW:\n\nhttps://review.opendev.org/c/openstack/nova/+/770643/4/nova/cmd/status.py\n\nNote at the time this was going to be libvirt specific, thus the additional check that isn\u0027t present in the object version.","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3ee865b7eaf9aa80f6b54a769fdb7478e3486f52","unresolved":true,"context_lines":[{"line_number":195,"context_line":"        if instance.deleted \u003d\u003d 0 and instance.vm_state !\u003d vm_states.BUILDING:"},{"line_number":196,"context_line":"            if instance.image_meta.properties.get(\u0027hw_machine_type\u0027) is None:"},{"line_number":197,"context_line":"                instances_without.append(instance)"},{"line_number":198,"context_line":"    return instances_without"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"def _get_cell_mappings("}],"source_content_type":"text/x-python","patch_set":4,"id":"58c0d955_2c178667","line":198,"in_reply_to":"29a1fe54_6a7c9e7e","updated":"2021-02-19 17:57:56.000000000","message":"Thanks for the link. All good points and I guess the object version is fine. The only way I see this breaking is if the database were somehow out of sync with the objects, but that\u0027s already a concern for all nova-manage commands that use objects. Let\u0027s go with this and we can revisit if needed.","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e64494aeadd68f571042a09c3a936e5259e1d2d3","unresolved":true,"context_lines":[{"line_number":214,"context_line":"        return objects.CellMappingList("},{"line_number":215,"context_line":"            objects\u003d[objects.CellMapping.get_by_uuid(context, cell_uuid)])"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    # Otherwise fetch all cell mappings, drop cell0 and return a list"},{"line_number":218,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":219,"context_line":"    return objects.CellMappingList("},{"line_number":220,"context_line":"        objects\u003d[m for m in cell_mappings if not m.is_cell0()])"}],"source_content_type":"text/x-python","patch_set":4,"id":"ae142b06_c00913fa","line":217,"updated":"2021-02-16 13:05:43.000000000","message":"yeah that is a good optimization, instances in cell0 will be in ERROR state anyhow","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"c63e846f8e868205978ef69eb233af7ada8bff30","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        return objects.CellMappingList("},{"line_number":215,"context_line":"            objects\u003d[objects.CellMapping.get_by_uuid(context, cell_uuid)])"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    # Otherwise fetch all cell mappings, drop cell0 and return a list"},{"line_number":218,"context_line":"    cell_mappings \u003d objects.CellMappingList.get_all(context)"},{"line_number":219,"context_line":"    return objects.CellMappingList("},{"line_number":220,"context_line":"        objects\u003d[m for m in cell_mappings if not m.is_cell0()])"}],"source_content_type":"text/x-python","patch_set":4,"id":"c6bcf45a_3681619f","line":217,"in_reply_to":"ae142b06_c00913fa","updated":"2021-02-17 09:29:55.000000000","message":"Ack","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e64494aeadd68f571042a09c3a936e5259e1d2d3","unresolved":true,"context_lines":[{"line_number":231,"context_line":"    :returns: A list of Instance objects or an empty list"},{"line_number":232,"context_line":"    \"\"\""},{"line_number":233,"context_line":"    instance_list \u003d []"},{"line_number":234,"context_line":"    for cell_mapping in _get_cell_mappings(context, cell_uuid):"},{"line_number":235,"context_line":"        with nova_context.target_cell(context, cell_mapping) as cctxt:"},{"line_number":236,"context_line":"            instance_list +\u003d _get_instances_without_mtype(cctxt)"},{"line_number":237,"context_line":"    return instance_list"}],"source_content_type":"text/x-python","patch_set":4,"id":"6ebf919d_58e17543","line":234,"range":{"start_line":234,"start_character":4,"end_line":234,"end_character":63},"updated":"2021-02-16 13:05:43.000000000","message":"can we use scatter_gather_all_cells() to make the query parallel in all cells?","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"c63e846f8e868205978ef69eb233af7ada8bff30","unresolved":true,"context_lines":[{"line_number":231,"context_line":"    :returns: A list of Instance objects or an empty list"},{"line_number":232,"context_line":"    \"\"\""},{"line_number":233,"context_line":"    instance_list \u003d []"},{"line_number":234,"context_line":"    for cell_mapping in _get_cell_mappings(context, cell_uuid):"},{"line_number":235,"context_line":"        with nova_context.target_cell(context, cell_mapping) as cctxt:"},{"line_number":236,"context_line":"            instance_list +\u003d _get_instances_without_mtype(cctxt)"},{"line_number":237,"context_line":"    return instance_list"}],"source_content_type":"text/x-python","patch_set":4,"id":"0c40f371_8e703a62","line":234,"range":{"start_line":234,"start_character":4,"end_line":234,"end_character":63},"in_reply_to":"6ebf919d_58e17543","updated":"2021-02-17 09:29:55.000000000","message":"Ah TIL about that, yeah that would be useful, I\u0027ll work that into it now.","commit_id":"061eae4b9428424afe129be7e88d0c9839bf715a"}]}
