)]}'
{"nova/api/openstack/compute/servers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a64d837e8cd365cda81b0fd002d5d7b0cf8dd2e5","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        :raises: webob.exc.HTTPBadRequest if the host is not in the"},{"line_number":556,"context_line":"            availability zone"},{"line_number":557,"context_line":"        \"\"\""},{"line_number":558,"context_line":"        aggregates \u003d objects.AggregateList.get_by_metadata("},{"line_number":559,"context_line":"            context, key\u003d\u0027availability_zone\u0027, value\u003davailability_zone)"},{"line_number":560,"context_line":"        if not aggregates:"},{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"a8646b75_bf47d8e0","line":558,"updated":"2021-06-27 13:17:21.000000000","message":"If the user used the default availability zone, e.g. nova, then this query will return [] as I don\u0027t think Nova automatically adds the default availability_zone metadata key to aggregates. So you need to branch on \n\n  if availability_zone \u003d\u003d CONF.default_availability_zone:","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"504ad501722da10d49df64593286c3eac64b19b2","unresolved":false,"context_lines":[{"line_number":555,"context_line":"        :raises: webob.exc.HTTPBadRequest if the host is not in the"},{"line_number":556,"context_line":"            availability zone"},{"line_number":557,"context_line":"        \"\"\""},{"line_number":558,"context_line":"        aggregates \u003d objects.AggregateList.get_by_metadata("},{"line_number":559,"context_line":"            context, key\u003d\u0027availability_zone\u0027, value\u003davailability_zone)"},{"line_number":560,"context_line":"        if not aggregates:"},{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"76d0aea7_64e2874b","line":558,"in_reply_to":"a8646b75_bf47d8e0","updated":"2021-06-28 09:48:09.000000000","message":"Done","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"397fc5fb1f14bb683589ec57718cdefc5e9a406e","unresolved":true,"context_lines":[{"line_number":555,"context_line":"        :raises: webob.exc.HTTPBadRequest if the host is not in the"},{"line_number":556,"context_line":"            availability zone"},{"line_number":557,"context_line":"        \"\"\""},{"line_number":558,"context_line":"        aggregates \u003d objects.AggregateList.get_by_metadata("},{"line_number":559,"context_line":"            context, key\u003d\u0027availability_zone\u0027, value\u003davailability_zone)"},{"line_number":560,"context_line":"        if not aggregates:"},{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"f9bba0a4_5ee71c21","line":558,"in_reply_to":"a8646b75_bf47d8e0","updated":"2021-06-28 08:42:47.000000000","message":"This is absolutely right. We need to check this case.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a64d837e8cd365cda81b0fd002d5d7b0cf8dd2e5","unresolved":true,"context_lines":[{"line_number":559,"context_line":"            context, key\u003d\u0027availability_zone\u0027, value\u003davailability_zone)"},{"line_number":560,"context_line":"        if not aggregates:"},{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"},{"line_number":562,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        for aggregate in aggregates:"},{"line_number":565,"context_line":"            if host in aggregate.hosts:"}],"source_content_type":"text/x-python","patch_set":1,"id":"11a30321_075937eb","line":562,"range":{"start_line":562,"start_character":52,"end_line":562,"end_character":53},"updated":"2021-06-27 13:17:21.000000000","message":", availablity_zone)","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"504ad501722da10d49df64593286c3eac64b19b2","unresolved":false,"context_lines":[{"line_number":559,"context_line":"            context, key\u003d\u0027availability_zone\u0027, value\u003davailability_zone)"},{"line_number":560,"context_line":"        if not aggregates:"},{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"},{"line_number":562,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        for aggregate in aggregates:"},{"line_number":565,"context_line":"            if host in aggregate.hosts:"}],"source_content_type":"text/x-python","patch_set":1,"id":"e91d1053_536516fd","line":562,"range":{"start_line":562,"start_character":52,"end_line":562,"end_character":53},"in_reply_to":"11a30321_075937eb","updated":"2021-06-28 09:48:09.000000000","message":"Done","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"50a33c6c3f500bad92ad300af0ef0669e4029332","unresolved":true,"context_lines":[{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"},{"line_number":562,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        for aggregate in aggregates:"},{"line_number":565,"context_line":"            if host in aggregate.hosts:"},{"line_number":566,"context_line":"                return None"},{"line_number":567,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9c8823c8_16163a04","line":564,"updated":"2021-06-25 18:59:35.000000000","message":"nit: Maybe rewrite the end of the method to something like:\n\n  if not any([host in aggregate.hosts for aggregate in aggregates]):\n    raise\n\nI realize it\u0027s a bit slower because you\u0027re always iterating over all of the aggregates (as opposed to bailing when you find a match), but it seems more legible to me.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a64d837e8cd365cda81b0fd002d5d7b0cf8dd2e5","unresolved":true,"context_lines":[{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"},{"line_number":562,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        for aggregate in aggregates:"},{"line_number":565,"context_line":"            if host in aggregate.hosts:"},{"line_number":566,"context_line":"                return None"},{"line_number":567,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"abdafcdf_6813eed1","line":564,"in_reply_to":"9c8823c8_16163a04","updated":"2021-06-27 13:17:21.000000000","message":"Actually I think the performance can be kept if you use a generator expression instead of a list comprehension\n\n    if not any(host in aggregate.hosts for aggregate in aggregates):\n        raise\n\nas any() bails out early.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"504ad501722da10d49df64593286c3eac64b19b2","unresolved":false,"context_lines":[{"line_number":561,"context_line":"            msg \u003d _(\"Requested availability zone \u0027%s\u0027 does not exist\")"},{"line_number":562,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        for aggregate in aggregates:"},{"line_number":565,"context_line":"            if host in aggregate.hosts:"},{"line_number":566,"context_line":"                return None"},{"line_number":567,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c3b4a555_ad5f0d2e","line":564,"in_reply_to":"abdafcdf_6813eed1","updated":"2021-06-28 09:48:09.000000000","message":"Done","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a64d837e8cd365cda81b0fd002d5d7b0cf8dd2e5","unresolved":true,"context_lines":[{"line_number":568,"context_line":"        msg \u003d _("},{"line_number":569,"context_line":"            \"Requested host \u0027%s\u0027 is not part of availability zone \u0027%s\u0027\""},{"line_number":570,"context_line":"        )"},{"line_number":571,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"    @staticmethod"},{"line_number":574,"context_line":"    def _process_hosts_for_create("}],"source_content_type":"text/x-python","patch_set":1,"id":"abaf29cf_6482de62","line":571,"range":{"start_line":571,"start_character":48,"end_line":571,"end_character":49},"updated":"2021-06-27 13:17:21.000000000","message":", host, availability_zone)","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"504ad501722da10d49df64593286c3eac64b19b2","unresolved":false,"context_lines":[{"line_number":568,"context_line":"        msg \u003d _("},{"line_number":569,"context_line":"            \"Requested host \u0027%s\u0027 is not part of availability zone \u0027%s\u0027\""},{"line_number":570,"context_line":"        )"},{"line_number":571,"context_line":"        raise exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"    @staticmethod"},{"line_number":574,"context_line":"    def _process_hosts_for_create("}],"source_content_type":"text/x-python","patch_set":1,"id":"5377a870_d69862a0","line":571,"range":{"start_line":571,"start_character":48,"end_line":571,"end_character":49},"in_reply_to":"abaf29cf_6482de62","updated":"2021-06-28 09:48:09.000000000","message":"Done","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"397fc5fb1f14bb683589ec57718cdefc5e9a406e","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"30200444_3fb1417b","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"updated":"2021-06-28 08:42:47.000000000","message":"Honestly, instead of making dangerous assumptions, we should stop supporting the AZ force_hosts hack and remove this BY A SPECIFIC NEW API MICROVERSION.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"efd18052676c1b3fc4e64f6528bfd431d53a545c","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"73c75cf5_411e76d7","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"024aaf6e_468a7c9d","updated":"2021-06-28 16:23:19.000000000","message":"\u003e The logs are eventually there and the consensus is https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2021-06-28.log.html#t2021-06-28T13:25:49\n\u003e \n\u003e 1/ provide a new microversion for not calling the parse_az method\n\nNo issues with that, though it\u0027s unfortunate we didn\u0027t do it in 2.74.\n\n\u003e 2/ modify the returned value of the az argument in the parse_az returned tuple, to be schedule_default_az option value.\n\nWhat about if the host isn\u0027t in the availability zone? Surely the better option would be to not set it? Also, what about if the host *is* in the availability zone? We\u0027re back in the situation where the RequestSpec.availability_zone field information is wrong.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"82963775888b1f972bd50182d0ad212573d6ad03","unresolved":false,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9858d4c1_38e20998","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"30200444_3fb1417b","updated":"2021-06-28 09:58:42.000000000","message":"We already did that. However, users can request the old microversion and get themselves into this error state. We need to handle that.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"aba1bde9d6a83575c206efbb60c4c1944aabe444","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cf3ea74b_a22325e6","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"73c75cf5_411e76d7","updated":"2021-06-29 09:46:10.000000000","message":"\u003e \u003e 2/ modify the returned value of the az argument in the parse_az returned tuple, to be schedule_default_az option value.\n\u003e \n\u003e What about if the host isn\u0027t in the availability zone? Surely the better option would be to not set it? Also, what about if the host *is* in the availability zone? We\u0027re back in the situation where the RequestSpec.availability_zone field information is wrong.\n\n\nYou\u0027re making a great point. We should just return a None value for the az attribute in the tuple then, hence letting the instance to be movable between AZs.\nIf operators wanna stick to a specific AZ *and* target a specific host, then they should use both --availability_zone (without the az hack) and --host then.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78234221497f66b8af508b240cbda26174cdc1f8","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"aa6ad650_bb584053","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"9858d4c1_38e20998","updated":"2021-06-28 12:53:49.000000000","message":"Agreed on the fact that old microversions would continue to have the AZ hack, but when saying \"we already did that\", where are we not calling this method if the user uses the 2.74 microversion ?","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a3c3dc2ba792e48182362f8d0523bd67c297e805","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"024aaf6e_468a7c9d","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"a07edf25_2df7e8f9","updated":"2021-06-28 15:01:04.000000000","message":"The logs are eventually there and the consensus is https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2021-06-28.log.html#t2021-06-28T13:25:49\n\n1/ provide a new microversion for not calling the parse_az method\n2/ modify the returned value of the az argument in the parse_az returned tuple, to be schedule_default_az option value.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"191572077fad4f428d8a00c934bcb80c0559e3bf","unresolved":true,"context_lines":[{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":685,"context_line":"        try:"},{"line_number":686,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":687,"context_line":"                                                     availability_zone)"},{"line_number":688,"context_line":"        except exception.InvalidInput as err:"},{"line_number":689,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"},{"line_number":690,"context_line":"        if host or node:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a07edf25_2df7e8f9","line":687,"range":{"start_line":686,"start_character":0,"end_line":687,"end_character":71},"in_reply_to":"aa6ad650_bb584053","updated":"2021-06-28 14:13:08.000000000","message":"We discussed this over IRC and the consensus seems to be two-case: \n* we propose a new microversion for stopping to support the AZ hack (ie. calling parse_az)\n* we propose, instead of validating the input and breaking users, to modify the behaviour of parse_az which would return the value of the schedule_default_az config option for the returned availability_zone argument of the returned tuple (and log a warning besides)\n\nThis way, we would prevent a DB inconsistency by letting the instance to be movable (or stuck to a default AZ), and for operators who would both force a target and stick with another AZ, they could use --host and --availability_zone arguments (without the az hack syntax) post-2.74.\n\n(fwiw, eavesdrop is lagging a bit so I can\u0027t provide the upstream resolution but you can see the conversation by https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2021-06-28.log.html once it\u0027s fully updated - for the moment, the HTML rendering bot is lagging but the raw logs are available on https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2021-06-28.log )","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"397fc5fb1f14bb683589ec57718cdefc5e9a406e","unresolved":true,"context_lines":[{"line_number":691,"context_line":"            context.can(server_policies.SERVERS % \u0027create:forced_host\u0027,"},{"line_number":692,"context_line":"                        target\u003dtarget)"},{"line_number":693,"context_line":"            self._validate_host_availability_zone("},{"line_number":694,"context_line":"                context, availability_zone, host)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        if api_version_request.is_supported(req, min_version\u003d\u00272.74\u0027):"},{"line_number":697,"context_line":"            self._process_hosts_for_create(context, target, server_dict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"f760ba48_8f190353","line":694,"updated":"2021-06-28 08:42:47.000000000","message":"This is a dangerous behaviour. Operators who opt out from using the AZFilter would then see a huge upgrade impact as they would explicitely need to ask their end users to *not* use the availability_zone option when creating instances.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"efd18052676c1b3fc4e64f6528bfd431d53a545c","unresolved":true,"context_lines":[{"line_number":691,"context_line":"            context.can(server_policies.SERVERS % \u0027create:forced_host\u0027,"},{"line_number":692,"context_line":"                        target\u003dtarget)"},{"line_number":693,"context_line":"            self._validate_host_availability_zone("},{"line_number":694,"context_line":"                context, availability_zone, host)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        if api_version_request.is_supported(req, min_version\u003d\u00272.74\u0027):"},{"line_number":697,"context_line":"            self._process_hosts_for_create(context, target, server_dict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"b90e6790_4bdb9cd4","line":694,"in_reply_to":"9a8c1811_cc3d6db0","updated":"2021-06-28 16:23:19.000000000","message":"This code only triggers if you use the \u0027--availability-zone az:host\u0027 option though (it\u0027s guarded by the \u0027if host or node\u0027 check), which means the \u0027AvailabilityZoneFilter\u0027 never had any part to play in this and never will. This code won\u0027t trigger for people using standard \u0027--availability-zone az\u0027 (i.e. no host). Those will be handed by the AvailabilityZoneFilter as before, assuming it\u0027s enabled.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78234221497f66b8af508b240cbda26174cdc1f8","unresolved":true,"context_lines":[{"line_number":691,"context_line":"            context.can(server_policies.SERVERS % \u0027create:forced_host\u0027,"},{"line_number":692,"context_line":"                        target\u003dtarget)"},{"line_number":693,"context_line":"            self._validate_host_availability_zone("},{"line_number":694,"context_line":"                context, availability_zone, host)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        if api_version_request.is_supported(req, min_version\u003d\u00272.74\u0027):"},{"line_number":697,"context_line":"            self._process_hosts_for_create(context, target, server_dict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8c1811_cc3d6db0","line":694,"in_reply_to":"bcff88aa_ce12f4da","updated":"2021-06-28 12:53:49.000000000","message":"We would validate the requested availability zone against the host AZ even if the AZFilter isn\u0027t used, which would change the behaviour for operators not using it.","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"82963775888b1f972bd50182d0ad212573d6ad03","unresolved":true,"context_lines":[{"line_number":691,"context_line":"            context.can(server_policies.SERVERS % \u0027create:forced_host\u0027,"},{"line_number":692,"context_line":"                        target\u003dtarget)"},{"line_number":693,"context_line":"            self._validate_host_availability_zone("},{"line_number":694,"context_line":"                context, availability_zone, host)"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        if api_version_request.is_supported(req, min_version\u003d\u00272.74\u0027):"},{"line_number":697,"context_line":"            self._process_hosts_for_create(context, target, server_dict,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bcff88aa_ce12f4da","line":694,"in_reply_to":"f760ba48_8f190353","updated":"2021-06-28 09:58:42.000000000","message":"Huh, why? I don\u0027t understand this point","commit_id":"0a62427434a67778f025191f7984230c9b8ea670"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9b224909444a4f80ca97838c8253113546b2ecbe","unresolved":true,"context_lines":[{"line_number":621,"context_line":"            \"requested availability zone to avoid bug #1934770\","},{"line_number":622,"context_line":"            availability_zone, host,"},{"line_number":623,"context_line":"        )"},{"line_number":624,"context_line":"        return host_availability_zone"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"    @staticmethod"},{"line_number":627,"context_line":"    def _process_hosts_for_create("}],"source_content_type":"text/x-python","patch_set":3,"id":"c0d1d143_54685855","line":624,"updated":"2021-07-06 12:55:55.000000000","message":"No, we said before to return a None value : \nhttps://review.opendev.org/c/openstack/nova/+/798145/1/nova/api/openstack/compute/servers.py#687\n\nBy getting a None value, the instance could be moved to any AZ.","commit_id":"a4dfaac3d43ed969258ca138154a8a57bc532c44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"97eae50d555ba9f79da17dcdee01f06d40e780b5","unresolved":false,"context_lines":[{"line_number":621,"context_line":"            \"requested availability zone to avoid bug #1934770\","},{"line_number":622,"context_line":"            availability_zone, host,"},{"line_number":623,"context_line":"        )"},{"line_number":624,"context_line":"        return host_availability_zone"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"    @staticmethod"},{"line_number":627,"context_line":"    def _process_hosts_for_create("}],"source_content_type":"text/x-python","patch_set":3,"id":"eb2deebb_c384ae70","line":624,"in_reply_to":"c0d1d143_54685855","updated":"2021-07-06 13:40:43.000000000","message":"Done","commit_id":"a4dfaac3d43ed969258ca138154a8a57bc532c44"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9b224909444a4f80ca97838c8253113546b2ecbe","unresolved":true,"context_lines":[{"line_number":736,"context_line":""},{"line_number":737,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":738,"context_line":"        try:"},{"line_number":739,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":740,"context_line":"                                                     availability_zone)"},{"line_number":741,"context_line":"        except exception.InvalidInput as err:"},{"line_number":742,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"}],"source_content_type":"text/x-python","patch_set":3,"id":"a857a0a1_0772599e","line":739,"updated":"2021-07-06 12:55:55.000000000","message":"we should only call this method before a new microversion. Would you create a spec for this, please ?","commit_id":"a4dfaac3d43ed969258ca138154a8a57bc532c44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"97eae50d555ba9f79da17dcdee01f06d40e780b5","unresolved":false,"context_lines":[{"line_number":736,"context_line":""},{"line_number":737,"context_line":"        parse_az \u003d self.compute_api.parse_availability_zone"},{"line_number":738,"context_line":"        try:"},{"line_number":739,"context_line":"            availability_zone, host, node \u003d parse_az(context,"},{"line_number":740,"context_line":"                                                     availability_zone)"},{"line_number":741,"context_line":"        except exception.InvalidInput as err:"},{"line_number":742,"context_line":"            raise exc.HTTPBadRequest(explanation\u003dstr(err))"}],"source_content_type":"text/x-python","patch_set":3,"id":"84f6385f_44319b75","line":739,"in_reply_to":"a857a0a1_0772599e","updated":"2021-07-06 13:40:43.000000000","message":"Can do. Should be separate to this change though","commit_id":"a4dfaac3d43ed969258ca138154a8a57bc532c44"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"90b22a07487dcb14b015eab0a3659c5ffb1b0633","unresolved":true,"context_lines":[{"line_number":552,"context_line":"        This is slightly tricky and it\u0027s probably worth recapping how host"},{"line_number":553,"context_line":"        aggregates and availability zones are related before reading. Hosts can"},{"line_number":554,"context_line":"        belong to zero or more host aggregates, but they will always belong to"},{"line_number":555,"context_line":"        exactly one availability zone. If the user has set the availability"},{"line_number":556,"context_line":"        zone key on one of the host aggregates that the host is a member of"},{"line_number":557,"context_line":"        then the host will belong to this availability zone. If the user has"},{"line_number":558,"context_line":"        not set the availability zone key on any of the host aggregates that"}],"source_content_type":"text/x-python","patch_set":4,"id":"894e6e30_a5115dc9","line":555,"range":{"start_line":555,"start_character":46,"end_line":555,"end_character":50},"updated":"2021-07-20 09:23:11.000000000","message":"nit: in general, we use the word \u0027admin\u0027 in order to make sure it\u0027s not an end-user API.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"90b22a07487dcb14b015eab0a3659c5ffb1b0633","unresolved":true,"context_lines":[{"line_number":612,"context_line":"        ):"},{"line_number":613,"context_line":"            # special case the default availability zone since this won\u0027t (or"},{"line_number":614,"context_line":"            # rather shouldn\u0027t) be explicitly stored on any host aggregate"},{"line_number":615,"context_line":"            return availability_zone"},{"line_number":616,"context_line":""},{"line_number":617,"context_line":"        # no match, so use the host\u0027s availability zone information, if any"},{"line_number":618,"context_line":"        LOG.warning("}],"source_content_type":"text/x-python","patch_set":4,"id":"7657381d_e3e7989c","line":615,"updated":"2021-07-20 09:23:11.000000000","message":"Agreed with this, even if that would mean that the instance would only be within the default AZ.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"}],"nova/compute/api.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"90b22a07487dcb14b015eab0a3659c5ffb1b0633","unresolved":true,"context_lines":[{"line_number":2022,"context_line":"                raise exception.InvalidRequest(msg)"},{"line_number":2023,"context_line":""},{"line_number":2024,"context_line":"        filter_properties \u003d scheduler_utils.build_filter_properties("},{"line_number":2025,"context_line":"            scheduler_hints, forced_host, forced_node, flavor)"},{"line_number":2026,"context_line":""},{"line_number":2027,"context_line":"        return self._create_instance("},{"line_number":2028,"context_line":"            context, flavor,"}],"source_content_type":"text/x-python","patch_set":4,"id":"172af656_26c160a6","line":2025,"updated":"2021-07-20 09:23:11.000000000","message":"a bit of unneeded changes but meh.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"}],"nova/tests/unit/api/openstack/compute/test_servers.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"90b22a07487dcb14b015eab0a3659c5ffb1b0633","unresolved":true,"context_lines":[{"line_number":4490,"context_line":""},{"line_number":4491,"context_line":"        mock_get_host_aggs.assert_called_once()"},{"line_number":4492,"context_line":"        # we should see a log since the host doesn\u0027t belong to the requested AZ"},{"line_number":4493,"context_line":"        self.assertIn(\u0027bug #1934770\u0027, mock_log.warning.call_args[0][0])"},{"line_number":4494,"context_line":""},{"line_number":4495,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4496,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"6d2fab66_4614263d","line":4493,"updated":"2021-07-20 09:23:11.000000000","message":"we could also add some UTs that would verify we return a None AZ.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31d20b101def0abe256c5d5b0d9ed1bcc7a5de0f","unresolved":false,"context_lines":[{"line_number":4490,"context_line":""},{"line_number":4491,"context_line":"        mock_get_host_aggs.assert_called_once()"},{"line_number":4492,"context_line":"        # we should see a log since the host doesn\u0027t belong to the requested AZ"},{"line_number":4493,"context_line":"        self.assertIn(\u0027bug #1934770\u0027, mock_log.warning.call_args[0][0])"},{"line_number":4494,"context_line":""},{"line_number":4495,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4496,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"061eb252_e914b3f4","line":4493,"in_reply_to":"6d2fab66_4614263d","updated":"2021-07-20 18:14:52.000000000","message":"I can\u0027t do that easily since we don\u0027t return that in the response from \u0027POST /servers\u0027 and I can\u0027t fetch anything since we\u0027re not actually creating records here (it\u0027s a unit test, not a functional tests). The presence of the log serves as a reasonable heuristic, IMO, but I\u0027ve add a follow-up that mocks the underlying \u0027create\u0027 call to check what we\u0027re sending.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"75d1e31abcc25e6a6a69c678615cf556ec0d4764","unresolved":true,"context_lines":[{"line_number":4534,"context_line":""},{"line_number":4535,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4536,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"},{"line_number":4537,"context_line":"    def test_create_instance_az_host_mismatch_with_aggs_in_default_az("},{"line_number":4538,"context_line":"        self, mock_log, mock_get_host_aggs,"},{"line_number":4539,"context_line":"    ):"},{"line_number":4540,"context_line":"        \"\"\"User requests the default AZ and host is in aggregates without AZ\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"b893c750_39d896b9","line":4537,"range":{"start_line":4537,"start_character":32,"end_line":4537,"end_character":45},"updated":"2021-07-06 15:48:40.000000000","message":"is it really a mismatch?","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"801aef94b465ad387a3452a0007e2d12fa7d19b3","unresolved":true,"context_lines":[{"line_number":4534,"context_line":""},{"line_number":4535,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4536,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"},{"line_number":4537,"context_line":"    def test_create_instance_az_host_mismatch_with_aggs_in_default_az("},{"line_number":4538,"context_line":"        self, mock_log, mock_get_host_aggs,"},{"line_number":4539,"context_line":"    ):"},{"line_number":4540,"context_line":"        \"\"\"User requests the default AZ and host is in aggregates without AZ\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"18bd445a_6425e9fa","line":4537,"range":{"start_line":4537,"start_character":32,"end_line":4537,"end_character":45},"in_reply_to":"08f73bb1_3decc092","updated":"2021-07-06 15:57:34.000000000","message":"thanks. I got the reason now.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31d20b101def0abe256c5d5b0d9ed1bcc7a5de0f","unresolved":false,"context_lines":[{"line_number":4534,"context_line":""},{"line_number":4535,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4536,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"},{"line_number":4537,"context_line":"    def test_create_instance_az_host_mismatch_with_aggs_in_default_az("},{"line_number":4538,"context_line":"        self, mock_log, mock_get_host_aggs,"},{"line_number":4539,"context_line":"    ):"},{"line_number":4540,"context_line":"        \"\"\"User requests the default AZ and host is in aggregates without AZ\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"c0fcfb3b_8a38d202","line":4537,"range":{"start_line":4537,"start_character":32,"end_line":4537,"end_character":45},"in_reply_to":"18bd445a_6425e9fa","updated":"2021-07-20 18:14:52.000000000","message":"Ack","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bcb88b01c4e79e1d5c1552acd4f9ed05f6c14724","unresolved":true,"context_lines":[{"line_number":4534,"context_line":""},{"line_number":4535,"context_line":"    @mock.patch.object(objects.AggregateList, \u0027get_by_host\u0027)"},{"line_number":4536,"context_line":"    @mock.patch.object(servers, \u0027LOG\u0027)"},{"line_number":4537,"context_line":"    def test_create_instance_az_host_mismatch_with_aggs_in_default_az("},{"line_number":4538,"context_line":"        self, mock_log, mock_get_host_aggs,"},{"line_number":4539,"context_line":"    ):"},{"line_number":4540,"context_line":"        \"\"\"User requests the default AZ and host is in aggregates without AZ\"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"08f73bb1_3decc092","line":4537,"range":{"start_line":4537,"start_character":32,"end_line":4537,"end_character":45},"in_reply_to":"b893c750_39d896b9","updated":"2021-07-06 15:55:38.000000000","message":"Sort of? Technically there is a mismatch in that none of the aggregates have the availability_zone key set so availability_zone\u003d\u003dNone, but the user requested availability_zone\u003d$DEFAULT_AZ. Perhaps I could come up with a better name though. Any suggestions? This is similar to \u0027test_create_instance_az_host_mismatch_without_aggs_in_default_az\u0027 above, only there we have no host aggregates and therefore still no AZ.","commit_id":"8f21ee42bd66b62e75e14acf4e91b786d78b9168"}]}