)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":2098,"context_line":"    @reverts_task_state"},{"line_number":2099,"context_line":"    @wrap_instance_fault"},{"line_number":2100,"context_line":"    def build_and_run_instance(self, context, instance, image, request_spec,"},{"line_number":2101,"context_line":"                     filter_properties, accel_uuids, admin_password\u003dNone,"},{"line_number":2102,"context_line":"                     injected_files\u003dNone, requested_networks\u003dNone,"},{"line_number":2103,"context_line":"                     security_groups\u003dNone, block_device_mapping\u003dNone,"},{"line_number":2104,"context_line":"                     node\u003dNone, limits\u003dNone, host_list\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e4579305_24df57aa","line":2101,"updated":"2021-03-02 16:33:32.000000000","message":"hm if this is now mandatory parameter on the RPC then we can make it mandatory deeper in the calls stack like in the signature of _do_build_and_run_instance()","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d17df51ab622e35141367487afeb4e203d8290d4","unresolved":false,"context_lines":[{"line_number":2098,"context_line":"    @reverts_task_state"},{"line_number":2099,"context_line":"    @wrap_instance_fault"},{"line_number":2100,"context_line":"    def build_and_run_instance(self, context, instance, image, request_spec,"},{"line_number":2101,"context_line":"                     filter_properties, accel_uuids, admin_password\u003dNone,"},{"line_number":2102,"context_line":"                     injected_files\u003dNone, requested_networks\u003dNone,"},{"line_number":2103,"context_line":"                     security_groups\u003dNone, block_device_mapping\u003dNone,"},{"line_number":2104,"context_line":"                     node\u003dNone, limits\u003dNone, host_list\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"6f54ee3f_f043c881","line":2101,"in_reply_to":"cf91964d_57a8b7b5","updated":"2021-03-03 17:09:39.000000000","message":"Ack","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":true,"context_lines":[{"line_number":2098,"context_line":"    @reverts_task_state"},{"line_number":2099,"context_line":"    @wrap_instance_fault"},{"line_number":2100,"context_line":"    def build_and_run_instance(self, context, instance, image, request_spec,"},{"line_number":2101,"context_line":"                     filter_properties, accel_uuids, admin_password\u003dNone,"},{"line_number":2102,"context_line":"                     injected_files\u003dNone, requested_networks\u003dNone,"},{"line_number":2103,"context_line":"                     security_groups\u003dNone, block_device_mapping\u003dNone,"},{"line_number":2104,"context_line":"                     node\u003dNone, limits\u003dNone, host_list\u003dNone):"}],"source_content_type":"text/x-python","patch_set":9,"id":"cf91964d_57a8b7b5","line":2101,"in_reply_to":"e4579305_24df57aa","updated":"2021-03-02 19:11:48.000000000","message":"sure, but I don\u0027t want to do this by this change. Here we just modify what it\u0027s needed for the 6.0 version.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":5112,"context_line":"        instance.system_metadata[\u0027old_vm_state\u0027] \u003d vm_state"},{"line_number":5113,"context_line":"        instance.save()"},{"line_number":5114,"context_line":""},{"line_number":5115,"context_line":"        if not isinstance(request_spec, objects.RequestSpec):"},{"line_number":5116,"context_line":"            # Prior to compute RPC API 5.1 conductor would pass a legacy dict"},{"line_number":5117,"context_line":"            # version of the request spec to compute and since Stein compute"},{"line_number":5118,"context_line":"            # could be sending that back to conductor on reschedule, so if we"}],"source_content_type":"text/x-python","patch_set":9,"id":"8362d91c_a41851d6","line":5115,"updated":"2021-03-02 16:33:32.000000000","message":"Can we drop this now as the V5 Proxy takes care of it?","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":true,"context_lines":[{"line_number":5112,"context_line":"        instance.system_metadata[\u0027old_vm_state\u0027] \u003d vm_state"},{"line_number":5113,"context_line":"        instance.save()"},{"line_number":5114,"context_line":""},{"line_number":5115,"context_line":"        if not isinstance(request_spec, objects.RequestSpec):"},{"line_number":5116,"context_line":"            # Prior to compute RPC API 5.1 conductor would pass a legacy dict"},{"line_number":5117,"context_line":"            # version of the request spec to compute and since Stein compute"},{"line_number":5118,"context_line":"            # could be sending that back to conductor on reschedule, so if we"}],"source_content_type":"text/x-python","patch_set":9,"id":"92ee8a04_8285ba82","line":5115,"in_reply_to":"8362d91c_a41851d6","updated":"2021-03-02 19:11:48.000000000","message":"That\u0027s an excellent catch. Yes, we should but this will add extra work and review needs while we can do this as a clean follow-up change. Let\u0027s try to focus only on the wiring changes for 6.0 support and then we could cut all the unnecessary later.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d17df51ab622e35141367487afeb4e203d8290d4","unresolved":false,"context_lines":[{"line_number":5112,"context_line":"        instance.system_metadata[\u0027old_vm_state\u0027] \u003d vm_state"},{"line_number":5113,"context_line":"        instance.save()"},{"line_number":5114,"context_line":""},{"line_number":5115,"context_line":"        if not isinstance(request_spec, objects.RequestSpec):"},{"line_number":5116,"context_line":"            # Prior to compute RPC API 5.1 conductor would pass a legacy dict"},{"line_number":5117,"context_line":"            # version of the request spec to compute and since Stein compute"},{"line_number":5118,"context_line":"            # could be sending that back to conductor on reschedule, so if we"}],"source_content_type":"text/x-python","patch_set":9,"id":"f8cf0d86_d71c9189","line":5115,"in_reply_to":"92ee8a04_8285ba82","updated":"2021-03-03 17:09:39.000000000","message":"Ack","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ed670064080ff87e4993e72ca2c67a1a96668449","unresolved":true,"context_lines":[{"line_number":7920,"context_line":"        \"\"\""},{"line_number":7921,"context_line":"        return self.driver.check_instance_shared_storage_remote(ctxt, data)"},{"line_number":7922,"context_line":""},{"line_number":7923,"context_line":"    def _dest_can_numa_live_migrate(self, dest_check_data, migration):"},{"line_number":7924,"context_line":"        # TODO(artom) If we have a libvirt driver we expect it to set"},{"line_number":7925,"context_line":"        # dst_supports_numa_live_migration, but we have to remove it if we"},{"line_number":7926,"context_line":"        # did not get a migration from the conductor, indicating that it"}],"source_content_type":"text/x-python","patch_set":9,"id":"154702f2_d7228396","line":7923,"updated":"2021-03-02 22:40:53.000000000","message":"This can be dropped in 6.0 (as the TODO below says)","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25f03cae392c8ff576f92e18481074354a3be37d","unresolved":true,"context_lines":[{"line_number":7920,"context_line":"        \"\"\""},{"line_number":7921,"context_line":"        return self.driver.check_instance_shared_storage_remote(ctxt, data)"},{"line_number":7922,"context_line":""},{"line_number":7923,"context_line":"    def _dest_can_numa_live_migrate(self, dest_check_data, migration):"},{"line_number":7924,"context_line":"        # TODO(artom) If we have a libvirt driver we expect it to set"},{"line_number":7925,"context_line":"        # dst_supports_numa_live_migration, but we have to remove it if we"},{"line_number":7926,"context_line":"        # did not get a migration from the conductor, indicating that it"}],"source_content_type":"text/x-python","patch_set":9,"id":"e2084ba9_b2e59914","line":7923,"in_reply_to":"154702f2_d7228396","updated":"2021-03-03 09:12:08.000000000","message":"Agreed, on further changes.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ed670064080ff87e4993e72ca2c67a1a96668449","unresolved":true,"context_lines":[{"line_number":7971,"context_line":"                    migrate_data.src_supports_numa_live_migration):"},{"line_number":7972,"context_line":"                migrate_data \u003d self._live_migration_claim("},{"line_number":7973,"context_line":"                    ctxt, instance, migrate_data, migration, limits, allocs)"},{"line_number":7974,"context_line":"            elif \u0027dst_supports_numa_live_migration\u0027 in dest_check_data:"},{"line_number":7975,"context_line":"                LOG.info(\u0027Destination was ready for NUMA live migration, \u0027"},{"line_number":7976,"context_line":"                         \u0027but source is either too old, or is set to an \u0027"},{"line_number":7977,"context_line":"                         \u0027older upgrade level.\u0027, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"cb8fbd12_3e94996d","line":7974,"updated":"2021-03-02 22:40:53.000000000","message":"Mostly ditto for this entire if/elif, except we need to account for drivers that don\u0027t support NUMA live migration (anything except libvirt)","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25f03cae392c8ff576f92e18481074354a3be37d","unresolved":true,"context_lines":[{"line_number":7971,"context_line":"                    migrate_data.src_supports_numa_live_migration):"},{"line_number":7972,"context_line":"                migrate_data \u003d self._live_migration_claim("},{"line_number":7973,"context_line":"                    ctxt, instance, migrate_data, migration, limits, allocs)"},{"line_number":7974,"context_line":"            elif \u0027dst_supports_numa_live_migration\u0027 in dest_check_data:"},{"line_number":7975,"context_line":"                LOG.info(\u0027Destination was ready for NUMA live migration, \u0027"},{"line_number":7976,"context_line":"                         \u0027but source is either too old, or is set to an \u0027"},{"line_number":7977,"context_line":"                         \u0027older upgrade level.\u0027, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"0bd239be_613128c9","line":7974,"in_reply_to":"cb8fbd12_3e94996d","updated":"2021-03-03 09:12:08.000000000","message":"Yup, on a follow-up, as this will require a drastic cleanup in the tests too.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ed670064080ff87e4993e72ca2c67a1a96668449","unresolved":true,"context_lines":[{"line_number":8034,"context_line":"        # If any of these are not met, we need to remove the"},{"line_number":8035,"context_line":"        # src_supports_numa_live_migration flag from source_check_data to avoid"},{"line_number":8036,"context_line":"        # incorrectly initiating a NUMA live migration."},{"line_number":8037,"context_line":"        # All of this can be removed in RPC 6.0/objects 2.0."},{"line_number":8038,"context_line":"        can_numa_live_migrate \u003d ("},{"line_number":8039,"context_line":"            \u0027dst_supports_numa_live_migration\u0027 in dest_check_data and"},{"line_number":8040,"context_line":"            dest_check_data.dst_supports_numa_live_migration and"}],"source_content_type":"text/x-python","patch_set":9,"id":"1fb198a9_501e1a3d","line":8037,"updated":"2021-03-02 22:40:53.000000000","message":"Ditto here","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25f03cae392c8ff576f92e18481074354a3be37d","unresolved":true,"context_lines":[{"line_number":8034,"context_line":"        # If any of these are not met, we need to remove the"},{"line_number":8035,"context_line":"        # src_supports_numa_live_migration flag from source_check_data to avoid"},{"line_number":8036,"context_line":"        # incorrectly initiating a NUMA live migration."},{"line_number":8037,"context_line":"        # All of this can be removed in RPC 6.0/objects 2.0."},{"line_number":8038,"context_line":"        can_numa_live_migrate \u003d ("},{"line_number":8039,"context_line":"            \u0027dst_supports_numa_live_migration\u0027 in dest_check_data and"},{"line_number":8040,"context_line":"            dest_check_data.dst_supports_numa_live_migration and"}],"source_content_type":"text/x-python","patch_set":9,"id":"c9ab4890_a542ce4d","line":8037,"in_reply_to":"1fb198a9_501e1a3d","updated":"2021-03-03 09:12:08.000000000","message":"Yup, follow-up.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":8082,"context_line":""},{"line_number":8083,"context_line":"        :param context: security context"},{"line_number":8084,"context_line":"        :param instance: dict of instance data"},{"line_number":8085,"context_line":"        :param block_migration: if true, prepare for block migration"},{"line_number":8086,"context_line":"        :param disk: disk info of instance"},{"line_number":8087,"context_line":"        :param migrate_data: A dict or LiveMigrateData object holding data"},{"line_number":8088,"context_line":"                             required for live migration without shared"}],"source_content_type":"text/x-python","patch_set":9,"id":"8b448fb9_91620279","line":8085,"updated":"2021-03-02 16:33:32.000000000","message":"drop the doc","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":false,"context_lines":[{"line_number":8082,"context_line":""},{"line_number":8083,"context_line":"        :param context: security context"},{"line_number":8084,"context_line":"        :param instance: dict of instance data"},{"line_number":8085,"context_line":"        :param block_migration: if true, prepare for block migration"},{"line_number":8086,"context_line":"        :param disk: disk info of instance"},{"line_number":8087,"context_line":"        :param migrate_data: A dict or LiveMigrateData object holding data"},{"line_number":8088,"context_line":"                             required for live migration without shared"}],"source_content_type":"text/x-python","patch_set":9,"id":"73fd88c8_68bfee26","line":8085,"in_reply_to":"8b448fb9_91620279","updated":"2021-03-02 19:11:48.000000000","message":"Ack","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ed670064080ff87e4993e72ca2c67a1a96668449","unresolved":true,"context_lines":[{"line_number":9087,"context_line":"        #   RPC 5.3)"},{"line_number":9088,"context_line":"        # - src_supports_numa_live_migration was set by the source driver and"},{"line_number":9089,"context_line":"        #   passed the send-RPC-5.3 check."},{"line_number":9090,"context_line":"        # This check can be removed in RPC 6.0."},{"line_number":9091,"context_line":"        if (\u0027src_supports_numa_live_migration\u0027 in migrate_data and"},{"line_number":9092,"context_line":"                migrate_data.src_supports_numa_live_migration):"},{"line_number":9093,"context_line":"            LOG.debug(\u0027Calling destination to drop move claim.\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"6667ca4c_a56bd2a6","line":9090,"updated":"2021-03-02 22:40:53.000000000","message":"This will need to be reworked to just check whether we performed a NUMA live migration.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25f03cae392c8ff576f92e18481074354a3be37d","unresolved":true,"context_lines":[{"line_number":9087,"context_line":"        #   RPC 5.3)"},{"line_number":9088,"context_line":"        # - src_supports_numa_live_migration was set by the source driver and"},{"line_number":9089,"context_line":"        #   passed the send-RPC-5.3 check."},{"line_number":9090,"context_line":"        # This check can be removed in RPC 6.0."},{"line_number":9091,"context_line":"        if (\u0027src_supports_numa_live_migration\u0027 in migrate_data and"},{"line_number":9092,"context_line":"                migrate_data.src_supports_numa_live_migration):"},{"line_number":9093,"context_line":"            LOG.debug(\u0027Calling destination to drop move claim.\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"162d89ee_bfbe0d35","line":9090,"in_reply_to":"6667ca4c_a56bd2a6","updated":"2021-03-03 09:12:08.000000000","message":"Agreed, on further changes.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":9478,"context_line":"            try:"},{"line_number":9479,"context_line":"                instance.task_state \u003d task_states.SHELVING_OFFLOADING"},{"line_number":9480,"context_line":"                instance.save(expected_task_state\u003d(None,))"},{"line_number":9481,"context_line":"                # FIXME(sbauza): we could pass the right list of accel_uuids"},{"line_number":9482,"context_line":"                # but previously, we weren\u0027t not doing this so it\u0027s not a bug."},{"line_number":9483,"context_line":"                self.shelve_offload_instance(context, instance,"},{"line_number":9484,"context_line":"                                             accel_uuids\u003d[],"}],"source_content_type":"text/x-python","patch_set":9,"id":"024a9f57_84bfda43","line":9481,"updated":"2021-03-02 16:33:32.000000000","message":"I guess this was missed during https://blueprints.launchpad.net/nova/+spec/cyborg-shelve-and-unshelve . I will ping brinzhang about it to take a look. But I agree that it is unrelated to this patch bumping the RPC version","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":false,"context_lines":[{"line_number":9478,"context_line":"            try:"},{"line_number":9479,"context_line":"                instance.task_state \u003d task_states.SHELVING_OFFLOADING"},{"line_number":9480,"context_line":"                instance.save(expected_task_state\u003d(None,))"},{"line_number":9481,"context_line":"                # FIXME(sbauza): we could pass the right list of accel_uuids"},{"line_number":9482,"context_line":"                # but previously, we weren\u0027t not doing this so it\u0027s not a bug."},{"line_number":9483,"context_line":"                self.shelve_offload_instance(context, instance,"},{"line_number":9484,"context_line":"                                             accel_uuids\u003d[],"}],"source_content_type":"text/x-python","patch_set":9,"id":"97c6aecd_3044f1d6","line":9481,"in_reply_to":"024a9f57_84bfda43","updated":"2021-03-02 19:11:48.000000000","message":"Ack","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"71594a46383ce6ae34f1c73035fc16a01ee866c1","unresolved":false,"context_lines":[{"line_number":9478,"context_line":"            try:"},{"line_number":9479,"context_line":"                instance.task_state \u003d task_states.SHELVING_OFFLOADING"},{"line_number":9480,"context_line":"                instance.save(expected_task_state\u003d(None,))"},{"line_number":9481,"context_line":"                # FIXME(sbauza): we could pass the right list of accel_uuids"},{"line_number":9482,"context_line":"                # but previously, we weren\u0027t not doing this so it\u0027s not a bug."},{"line_number":9483,"context_line":"                self.shelve_offload_instance(context, instance,"},{"line_number":9484,"context_line":"                                             accel_uuids\u003d[],"}],"source_content_type":"text/x-python","patch_set":9,"id":"26770f72_25aa33d0","line":9481,"in_reply_to":"97c6aecd_3044f1d6","updated":"2021-03-03 08:07:57.000000000","message":"gibi, bauzas, yes, I think this was missed in the cborg-shelve-and-unshelve changed, do I need to fix it with a new patch?","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":10674,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":10675,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":10676,"context_line":"        if not isinstance(request_spec, objects.RequestSpec):"},{"line_number":10677,"context_line":"            # Prior to compute RPC API 5.1 conductor would pass a legacy dict"},{"line_number":10678,"context_line":"            # version of the request spec to compute and since Stein compute"},{"line_number":10679,"context_line":"            # could be sending that back to conductor on reschedule, so if we"},{"line_number":10680,"context_line":"            # got a dict convert it to an object."}],"source_content_type":"text/x-python","patch_set":11,"id":"1aaee338_ea8b5db4","line":10677,"updated":"2021-03-05 19:33:27.000000000","message":"I think in the past we technically only supported 5.max or 5.min_in_V in the bridge release, even though that\u0027s kindof cheating. However, I guess if people can pin V (and maybe W) to 5.1 then we\u0027d need to either handle that better or do this, so might as well do this.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":true,"context_lines":[{"line_number":10674,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":10675,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":10676,"context_line":"        if not isinstance(request_spec, objects.RequestSpec):"},{"line_number":10677,"context_line":"            # Prior to compute RPC API 5.1 conductor would pass a legacy dict"},{"line_number":10678,"context_line":"            # version of the request spec to compute and since Stein compute"},{"line_number":10679,"context_line":"            # could be sending that back to conductor on reschedule, so if we"},{"line_number":10680,"context_line":"            # got a dict convert it to an object."}],"source_content_type":"text/x-python","patch_set":11,"id":"b3db964f_fd1b0e1c","line":10677,"in_reply_to":"1aaee338_ea8b5db4","updated":"2021-03-12 15:48:06.000000000","message":"That\u0027s a good point, and maybe I should try to only support \u003e\u003d5.12 versions. That being said, given the timeline we have, I\u0027ll first provide a new revision for the other nits and then if we have time, I\u0027ll try to do the above.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":10693,"context_line":"    # 5.2 support for optional request_spec argument"},{"line_number":10694,"context_line":"    def resize_instance(self, context, instance, image,"},{"line_number":10695,"context_line":"                        migration, instance_type, clean_shutdown,"},{"line_number":10696,"context_line":"                        request_spec\u003dNone):"},{"line_number":10697,"context_line":"        self.manager.resize_instance(context, instance, image,"},{"line_number":10698,"context_line":"                                    migration, instance_type, clean_shutdown,"},{"line_number":10699,"context_line":"                                    request_spec)"}],"source_content_type":"text/x-python","patch_set":11,"id":"90642b42_54a1b190","line":10696,"range":{"start_line":10696,"start_character":24,"end_line":10696,"end_character":41},"updated":"2021-03-05 19:33:27.000000000","message":"This makes it optional in the signature, but doesn\u0027t really fix that code underneath may expect it. I guess you\u0027re just assuming that all the code underneath the main call had to tolerate this being None at some point and thus must be able to handle it?\n\nAlso not really sure what we\u0027d do, other than try to look it up in the api db from here.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":true,"context_lines":[{"line_number":10693,"context_line":"    # 5.2 support for optional request_spec argument"},{"line_number":10694,"context_line":"    def resize_instance(self, context, instance, image,"},{"line_number":10695,"context_line":"                        migration, instance_type, clean_shutdown,"},{"line_number":10696,"context_line":"                        request_spec\u003dNone):"},{"line_number":10697,"context_line":"        self.manager.resize_instance(context, instance, image,"},{"line_number":10698,"context_line":"                                    migration, instance_type, clean_shutdown,"},{"line_number":10699,"context_line":"                                    request_spec)"}],"source_content_type":"text/x-python","patch_set":11,"id":"a916c36a_8b4141e7","line":10696,"range":{"start_line":10696,"start_character":24,"end_line":10696,"end_character":41},"in_reply_to":"90642b42_54a1b190","updated":"2021-03-12 15:48:06.000000000","message":"That means that before 5.2, the request_spec argument wasn\u0027t passed there so the called methods were supporting to have a None value for it.\n\nIf we want to support 5.0, then that means we need to continue to support the fact that the client wouldn\u0027t provide this argument, right?","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6010e73005d36071bfa9725a0aff84ee1e78c58","unresolved":true,"context_lines":[{"line_number":10693,"context_line":"    # 5.2 support for optional request_spec argument"},{"line_number":10694,"context_line":"    def resize_instance(self, context, instance, image,"},{"line_number":10695,"context_line":"                        migration, instance_type, clean_shutdown,"},{"line_number":10696,"context_line":"                        request_spec\u003dNone):"},{"line_number":10697,"context_line":"        self.manager.resize_instance(context, instance, image,"},{"line_number":10698,"context_line":"                                    migration, instance_type, clean_shutdown,"},{"line_number":10699,"context_line":"                                    request_spec)"}],"source_content_type":"text/x-python","patch_set":11,"id":"076f60cb_5abb0031","line":10696,"range":{"start_line":10696,"start_character":24,"end_line":10696,"end_character":41},"in_reply_to":"a916c36a_8b4141e7","updated":"2021-03-12 16:01:23.000000000","message":"No, my point was that this is now required, so code underneath could have evolved to assume it is not-None, but you\u0027re passing None to the API above that requires this argument. Meaning 6.0 does not allow this to be optional or None, but the code underneath 6.0 *likely* still tolerates it. It\u0027s just a violation of 6.0 and a bad promise that we support 5.2 when we don\u0027t really.\n\nAnother option would just be to raise an exception here, like TooOldCompute or something and say \"Okay, we support 5.x but not THAT old of 5.x, sorry\". Just in case anyone still has things pinned low and hasn\u0027t noticed because they don\u0027t use any new features. It\u0027s unlikely, but an explicit fail would be more helpful than the alternative.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":10720,"context_line":"                          node, request_spec\u003dNone, accel_uuids\u003dNone):"},{"line_number":10721,"context_line":"        self.manager.unshelve_instance(context, instance, image,"},{"line_number":10722,"context_line":"                                       filter_properties, node, request_spec,"},{"line_number":10723,"context_line":"                                       accel_uuids)"},{"line_number":10724,"context_line":""},{"line_number":10725,"context_line":"    # 5.3 support for optional migration and limits arguments"},{"line_number":10726,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"55168b5d_65d8b03b","line":10723,"range":{"start_line":10723,"start_character":39,"end_line":10723,"end_character":50},"updated":"2021-03-05 19:33:27.000000000","message":"shouldn\u0027t this be \"accel_uuids or []\"? Not strictly required as we assume that the underlying code was handling this, but we can easily convert this one to the new expected value, right?","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":10720,"context_line":"                          node, request_spec\u003dNone, accel_uuids\u003dNone):"},{"line_number":10721,"context_line":"        self.manager.unshelve_instance(context, instance, image,"},{"line_number":10722,"context_line":"                                       filter_properties, node, request_spec,"},{"line_number":10723,"context_line":"                                       accel_uuids)"},{"line_number":10724,"context_line":""},{"line_number":10725,"context_line":"    # 5.3 support for optional migration and limits arguments"},{"line_number":10726,"context_line":"    def check_can_live_migrate_destination(self, ctxt, instance,"}],"source_content_type":"text/x-python","patch_set":11,"id":"bb801d79_3f674d72","line":10723,"range":{"start_line":10723,"start_character":39,"end_line":10723,"end_character":50},"in_reply_to":"55168b5d_65d8b03b","updated":"2021-03-12 15:48:06.000000000","message":"Ack","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c6b153fe5c753856f3ced01b8e77ec2c0093ca9b","unresolved":true,"context_lines":[{"line_number":5150,"context_line":"    @reverts_task_state"},{"line_number":5151,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5152,"context_line":"    @wrap_instance_fault"},{"line_number":5153,"context_line":"    def prep_resize(self, context, image, instance, instance_type,"},{"line_number":5154,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":5155,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":5156,"context_line":"        \"\"\"Initiates the process of moving a running instance to another host."}],"source_content_type":"text/x-python","patch_set":14,"id":"76e07463_9f81952e","line":5153,"range":{"start_line":5153,"start_character":52,"end_line":5153,"end_character":65},"updated":"2021-03-24 12:44:16.000000000","message":"Can we rename this to \u0027flavor\u0027? Instance types are long-dead and now seems like a great time to rename this.","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"303ada7196d10cf2496a36e33b404f6f9ac4c3f1","unresolved":true,"context_lines":[{"line_number":5150,"context_line":"    @reverts_task_state"},{"line_number":5151,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5152,"context_line":"    @wrap_instance_fault"},{"line_number":5153,"context_line":"    def prep_resize(self, context, image, instance, instance_type,"},{"line_number":5154,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":5155,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":5156,"context_line":"        \"\"\"Initiates the process of moving a running instance to another host."}],"source_content_type":"text/x-python","patch_set":14,"id":"b2c7813b_f76656dc","line":5153,"range":{"start_line":5153,"start_character":52,"end_line":5153,"end_character":65},"in_reply_to":"76e07463_9f81952e","updated":"2021-03-24 13:50:39.000000000","message":"\u003e Can we rename this to \u0027flavor\u0027?\n\nI guess we can if we handle it properly in client. We\u0027ve not (TMK) changed the name of any parameters like this in the past, so I don\u0027t think we have any prior art on that.\n\n\u003e Instance types are long-dead and now seems like a great time to rename this.\n\nYou mean this terminology, right? They were always both names for the same concept. I thought we had instance_type sprinkled through the API more than we did, but the only place I found it with a quick search is in the migrations api.\n\nAnyway, now\u0027s the time to do it for sure if we want to go to the trouble.","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"8293ae921f88c122e43056dffa52a61e3806bacc","unresolved":true,"context_lines":[{"line_number":5150,"context_line":"    @reverts_task_state"},{"line_number":5151,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5152,"context_line":"    @wrap_instance_fault"},{"line_number":5153,"context_line":"    def prep_resize(self, context, image, instance, instance_type,"},{"line_number":5154,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":5155,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":5156,"context_line":"        \"\"\"Initiates the process of moving a running instance to another host."}],"source_content_type":"text/x-python","patch_set":14,"id":"487a3fd6_981fa402","line":5153,"range":{"start_line":5153,"start_character":52,"end_line":5153,"end_character":65},"in_reply_to":"8df720ba_452c8d31","updated":"2021-03-24 17:23:06.000000000","message":"\u003e \u003e Instance types are long-dead and now seems like a great time to rename this.\n\u003e You mean this terminology, right? They were always both names for the same concept. I thought we had instance_type sprinkled through the API more than we did, but the only place I found it with a quick search is in the migrations api.\n\nSorry, yeah. I meant the idea of this as a resource vs. flavors.\n\n\u003e Honestly, I\u0027d prefer to have a different change for asking to change the parameters name, as it\u0027s not related to the main modification, which is to support 5.0 while we still want to have a new 6.0 API version.\n\nYeah, I think you got the issue now but for anyone that doesn\u0027t, we can only change version names in a major bump. The patch I\u0027ve proposed to do this is at [1]. That\u0027s the absolute minimum we need to do to fix this and test it and I think it\u0027s a reasonable inclusion. Could we squash that in here?\n\n[1] https://review.opendev.org/c/openstack/nova/+/782804","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"10ededfa849c307798e112b31f48d9032aaeab4c","unresolved":true,"context_lines":[{"line_number":5150,"context_line":"    @reverts_task_state"},{"line_number":5151,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5152,"context_line":"    @wrap_instance_fault"},{"line_number":5153,"context_line":"    def prep_resize(self, context, image, instance, instance_type,"},{"line_number":5154,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":5155,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":5156,"context_line":"        \"\"\"Initiates the process of moving a running instance to another host."}],"source_content_type":"text/x-python","patch_set":14,"id":"8df720ba_452c8d31","line":5153,"range":{"start_line":5153,"start_character":52,"end_line":5153,"end_character":65},"in_reply_to":"a35c547b_d651bd09","updated":"2021-03-24 14:28:45.000000000","message":"Actually, I understand your concern. We can\u0027t change the parameter name after this or we would need to have a 7.0 version. Lemme try to see how to help.","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"cc6d5caefee1e5c7e5498f93ce04207854822663","unresolved":true,"context_lines":[{"line_number":5150,"context_line":"    @reverts_task_state"},{"line_number":5151,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5152,"context_line":"    @wrap_instance_fault"},{"line_number":5153,"context_line":"    def prep_resize(self, context, image, instance, instance_type,"},{"line_number":5154,"context_line":"                    request_spec, filter_properties, node,"},{"line_number":5155,"context_line":"                    clean_shutdown, migration, host_list):"},{"line_number":5156,"context_line":"        \"\"\"Initiates the process of moving a running instance to another host."}],"source_content_type":"text/x-python","patch_set":14,"id":"a35c547b_d651bd09","line":5153,"range":{"start_line":5153,"start_character":52,"end_line":5153,"end_character":65},"in_reply_to":"b2c7813b_f76656dc","updated":"2021-03-24 14:22:06.000000000","message":"Honestly, I\u0027d prefer to have a different change for asking to change the parameters name, as it\u0027s not related to the main modification, which is to support 5.0 while we still want to have a new 6.0 API version.","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c6b153fe5c753856f3ced01b8e77ec2c0093ca9b","unresolved":true,"context_lines":[{"line_number":5490,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"},{"line_number":5491,"context_line":"    @wrap_instance_fault"},{"line_number":5492,"context_line":"    def resize_instance(self, context, instance, image,"},{"line_number":5493,"context_line":"                        migration, instance_type, clean_shutdown,"},{"line_number":5494,"context_line":"                        request_spec):"},{"line_number":5495,"context_line":"        \"\"\"Starts the migration of a running instance to another host."},{"line_number":5496,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1bf45a23_40b30da2","line":5493,"range":{"start_line":5493,"start_character":35,"end_line":5493,"end_character":48},"updated":"2021-03-24 12:44:16.000000000","message":"Ditto","commit_id":"340690fb00dfaf69760ed8429c9a41aa142c27e5"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":950,"context_line":"        if not client.can_send_version(version):"},{"line_number":951,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":952,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":953,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":954,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":955,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":956,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":9,"id":"b044f66a_cfed0846","line":953,"updated":"2021-03-02 16:33:32.000000000","message":"shouldn\u0027t it be\n\n    msg_args[\u0027block_migration\u0027] \u003d block_migration\n\nor it doesn\u0027t matter any more?","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":true,"context_lines":[{"line_number":950,"context_line":"        if not client.can_send_version(version):"},{"line_number":951,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":952,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":953,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":954,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":955,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":956,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":9,"id":"cdd93992_ff2f6237","line":953,"in_reply_to":"b044f66a_cfed0846","updated":"2021-03-02 19:11:48.000000000","message":"Nah, we no longer need it in the compute manager service, it\u0027s just for providing an argument over the wire to respect the 5.0 contract.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d17df51ab622e35141367487afeb4e203d8290d4","unresolved":false,"context_lines":[{"line_number":950,"context_line":"        if not client.can_send_version(version):"},{"line_number":951,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":952,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":953,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":954,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":955,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":956,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":9,"id":"e7c98f63_dada30e0","line":953,"in_reply_to":"cdd93992_ff2f6237","updated":"2021-03-03 17:09:39.000000000","message":"Ack","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":531,"context_line":"                server\u003dhost, version\u003dversion)"},{"line_number":532,"context_line":"        cctxt.cast(ctxt, \u0027add_aggregate_host\u0027,"},{"line_number":533,"context_line":"                   aggregate\u003daggregate, host\u003dhost_param,"},{"line_number":534,"context_line":"                   slave_info\u003dslave_info)"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"    def add_fixed_ip_to_instance(self, ctxt, instance, network_id):"},{"line_number":537,"context_line":"        version \u003d \u00275.0\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"0b5a3f0e_803053e8","side":"PARENT","line":534,"updated":"2021-03-05 19:33:27.000000000","message":"Technically this means we can\u0027t be pinned to 5.0, but we know this won\u0027t be called anyway :)","commit_id":"44611935b85108c500495cf7f967342d4fc57459"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":true,"context_lines":[{"line_number":531,"context_line":"                server\u003dhost, version\u003dversion)"},{"line_number":532,"context_line":"        cctxt.cast(ctxt, \u0027add_aggregate_host\u0027,"},{"line_number":533,"context_line":"                   aggregate\u003daggregate, host\u003dhost_param,"},{"line_number":534,"context_line":"                   slave_info\u003dslave_info)"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"    def add_fixed_ip_to_instance(self, ctxt, instance, network_id):"},{"line_number":537,"context_line":"        version \u003d \u00275.0\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"3d3e5a45_443ddc7f","side":"PARENT","line":534,"in_reply_to":"0b5a3f0e_803053e8","updated":"2021-03-12 15:48:06.000000000","message":"Hum, that\u0027s the client, right? So, even if we pin to 5.0, none of all of the existing services will call this method, right?","commit_id":"44611935b85108c500495cf7f967342d4fc57459"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6010e73005d36071bfa9725a0aff84ee1e78c58","unresolved":true,"context_lines":[{"line_number":531,"context_line":"                server\u003dhost, version\u003dversion)"},{"line_number":532,"context_line":"        cctxt.cast(ctxt, \u0027add_aggregate_host\u0027,"},{"line_number":533,"context_line":"                   aggregate\u003daggregate, host\u003dhost_param,"},{"line_number":534,"context_line":"                   slave_info\u003dslave_info)"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"    def add_fixed_ip_to_instance(self, ctxt, instance, network_id):"},{"line_number":537,"context_line":"        version \u003d \u00275.0\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"4280ed5a_5f8d458c","side":"PARENT","line":534,"in_reply_to":"3d3e5a45_443ddc7f","updated":"2021-03-12 16:01:23.000000000","message":"Yep, but that\u0027s my point: \"we know this won\u0027t be called anyway\".","commit_id":"44611935b85108c500495cf7f967342d4fc57459"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":385,"context_line":"                 unshelve_instance()"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        ... Version 6.0 is functionally equivalent to 5.13, aside from"},{"line_number":388,"context_line":"        removing deprecated parameters. Wallaby sends 6.0 by default,"},{"line_number":389,"context_line":"        can accept 5.x calls from Victoria nodes, and can be pinned to 5.x"},{"line_number":390,"context_line":"        for Victoria compatibility. All new changes should go against 6.x."},{"line_number":391,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"402afc87_f53f6334","line":388,"range":{"start_line":388,"start_character":28,"end_line":388,"end_character":38},"updated":"2021-03-05 19:33:27.000000000","message":"...and methods","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                 unshelve_instance()"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        ... Version 6.0 is functionally equivalent to 5.13, aside from"},{"line_number":388,"context_line":"        removing deprecated parameters. Wallaby sends 6.0 by default,"},{"line_number":389,"context_line":"        can accept 5.x calls from Victoria nodes, and can be pinned to 5.x"},{"line_number":390,"context_line":"        for Victoria compatibility. All new changes should go against 6.x."},{"line_number":391,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"0c53066f_b014130d","line":388,"range":{"start_line":388,"start_character":28,"end_line":388,"end_character":38},"in_reply_to":"402afc87_f53f6334","updated":"2021-03-12 15:48:06.000000000","message":"Ack","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        ... Version 6.0 is functionally equivalent to 5.13, aside from"},{"line_number":388,"context_line":"        removing deprecated parameters. Wallaby sends 6.0 by default,"},{"line_number":389,"context_line":"        can accept 5.x calls from Victoria nodes, and can be pinned to 5.x"},{"line_number":390,"context_line":"        for Victoria compatibility. All new changes should go against 6.x."},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"        * 6.0 - Remove 5.x compatibility"}],"source_content_type":"text/x-python","patch_set":11,"id":"50670653_ca900b62","line":389,"range":{"start_line":389,"start_character":71,"end_line":389,"end_character":74},"updated":"2021-03-05 19:33:27.000000000","message":"Maybe we should specifically say 5.12 - promise no more than we have to?","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":true,"context_lines":[{"line_number":386,"context_line":""},{"line_number":387,"context_line":"        ... Version 6.0 is functionally equivalent to 5.13, aside from"},{"line_number":388,"context_line":"        removing deprecated parameters. Wallaby sends 6.0 by default,"},{"line_number":389,"context_line":"        can accept 5.x calls from Victoria nodes, and can be pinned to 5.x"},{"line_number":390,"context_line":"        for Victoria compatibility. All new changes should go against 6.x."},{"line_number":391,"context_line":""},{"line_number":392,"context_line":"        * 6.0 - Remove 5.x compatibility"}],"source_content_type":"text/x-python","patch_set":11,"id":"4d973846_e0a67ce8","line":389,"range":{"start_line":389,"start_character":71,"end_line":389,"end_character":74},"in_reply_to":"50670653_ca900b62","updated":"2021-03-12 15:48:06.000000000","message":"As said, I\u0027ll try to do it later if I have time.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":602,"context_line":"                          instance\u003dinstance,"},{"line_number":603,"context_line":"                          dest_check_data\u003ddest_check_data)"},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"    def check_instance_shared_storage(self, ctxt, instance, data, host\u003dNone):"},{"line_number":606,"context_line":"        msg_args \u003d {\u0027data\u0027: data}"},{"line_number":607,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"},{"line_number":608,"context_line":"        client \u003d self.router.client(ctxt)"}],"source_content_type":"text/x-python","patch_set":11,"id":"bbe4548d_61de8775","line":605,"range":{"start_line":605,"start_character":50,"end_line":605,"end_character":58},"updated":"2021-03-05 19:33:27.000000000","message":"Okay so you\u0027ll remove this in X?\n\nIf we were to only promise 5.13 client compatibility, we could go ahead and drop this now and just put instance\u003dNone in the call.\n\nAgain, a little cheat-y but we already won\u0027t really support 5.0 anyway.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":602,"context_line":"                          instance\u003dinstance,"},{"line_number":603,"context_line":"                          dest_check_data\u003ddest_check_data)"},{"line_number":604,"context_line":""},{"line_number":605,"context_line":"    def check_instance_shared_storage(self, ctxt, instance, data, host\u003dNone):"},{"line_number":606,"context_line":"        msg_args \u003d {\u0027data\u0027: data}"},{"line_number":607,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"},{"line_number":608,"context_line":"        client \u003d self.router.client(ctxt)"}],"source_content_type":"text/x-python","patch_set":11,"id":"e93136f5_1e3b40fb","line":605,"range":{"start_line":605,"start_character":50,"end_line":605,"end_character":58},"in_reply_to":"bbe4548d_61de8775","updated":"2021-03-12 15:48:06.000000000","message":"Ack","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":931,"context_line":"        if not client.can_send_version(version):"},{"line_number":932,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":933,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":934,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":935,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":936,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":937,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":11,"id":"41596ad8_a2380932","line":934,"range":{"start_line":934,"start_character":12,"end_line":934,"end_character":46},"updated":"2021-03-05 19:33:27.000000000","message":"If you\u0027re going to keep the arg in the signature of the client method here, why not send it? And if you\u0027re doing this \u003dNone thing here, why not just remove these args, send None, and avoid the future debt?","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":931,"context_line":"        if not client.can_send_version(version):"},{"line_number":932,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":933,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":934,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":935,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":936,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":937,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":11,"id":"e97850b5_21f5071c","line":934,"range":{"start_line":934,"start_character":12,"end_line":934,"end_character":46},"in_reply_to":"41596ad8_a2380932","updated":"2021-03-12 15:48:06.000000000","message":"Good call, I\u0027ll make it nullable.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba34ee977d3def466fd3c4817008737e1f605e18","unresolved":true,"context_lines":[{"line_number":933,"context_line":"        if not client.can_send_version(version):"},{"line_number":934,"context_line":"            version \u003d \u00275.0\u0027"},{"line_number":935,"context_line":"            # We just need to honor the argument in the v5.0 RPC API method"},{"line_number":936,"context_line":"            msg_args[\u0027block_migration\u0027] \u003d None"},{"line_number":937,"context_line":"        cctxt \u003d client.prepare(server\u003dhost, version\u003dversion,"},{"line_number":938,"context_line":"                               timeout\u003dCONF.long_rpc_timeout,"},{"line_number":939,"context_line":"                               call_monitor_timeout\u003dCONF.rpc_response_timeout)"}],"source_content_type":"text/x-python","patch_set":13,"id":"8b0d19f3_dbb09112","line":936,"updated":"2021-03-23 14:33:55.000000000","message":"Again, why are you not just passing the block_migration you have from L928? Or, better yet, remove it from the signature to prove that nothing in 6.0 needs it?","commit_id":"f45b0cd02bcdd8cbc556415480810099641d782c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"04a8c91d71abc05c4e34786a55791c3a07ff79d3","unresolved":true,"context_lines":[{"line_number":605,"context_line":"                          instance\u003dinstance,"},{"line_number":606,"context_line":"                          dest_check_data\u003ddest_check_data)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"    def check_instance_shared_storage(self, ctxt, data, instance\u003dNone,"},{"line_number":609,"context_line":"                                      host\u003dNone):"},{"line_number":610,"context_line":"        msg_args \u003d {\u0027data\u0027: data}"},{"line_number":611,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"f98a4bec_7d348dfc","line":608,"range":{"start_line":608,"start_character":50,"end_line":608,"end_character":69},"updated":"2021-03-25 14:53:14.000000000","message":"I think this is breaking things:\n\nhttps://zuul.opendev.org/t/openstack/build/952de21f47e04866a8a4dcc34f557135/log/logs/screen-n-cpu.txt?severity\u003d3#21829\n\nI\u0027m guessing the callers of this weren\u0027t changed?","commit_id":"1780005eedb2785e5a69a683791f5cf4575791bc"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4fb64646b93c7d058ebbbc7c8a01b011791cea18","unresolved":true,"context_lines":[{"line_number":605,"context_line":"                          instance\u003dinstance,"},{"line_number":606,"context_line":"                          dest_check_data\u003ddest_check_data)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"    def check_instance_shared_storage(self, ctxt, data, instance\u003dNone,"},{"line_number":609,"context_line":"                                      host\u003dNone):"},{"line_number":610,"context_line":"        msg_args \u003d {\u0027data\u0027: data}"},{"line_number":611,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"a11867ff_7e8dd823","line":608,"range":{"start_line":608,"start_character":50,"end_line":608,"end_character":69},"in_reply_to":"b7b863e0_12ead26f","updated":"2021-03-25 15:14:58.000000000","message":"So would it be better not to change the signature at all?","commit_id":"1780005eedb2785e5a69a683791f5cf4575791bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"40284312bac9773d2f594b8fa84aa098de384ce6","unresolved":true,"context_lines":[{"line_number":605,"context_line":"                          instance\u003dinstance,"},{"line_number":606,"context_line":"                          dest_check_data\u003ddest_check_data)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"    def check_instance_shared_storage(self, ctxt, data, instance\u003dNone,"},{"line_number":609,"context_line":"                                      host\u003dNone):"},{"line_number":610,"context_line":"        msg_args \u003d {\u0027data\u0027: data}"},{"line_number":611,"context_line":"        version \u003d self._ver(ctxt, \u00275.0\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b7b863e0_12ead26f","line":608,"range":{"start_line":608,"start_character":50,"end_line":608,"end_character":69},"in_reply_to":"f98a4bec_7d348dfc","updated":"2021-03-25 14:56:28.000000000","message":"you\u0027re f*** right.","commit_id":"1780005eedb2785e5a69a683791f5cf4575791bc"}],"nova/objects/service.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    # Version 55: Compute RPC v5.13:"},{"line_number":198,"context_line":"    # Add support for qos interface attach"},{"line_number":199,"context_line":"    {\u0027compute_rpc\u0027: \u00275.13\u0027},"},{"line_number":200,"context_line":"    # Version 56: Compute RPC version 6.0:"},{"line_number":201,"context_line":"    {\u0027compute_rpc\u0027: \u00276.0\u0027},"},{"line_number":202,"context_line":")"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"e256c5ad_cd56ba98","line":200,"range":{"start_line":200,"start_character":30,"end_line":200,"end_character":37},"updated":"2021-03-05 19:33:27.000000000","message":"Everywhere above (that I can see) we used vN.M. Why do you have to be different bauzas? :)","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6010e73005d36071bfa9725a0aff84ee1e78c58","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    # Version 55: Compute RPC v5.13:"},{"line_number":198,"context_line":"    # Add support for qos interface attach"},{"line_number":199,"context_line":"    {\u0027compute_rpc\u0027: \u00275.13\u0027},"},{"line_number":200,"context_line":"    # Version 56: Compute RPC version 6.0:"},{"line_number":201,"context_line":"    {\u0027compute_rpc\u0027: \u00276.0\u0027},"},{"line_number":202,"context_line":")"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"75cea8d6_d6305df6","line":200,"range":{"start_line":200,"start_character":30,"end_line":200,"end_character":37},"in_reply_to":"1761a226_85b3697f","updated":"2021-03-12 16:01:23.000000000","message":"Ah, I didn\u0027t look that high.. well, seems like we\u0027re doing v now :P","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    # Version 55: Compute RPC v5.13:"},{"line_number":198,"context_line":"    # Add support for qos interface attach"},{"line_number":199,"context_line":"    {\u0027compute_rpc\u0027: \u00275.13\u0027},"},{"line_number":200,"context_line":"    # Version 56: Compute RPC version 6.0:"},{"line_number":201,"context_line":"    {\u0027compute_rpc\u0027: \u00276.0\u0027},"},{"line_number":202,"context_line":")"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1761a226_85b3697f","line":200,"range":{"start_line":200,"start_character":30,"end_line":200,"end_character":37},"in_reply_to":"e256c5ad_cd56ba98","updated":"2021-03-12 15:48:06.000000000","message":"Because I dumbly copy/pasted from the 5.0 comment ? ;)\nAnyway, just did s/version /v/","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"}],"nova/tests/functional/libvirt/test_numa_live_migration.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"bf481a16b74dd8f06552a680d217a1dac7718c35","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"1b552039_6ce37ff8","line":495,"updated":"2021-03-02 16:33:32.000000000","message":"Why we need to change this? There is still support for sending 5.3 I guess. Probably in X we can drop the whole legacy codepath and test for it though.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25f03cae392c8ff576f92e18481074354a3be37d","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"b747ad77_f9109e3c","line":495,"in_reply_to":"0a4ca134_fdc819b1","updated":"2021-03-03 09:12:08.000000000","message":"Yup, and cleanup will have to be made. My only concern that removing bits from the code also means large refactoring of unit and func tests as they were pretty tangled with checking the RPC versioning.\n\nFor this exact reason, I\u0027ll stick with providing the only required modifications for bumping the major version and I\u0027ll defer cleanups on follow-ups or this change will grow that fat that nobody will have the strenghts and the guts to review it while we\u0027re so close from the end of the development cycle.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"2d89748957ae11fabbacae4f9fcc9e483edfa805","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"4a25e090_43f269f3","line":495,"in_reply_to":"1b552039_6ce37ff8","updated":"2021-03-02 19:11:48.000000000","message":"the problem here is that since we don\u0027t mock the compute RPC API, by default the supported version cap is 6.0, so the test would fail if we don\u0027t change it.\n\nThere are two ways to solve :\n- modify the check to say that we only support 6.0 now (that\u0027s what I wrote)\n- or, somehow mock the RPC API even if we don\u0027t cap to 5.2 but rather cap it for 5.13 like it was before this change\n\nThe problem here is that this whole testclass is doing a lot of verifications about RPC API support for live migrations. I think we should resolve this by cutting the no longer verifications in a separate patch but I just want to make sure here that we only modify by this change what we need for the RPC API bump and that\u0027s it.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"047254fe81911a4b813ed98fb70b16746d7475c1","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"50d7086c_62fcdcbe","line":495,"in_reply_to":"4a25e090_43f269f3","updated":"2021-03-02 20:35:51.000000000","message":"I think that modifying the checks to use 6.0 makes the thing that is being tested here irrelevant. This was specifically crafted to make sure we could survive the boundary across which the numa stuff was communicated during live migration. In 6.0, everyone speaks the same version and has the same assumptions, so there\u0027s really not as much to poke at here.\n\nIMHO, the thing to do is keep this test as it is until we drop 5.x, but fix it to assume the \"we\u0027re still pinned at 5.x\" environment that this test is intended to validate, and then revisit when we drop 5.x to see if there\u0027s anything here that we need to salvage, or if 6.0 makes it totally duplicative of the other tests we have.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"ed670064080ff87e4993e72ca2c67a1a96668449","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"0a4ca134_fdc819b1","line":495,"in_reply_to":"50d7086c_62fcdcbe","updated":"2021-03-02 22:40:53.000000000","message":"Yeah, this was written to validate the questions around \"I\u0027m on Rocky, I\u0027m upgrading to Train, what happens when I live migrate NUMA instances from a Rocky compute to a Train compute, or from a Train compute to a Rocky compute?\" I think as long as we\u0027re supporting 5.X, they should remain. Once 5.x is gone, a lot of the stuff here can be dropped - I left some pointers in nova/compute/manager.py as well.","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d17df51ab622e35141367487afeb4e203d8290d4","unresolved":true,"context_lines":[{"line_number":492,"context_line":"        self.assertEqual("},{"line_number":493,"context_line":"            not pin_source,"},{"line_number":494,"context_line":"            src_mgr.compute_rpcapi.router.client("},{"line_number":495,"context_line":"                ctxt).can_send_version(\u00276.0\u0027))"},{"line_number":496,"context_line":"        self.assertEqual("},{"line_number":497,"context_line":"            not pin_cond,"},{"line_number":498,"context_line":"            cond_mgr.compute_rpcapi.router.client("}],"source_content_type":"text/x-python","patch_set":9,"id":"4298cf7e_1ddf5dcd","line":495,"in_reply_to":"b747ad77_f9109e3c","updated":"2021-03-03 17:09:39.000000000","message":"Thanks. You change make sense","commit_id":"7fef84e1941efcdbfaf81d0b845d92c1603699c9"}],"nova/tests/unit/compute/test_rpcapi.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":141,"context_line":"                # Copy the existing return value"},{"line_number":142,"context_line":"                expected_kwargs[\u0027_return_value\u0027] \u003d kwargs[\u0027_return_value\u0027]"},{"line_number":143,"context_line":"        else:"},{"line_number":144,"context_line":"            expected_kwargs \u003d kwargs.copy()"},{"line_number":145,"context_line":"        if \u0027host_param\u0027 in expected_kwargs:"},{"line_number":146,"context_line":"            expected_kwargs[\u0027host\u0027] \u003d expected_kwargs.pop(\u0027host_param\u0027)"},{"line_number":147,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"c07a6c49_1b998a2e","line":144,"updated":"2021-03-05 19:33:27.000000000","message":"I\u0027m not sure why this has to change, and the \"cheat\" in the comment makes me concerned. Can you explain more?","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                # Copy the existing return value"},{"line_number":142,"context_line":"                expected_kwargs[\u0027_return_value\u0027] \u003d kwargs[\u0027_return_value\u0027]"},{"line_number":143,"context_line":"        else:"},{"line_number":144,"context_line":"            expected_kwargs \u003d kwargs.copy()"},{"line_number":145,"context_line":"        if \u0027host_param\u0027 in expected_kwargs:"},{"line_number":146,"context_line":"            expected_kwargs[\u0027host\u0027] \u003d expected_kwargs.pop(\u0027host_param\u0027)"},{"line_number":147,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"f4d2a470_97e83703","line":144,"in_reply_to":"c07a6c49_1b998a2e","updated":"2021-03-12 15:48:06.000000000","message":"Ack","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":163,"context_line":"            # This is the \"server\" kwarg to the prepare() method so remove it"},{"line_number":164,"context_line":"            # from both kwargs that go to the actual RPC method call."},{"line_number":165,"context_line":"            # NOTE(sbauza): Since we don\u0027t copy expected_kwags from kwargs,"},{"line_number":166,"context_line":"            # this key could be missing but this is fine."},{"line_number":167,"context_line":"            expected_kwargs.pop(\u0027prepare_server\u0027, None)"},{"line_number":168,"context_line":"            host \u003d kwargs.pop(\u0027prepare_server\u0027)"},{"line_number":169,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"383268ec_c1bd0138","line":166,"updated":"2021-03-05 19:33:27.000000000","message":"But we do on L144, so this is sometimes not accurate. Still not sure about the change above, which obviously has an impact here.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            # This is the \"server\" kwarg to the prepare() method so remove it"},{"line_number":164,"context_line":"            # from both kwargs that go to the actual RPC method call."},{"line_number":165,"context_line":"            # NOTE(sbauza): Since we don\u0027t copy expected_kwags from kwargs,"},{"line_number":166,"context_line":"            # this key could be missing but this is fine."},{"line_number":167,"context_line":"            expected_kwargs.pop(\u0027prepare_server\u0027, None)"},{"line_number":168,"context_line":"            host \u003d kwargs.pop(\u0027prepare_server\u0027)"},{"line_number":169,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5329c61b_ee163982","line":166,"in_reply_to":"383268ec_c1bd0138","updated":"2021-03-12 15:48:06.000000000","message":"Ack","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bdf782356202e144bd7c8c68e843a1d9b6fb4364","unresolved":true,"context_lines":[{"line_number":239,"context_line":"            server\u003dself.fake_instance_obj.host, version\u003d\u00275.0\u0027)"},{"line_number":240,"context_line":"        mock_cctx.call.assert_called_with("},{"line_number":241,"context_line":"            ctxt, \u0027check_instance_shared_storage\u0027,"},{"line_number":242,"context_line":"            instance\u003dself.fake_instance_obj, data\u003d\u0027foo\u0027)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def test_confirm_resize_cast(self):"},{"line_number":245,"context_line":"        self._test_compute_api(\u0027confirm_resize\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"d266279f_007a0ecf","line":242,"updated":"2021-03-05 19:33:27.000000000","message":"How come this was implemented differently than all the other tests here? Maybe I\u0027m missing some detail, but a comment might help explain what was hard to test with the helper.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a2baab13b2a500a382a88d3cebe7bbc7df5af76","unresolved":true,"context_lines":[{"line_number":239,"context_line":"            server\u003dself.fake_instance_obj.host, version\u003d\u00275.0\u0027)"},{"line_number":240,"context_line":"        mock_cctx.call.assert_called_with("},{"line_number":241,"context_line":"            ctxt, \u0027check_instance_shared_storage\u0027,"},{"line_number":242,"context_line":"            instance\u003dself.fake_instance_obj, data\u003d\u0027foo\u0027)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def test_confirm_resize_cast(self):"},{"line_number":245,"context_line":"        self._test_compute_api(\u0027confirm_resize\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"e176976a_24393ba4","line":242,"in_reply_to":"d266279f_007a0ecf","updated":"2021-03-12 15:48:06.000000000","message":"For the other tests, we use a large helper method named \"_test_compute_api()\u0027. That said, this large helper method doesn\u0027t support to verify we have more than one call for can_send_version() hence this new different test.\n\nChanging _test_compute_api() just for this could be able, but I think this would be more difficult to review.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e6010e73005d36071bfa9725a0aff84ee1e78c58","unresolved":true,"context_lines":[{"line_number":239,"context_line":"            server\u003dself.fake_instance_obj.host, version\u003d\u00275.0\u0027)"},{"line_number":240,"context_line":"        mock_cctx.call.assert_called_with("},{"line_number":241,"context_line":"            ctxt, \u0027check_instance_shared_storage\u0027,"},{"line_number":242,"context_line":"            instance\u003dself.fake_instance_obj, data\u003d\u0027foo\u0027)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    def test_confirm_resize_cast(self):"},{"line_number":245,"context_line":"        self._test_compute_api(\u0027confirm_resize\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1d1b07b2_d121f154","line":242,"in_reply_to":"e176976a_24393ba4","updated":"2021-03-12 16:01:23.000000000","message":"Okay so the answer is \"because the existing helper doesn\u0027t check that we made two calls to can_send_version()\"? Couldn\u0027t we just call that method and then check it ourselves after? Anyway, it\u0027s for the old version and will go away soon, it\u0027s just.. weird.","commit_id":"f8bada4e16f50d7937c4c1dbcd358fabe892fa6a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba34ee977d3def466fd3c4817008737e1f605e18","unresolved":true,"context_lines":[{"line_number":169,"context_line":"            # NOTE(sbauza): If we copy expected args from the provided kwargs,"},{"line_number":170,"context_line":"            # sometimes we don\u0027t have a prepare_server argument in them hence"},{"line_number":171,"context_line":"            # the default None value."},{"line_number":172,"context_line":"            expected_kwargs.pop(\u0027prepare_server\u0027, None)"},{"line_number":173,"context_line":"            host \u003d kwargs.pop(\u0027prepare_server\u0027)"},{"line_number":174,"context_line":"        else:"},{"line_number":175,"context_line":"            host \u003d kwargs[\u0027instance\u0027][\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"0e3576e1_bde036ff","line":172,"updated":"2021-03-23 14:33:55.000000000","message":"I still don\u0027t understand why these changes (here and above) are needed and I don\u0027t think there\u0027s additional explanation like I asked (in comments or gerrit) to help. But I don\u0027t have time to dig in further, so hopefully other reviewers have sussed out those details.","commit_id":"f45b0cd02bcdd8cbc556415480810099641d782c"}]}