)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b09559e905b6ae54b809e3115215e92f298dd50e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ignore deleted instances when populating with availability zones"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There is no need to add missing AZ to the deleted instances."},{"line_number":10,"context_line":"And if the AZ of such instance cannot be found, populate_missing_availability_zones"},{"line_number":11,"context_line":"will work in endless cycle during online_migrations."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_c828cd98","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":60},"updated":"2018-08-21 08:14:09.000000000","message":"maybe True, the only reason I included all old instances (living and deleted was to keep it consistent)","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b09559e905b6ae54b809e3115215e92f298dd50e","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ignore deleted instances when populating with availability zones"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There is no need to add missing AZ to the deleted instances."},{"line_number":10,"context_line":"And if the AZ of such instance cannot be found, populate_missing_availability_zones"},{"line_number":11,"context_line":"will work in endless cycle during online_migrations."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_4816fd57","line":10,"range":{"start_line":10,"start_character":4,"end_line":10,"end_character":46},"updated":"2018-08-21 08:14:09.000000000","message":"This could very well be true for non-deleted instances stuck in scheduling; which is why filtering based on just deleted\u003d0 might not fix this.","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"27a3a46c2e37b6c3304b891af74d5eb5141ea525","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ignore deleted instances when populating with availability zones"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There is no need to add missing AZ to the deleted instances."},{"line_number":10,"context_line":"And if the AZ of such instance cannot be found, populate_missing_availability_zones"},{"line_number":11,"context_line":"will work in endless cycle during online_migrations."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_0350e65d","line":10,"range":{"start_line":10,"start_character":4,"end_line":10,"end_character":46},"in_reply_to":"3f79a3b5_235962fd","updated":"2018-08-21 08:43:18.000000000","message":"\u003e That\u0027s true.\n\ncool, so it was because instance.host was NULL ?\n\n \u003e Question is, if it should be fixed somehow in \n \u003e populate_missing_availability_zones\n\nyes here.\n\n \u003e or elsewhere","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"9ebbb48ebe58b872ea27d9b16c4ac775ff07deb5","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Ignore deleted instances when populating with availability zones"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"There is no need to add missing AZ to the deleted instances."},{"line_number":10,"context_line":"And if the AZ of such instance cannot be found, populate_missing_availability_zones"},{"line_number":11,"context_line":"will work in endless cycle during online_migrations."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_235962fd","line":10,"range":{"start_line":10,"start_character":4,"end_line":10,"end_character":46},"in_reply_to":"3f79a3b5_4816fd57","updated":"2018-08-21 08:26:29.000000000","message":"That\u0027s true.\nQuestion is, if it should be fixed somehow in populate_missing_availability_zones or elsewhere","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"29aaa59d356eb915a09ef23b3e6859efafe0c646","unresolved":false,"context_lines":[{"line_number":10,"context_line":"when its creation failed before it was scheduled."},{"line_number":11,"context_line":"During online_migration, populate_missing_availability_zones tries to"},{"line_number":12,"context_line":"add missing AZs to all instances. However for instances without a host"},{"line_number":13,"context_line":"there\u0027s no reasonable value for AZ (we can\u0027t use a logic that bases"},{"line_number":14,"context_line":"the value on a host) so let\u0027s skip this kind of instances completely."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_6e3dfedb","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":35},"updated":"2018-08-21 12:08:14.000000000","message":"should be None.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"7d2f7384251dd31d64628fa4696a27ca36c4a756","unresolved":false,"context_lines":[{"line_number":10,"context_line":"when its creation failed before it was scheduled."},{"line_number":11,"context_line":"During online_migration, populate_missing_availability_zones tries to"},{"line_number":12,"context_line":"add missing AZs to all instances. However for instances without a host"},{"line_number":13,"context_line":"there\u0027s no reasonable value for AZ (we can\u0027t use a logic that bases"},{"line_number":14,"context_line":"the value on a host) so let\u0027s skip this kind of instances completely."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_8905f47c","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":35},"in_reply_to":"3f79a3b5_0921e44d","updated":"2018-08-21 12:21:15.000000000","message":"\u003e What should be None? \"However, for instances without a host, the\n \u003e instance.availability_zone should be None.\" Is that what you meant?\n\nyea, should remain None. But I was just thinking out loud, doesn\u0027t have to be updated.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"04da687e4e3085ac1a796616fc682cd734b3dd8e","unresolved":false,"context_lines":[{"line_number":10,"context_line":"when its creation failed before it was scheduled."},{"line_number":11,"context_line":"During online_migration, populate_missing_availability_zones tries to"},{"line_number":12,"context_line":"add missing AZs to all instances. However for instances without a host"},{"line_number":13,"context_line":"there\u0027s no reasonable value for AZ (we can\u0027t use a logic that bases"},{"line_number":14,"context_line":"the value on a host) so let\u0027s skip this kind of instances completely."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_a9d5501d","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":35},"in_reply_to":"3f79a3b5_6e3dfedb","updated":"2018-08-21 12:18:10.000000000","message":"Sorry? You mean \"AZ should be None\" ?\n\nI think no reasonable value also makes sense - if we would know what to set, we would set it. But because we don\u0027t, we skip the instance completely.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3e42e68a8572ceae1a8d47bddb6a601a34633102","unresolved":false,"context_lines":[{"line_number":10,"context_line":"when its creation failed before it was scheduled."},{"line_number":11,"context_line":"During online_migration, populate_missing_availability_zones tries to"},{"line_number":12,"context_line":"add missing AZs to all instances. However for instances without a host"},{"line_number":13,"context_line":"there\u0027s no reasonable value for AZ (we can\u0027t use a logic that bases"},{"line_number":14,"context_line":"the value on a host) so let\u0027s skip this kind of instances completely."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: Ic6060beaa08af5ea70e5e54fffb94eea58aa7bbf"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_0921e44d","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":35},"in_reply_to":"3f79a3b5_6e3dfedb","updated":"2018-08-21 12:17:32.000000000","message":"What should be None? \"However, for instances without a host, the instance.availability_zone should be None.\" Is that what you meant?","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"}],"nova/objects/instance.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"b09559e905b6ae54b809e3115215e92f298dd50e","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"@db_api.pick_context_manager_writer"},{"line_number":1280,"context_line":"def populate_missing_availability_zones(context, count):"},{"line_number":1281,"context_line":"    instances \u003d (context.session.query(models.Instance)."},{"line_number":1282,"context_line":"        filter_by(deleted\u003d0)."},{"line_number":1283,"context_line":"        filter_by(availability_zone\u003dNone).limit(count).all())"},{"line_number":1284,"context_line":"    count_all \u003d len(instances)"},{"line_number":1285,"context_line":"    count_hit \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_a858f155","line":1282,"updated":"2018-08-21 08:14:09.000000000","message":"Question: https://bugs.launchpad.net/nova/+bug/1788115/comments/2 . If that\u0027s the case, then it might not just be for the deleted instances. It could also be that some instances get stuck in scheduling and might see the problem (https://bugs.launchpad.net/nova/+bug/1784074), and the best approach might be to just set them to CONF.default_availability_zone maybe in those situations.","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"27a3a46c2e37b6c3304b891af74d5eb5141ea525","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"@db_api.pick_context_manager_writer"},{"line_number":1280,"context_line":"def populate_missing_availability_zones(context, count):"},{"line_number":1281,"context_line":"    instances \u003d (context.session.query(models.Instance)."},{"line_number":1282,"context_line":"        filter_by(deleted\u003d0)."},{"line_number":1283,"context_line":"        filter_by(availability_zone\u003dNone).limit(count).all())"},{"line_number":1284,"context_line":"    count_all \u003d len(instances)"},{"line_number":1285,"context_line":"    count_hit \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c370ae0b","line":1282,"in_reply_to":"3f79a3b5_43b81ee4","updated":"2018-08-21 08:43:18.000000000","message":"\u003e You mean we should set az to CONF.default_availability_zone right\n \u003e here in populate_missing_availability_zones?\n \u003e \n \u003e az \u003d avail_zone.get_instance_availability_zone(context, instance)\n \u003e if not az:\n \u003e az \u003d CONF.default_availability_zone\n\nyes, my thoughts were to do this ^^ adding sufficient in-line comment to avoid confusions.\n\n \u003e \n \u003e That might actually be better than adapting get_instance_availability_zone\n \u003e which is called from other places\n\nwe should not be touching the get_instance_availability_zone IMHO since the logic there is correct and consistent with the current method of dynamic avz calculation during boot time.","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"9ebbb48ebe58b872ea27d9b16c4ac775ff07deb5","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"@db_api.pick_context_manager_writer"},{"line_number":1280,"context_line":"def populate_missing_availability_zones(context, count):"},{"line_number":1281,"context_line":"    instances \u003d (context.session.query(models.Instance)."},{"line_number":1282,"context_line":"        filter_by(deleted\u003d0)."},{"line_number":1283,"context_line":"        filter_by(availability_zone\u003dNone).limit(count).all())"},{"line_number":1284,"context_line":"    count_all \u003d len(instances)"},{"line_number":1285,"context_line":"    count_hit \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_43b81ee4","line":1282,"in_reply_to":"3f79a3b5_a858f155","updated":"2018-08-21 08:26:29.000000000","message":"You mean we should set az to CONF.default_availability_zone right here in populate_missing_availability_zones?\n\naz \u003d avail_zone.get_instance_availability_zone(context, instance)\nif not az:\n  az \u003d CONF.default_availability_zone\n\nThat might actually be better than adapting get_instance_availability_zone which is called from other places","commit_id":"389d6df2a83a98f90fc0b347209d11bbc5ad77a1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1d7e7aacd7edc2f31432ee14c9e5ad0d1e47ecb3","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            # AZ could be empty for deleted instances that did not have host id"},{"line_number":1289,"context_line":"            # set. Set the default value so that the method does not go into"},{"line_number":1290,"context_line":"            # endless cycle."},{"line_number":1291,"context_line":"            az \u003d CONF.default_availability_zone"},{"line_number":1292,"context_line":"        instance.availability_zone \u003d az"},{"line_number":1293,"context_line":"        instance.save(context.session)"},{"line_number":1294,"context_line":"        count_hit +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_185ff041","line":1291,"range":{"start_line":1291,"start_character":22,"end_line":1291,"end_character":47},"updated":"2018-08-21 10:31:14.000000000","message":"Hmm, shouldn\u0027t this be default_schedule_zone?\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.default_schedule_zone\n\nBut then you still get into the loop where availability_zone\u003dNone which is your bug.\n\nWhy not just filter out instances that don\u0027t have a host in the query above?","commit_id":"31c08dd5663c1d684dc4351ea29bedf2568e97ea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8fbc648b09c929c6609e913bda21bbf524358908","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            # AZ could be empty for deleted instances that did not have host id"},{"line_number":1289,"context_line":"            # set. Set the default value so that the method does not go into"},{"line_number":1290,"context_line":"            # endless cycle."},{"line_number":1291,"context_line":"            az \u003d CONF.default_availability_zone"},{"line_number":1292,"context_line":"        instance.availability_zone \u003d az"},{"line_number":1293,"context_line":"        instance.save(context.session)"},{"line_number":1294,"context_line":"        count_hit +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_d8065821","line":1291,"range":{"start_line":1291,"start_character":22,"end_line":1291,"end_character":47},"in_reply_to":"3f79a3b5_185ff041","updated":"2018-08-21 10:33:23.000000000","message":"The only question I have is does that mean these instances, without a host, would cause problems for the metadata API (the original reason this code was added in the first place)?","commit_id":"31c08dd5663c1d684dc4351ea29bedf2568e97ea"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"2058e36c6d6633406241cbba064aca9357edd377","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            # AZ could be empty for deleted instances that did not have host id"},{"line_number":1289,"context_line":"            # set. Set the default value so that the method does not go into"},{"line_number":1290,"context_line":"            # endless cycle."},{"line_number":1291,"context_line":"            az \u003d CONF.default_availability_zone"},{"line_number":1292,"context_line":"        instance.availability_zone \u003d az"},{"line_number":1293,"context_line":"        instance.save(context.session)"},{"line_number":1294,"context_line":"        count_hit +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_cb1a0416","line":1291,"range":{"start_line":1291,"start_character":22,"end_line":1291,"end_character":47},"in_reply_to":"3f79a3b5_2b64d8e4","updated":"2018-08-21 10:57:11.000000000","message":"I agree with Matt, if the instance doesn\u0027t have a host at all I guess it should be CONF.default_schedule_zone and not the CONF.default_availability_zone, since that\u0027s for cases where the host in not in any defined avz, my bad.\n\nSince CONF.default_schedule_zone is None and doesn\u0027t fix our bug, +1 to filtering out those records where instance.host is None. This keeps the logic for the original metadata API response intact as the zone should be None for instances which do not have a host.","commit_id":"31c08dd5663c1d684dc4351ea29bedf2568e97ea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"555182c0e173211e8e075aecb790cc84f468d7c0","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            # AZ could be empty for deleted instances that did not have host id"},{"line_number":1289,"context_line":"            # set. Set the default value so that the method does not go into"},{"line_number":1290,"context_line":"            # endless cycle."},{"line_number":1291,"context_line":"            az \u003d CONF.default_availability_zone"},{"line_number":1292,"context_line":"        instance.availability_zone \u003d az"},{"line_number":1293,"context_line":"        instance.save(context.session)"},{"line_number":1294,"context_line":"        count_hit +\u003d 1"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_2b64d8e4","line":1291,"range":{"start_line":1291,"start_character":22,"end_line":1291,"end_character":47},"in_reply_to":"3f79a3b5_d8065821","updated":"2018-08-21 10:40:50.000000000","message":"So looking back, this online data migration was added for this change:\n\nhttps://review.openstack.org/#/c/439754/29/nova/api/metadata/base.py@132\n\nSo in the case that the instance.availability_zone is set to None and the instance doesn\u0027t have a host, self.availability_zone \u003d None is the correct value.\n\nTo compare, the compute REST API will return \u0027\u0027 if the instance doesn\u0027t have a host or az set:\n\nhttps://github.com/openstack/nova/blob/722d5b477219f0a2435a9f4ad4d54c61b83219f1/nova/api/openstack/compute/views/servers.py#L170","commit_id":"31c08dd5663c1d684dc4351ea29bedf2568e97ea"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"29aaa59d356eb915a09ef23b3e6859efafe0c646","unresolved":false,"context_lines":[{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"@db_api.pick_context_manager_writer"},{"line_number":1280,"context_line":"def populate_missing_availability_zones(context, count):"},{"line_number":1281,"context_line":"    # instances without host have no reasonable AZ to set"},{"line_number":1282,"context_line":"    not_empty_host \u003d models.Instance.host !\u003d None"},{"line_number":1283,"context_line":"    instances \u003d (context.session.query(models.Instance)."},{"line_number":1284,"context_line":"        filter(not_empty_host)."}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_eea6eeab","line":1281,"range":{"start_line":1281,"start_character":29,"end_line":1281,"end_character":57},"updated":"2018-08-21 12:08:14.000000000","message":"nit: instances without host need not be populated as they should have their availability_zone as None.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"}],"nova/tests/functional/db/test_instance.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b5bbb2a75939c1401d51edf27c510c9724c6000","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.assertEqual(1, count_all)"},{"line_number":90,"context_line":"        self.assertEqual(1, count_hit)"},{"line_number":91,"context_line":"        inst1 \u003d objects.Instance.get_by_uuid(self.context, uuid1)"},{"line_number":92,"context_line":"        # since instance.host was None, avz is set to"},{"line_number":93,"context_line":"        # CONF.default_availability_zone i.e \u0027nova\u0027 which is the default zone"},{"line_number":94,"context_line":"        # for compute services."},{"line_number":95,"context_line":"        self.assertEqual(\u0027nova\u0027, inst1.availability_zone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_6ebade6e","side":"PARENT","line":92,"range":{"start_line":92,"start_character":10,"end_line":92,"end_character":38},"updated":"2018-08-21 12:03:48.000000000","message":"Yeah I\u0027m not sure why this was ever in here, must have been a typo, because it\u0027s clear the 2 fake instances created for this test both have hosts set.","commit_id":"a7614291033b40464230cb4f27515867e90a9ce1"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"29aaa59d356eb915a09ef23b3e6859efafe0c646","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        self.assertEqual(1, count_all)"},{"line_number":90,"context_line":"        self.assertEqual(1, count_hit)"},{"line_number":91,"context_line":"        inst1 \u003d objects.Instance.get_by_uuid(self.context, uuid1)"},{"line_number":92,"context_line":"        # since instance.host was None, avz is set to"},{"line_number":93,"context_line":"        # CONF.default_availability_zone i.e \u0027nova\u0027 which is the default zone"},{"line_number":94,"context_line":"        # for compute services."},{"line_number":95,"context_line":"        self.assertEqual(\u0027nova\u0027, inst1.availability_zone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_2e08662b","side":"PARENT","line":92,"range":{"start_line":92,"start_character":10,"end_line":92,"end_character":38},"in_reply_to":"3f79a3b5_6ebade6e","updated":"2018-08-21 12:08:14.000000000","message":"true, I think it was supposed to be instance.host.avz is None... we put it to default.","commit_id":"a7614291033b40464230cb4f27515867e90a9ce1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4b5bbb2a75939c1401d51edf27c510c9724c6000","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        inst2 \u003d self._create_instance(availability_zone\u003d\"fake\","},{"line_number":83,"context_line":"                                      host\u003d\"fake-host2\")"},{"line_number":84,"context_line":"        # ... and one without a host (simulating failed spawn)"},{"line_number":85,"context_line":"        inst4 \u003d self._create_instance(host\u003dNone)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        self.assertIsNone(inst1.availability_zone)"},{"line_number":88,"context_line":"        self.assertEqual(\"fake\", inst2.availability_zone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_6ea17e46","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":13},"updated":"2018-08-21 12:03:48.000000000","message":"inst3","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"04da687e4e3085ac1a796616fc682cd734b3dd8e","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        inst2 \u003d self._create_instance(availability_zone\u003d\"fake\","},{"line_number":83,"context_line":"                                      host\u003d\"fake-host2\")"},{"line_number":84,"context_line":"        # ... and one without a host (simulating failed spawn)"},{"line_number":85,"context_line":"        inst4 \u003d self._create_instance(host\u003dNone)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        self.assertIsNone(inst1.availability_zone)"},{"line_number":88,"context_line":"        self.assertEqual(\"fake\", inst2.availability_zone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_09cfa47f","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":13},"in_reply_to":"3f79a3b5_4e6cc207","updated":"2018-08-21 12:18:10.000000000","message":"OK","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c1a5e0b426fe2f422f85f5d34b67dbf07b0bf820","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        inst2 \u003d self._create_instance(availability_zone\u003d\"fake\","},{"line_number":83,"context_line":"                                      host\u003d\"fake-host2\")"},{"line_number":84,"context_line":"        # ... and one without a host (simulating failed spawn)"},{"line_number":85,"context_line":"        inst4 \u003d self._create_instance(host\u003dNone)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        self.assertIsNone(inst1.availability_zone)"},{"line_number":88,"context_line":"        self.assertEqual(\"fake\", inst2.availability_zone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_4e6cc207","line":85,"range":{"start_line":85,"start_character":8,"end_line":85,"end_character":13},"in_reply_to":"3f79a3b5_6ea17e46","updated":"2018-08-21 12:04:36.000000000","message":"Oh I see inst3 exists below. We don\u0027t actually use this variable so just remove it (create the instance with no host, but don\u0027t store it in a local inst4 variable).","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"29aaa59d356eb915a09ef23b3e6859efafe0c646","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # we get only the instance whose avz was None and where host is set"},{"line_number":92,"context_line":"        self.assertEqual(1, count_all)"},{"line_number":93,"context_line":"        self.assertEqual(1, count_hit)"},{"line_number":94,"context_line":"        # avz is set by get_host_availability_zone to"},{"line_number":95,"context_line":"        # CONF.default_availability_zone i.e \u0027nova\u0027 which is the default zone"},{"line_number":96,"context_line":"        # for compute services."},{"line_number":97,"context_line":"        inst1 \u003d objects.Instance.get_by_uuid(self.context, uuid1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_ae65561f","line":94,"range":{"start_line":94,"start_character":10,"end_line":94,"end_character":50},"updated":"2018-08-21 12:08:14.000000000","message":"nit: Since instance.host has no avz, avz is set to CONF.default_availability_zone...","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3e42e68a8572ceae1a8d47bddb6a601a34633102","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # we get only the instance whose avz was None and where host is set"},{"line_number":92,"context_line":"        self.assertEqual(1, count_all)"},{"line_number":93,"context_line":"        self.assertEqual(1, count_hit)"},{"line_number":94,"context_line":"        # avz is set by get_host_availability_zone to"},{"line_number":95,"context_line":"        # CONF.default_availability_zone i.e \u0027nova\u0027 which is the default zone"},{"line_number":96,"context_line":"        # for compute services."},{"line_number":97,"context_line":"        inst1 \u003d objects.Instance.get_by_uuid(self.context, uuid1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_69b898e4","line":94,"range":{"start_line":94,"start_character":10,"end_line":94,"end_character":50},"in_reply_to":"3f79a3b5_ae65561f","updated":"2018-08-21 12:17:32.000000000","message":"I think he\u0027s expanding the comment to explain that the instance.az is set via get_host_availability_zone which relies on using CONF.default_availability_zone if instance.host is set but instance.availability_zone is not. For that I\u0027m OK with the comment change.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"},{"author":{"_account_id":9963,"name":"Jiri Suchomel","email":"jiri.suchomel@suse.com","username":"jsuchome"},"change_message_id":"04da687e4e3085ac1a796616fc682cd734b3dd8e","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        # we get only the instance whose avz was None and where host is set"},{"line_number":92,"context_line":"        self.assertEqual(1, count_all)"},{"line_number":93,"context_line":"        self.assertEqual(1, count_hit)"},{"line_number":94,"context_line":"        # avz is set by get_host_availability_zone to"},{"line_number":95,"context_line":"        # CONF.default_availability_zone i.e \u0027nova\u0027 which is the default zone"},{"line_number":96,"context_line":"        # for compute services."},{"line_number":97,"context_line":"        inst1 \u003d objects.Instance.get_by_uuid(self.context, uuid1)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_49aa7c85","line":94,"range":{"start_line":94,"start_character":10,"end_line":94,"end_character":50},"in_reply_to":"3f79a3b5_ae65561f","updated":"2018-08-21 12:18:10.000000000","message":"OK, will do.","commit_id":"8183d10f6abc9dbe79472ec92d72ed95257e92f8"}]}