)]}'
{"nova/api/openstack/compute/plugins/v3/extended_availability_zone.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"de24c051180acc7745df0165f7cc2ca24769a8e9","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def _extend_server(self, context, server, instance):"},{"line_number":30,"context_line":"        key \u003d \"%s:availability_zone\" % PREFIX"},{"line_number":31,"context_line":"        az \u003d avail_zone.get_instance_availability_zone(context, instance)"},{"line_number":32,"context_line":"        server[key] \u003d az or \u0027\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @wsgi.extends"},{"line_number":35,"context_line":"    def show(self, req, resp_obj, id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_39768407","line":32,"updated":"2015-02-08 10:17:50.000000000","message":"When there is no host, there will return None previously. With this patch, use empty string instead of None. I think we shouldn\u0027t change it. We didn\u0027t allow empty string as az name.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"efce7c0ab08b8a5ee9fee96fa25422e1ac4c99ac","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def _extend_server(self, context, server, instance):"},{"line_number":30,"context_line":"        key \u003d \"%s:availability_zone\" % PREFIX"},{"line_number":31,"context_line":"        az \u003d avail_zone.get_instance_availability_zone(context, instance)"},{"line_number":32,"context_line":"        server[key] \u003d az or \u0027\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @wsgi.extends"},{"line_number":35,"context_line":"    def show(self, req, resp_obj, id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_94a577b7","line":32,"in_reply_to":"da86d52c_39768407","updated":"2015-02-08 13:58:00.000000000","message":"Well, previously it didn\u0027t return None but rather the default az whenever host was not set due to a bug in get_instance_availability_zone(). Tempest actually enforces az is a string.\n\nThe change here is to cover the scenario that neither host nor az is set in the instance, something that was never handled before due to this bug. With the bug now fixed I have to account for None being returned and convert it to a string to not fail tempest tests.\n\nShould I return something else? Default az maybe, I don\u0027t know.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2d9d6692f6e78f1e7e53966b5bf38eedb8a62aeb","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def _extend_server(self, context, server, instance):"},{"line_number":30,"context_line":"        key \u003d \"%s:availability_zone\" % PREFIX"},{"line_number":31,"context_line":"        az \u003d avail_zone.get_instance_availability_zone(context, instance)"},{"line_number":32,"context_line":"        server[key] \u003d az or \u0027\u0027"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @wsgi.extends"},{"line_number":35,"context_line":"    def show(self, req, resp_obj, id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_b4d0bbca","line":32,"in_reply_to":"da86d52c_94a577b7","updated":"2015-02-08 15:00:53.000000000","message":"I recheck again, \u0027default_availability_zone\u0027 can be set as empty string.  And no other chance to return None at here. How about just assign az to server[key] directly? Does make sense? (correct me if I\u0027m wrong...maybe I need go to sleep now.)","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"}],"nova/availability_zones.py":[{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e0de85833fe36bdf22e497fe1e68c99ad07c1e54","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_9f78daf3","line":183,"updated":"2015-01-23 11:41:45.000000000","message":"Hans, Shouldn\u0027t this code use instance.get()? as it used to be before? since the attribute may not exist and will end up in an error","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"80c0f9c1bb8622fdd41d2de3a9d6a772e369e96b","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_5b53d3e1","line":183,"in_reply_to":"1a930d6b_021489f3","updated":"2015-01-23 20:04:37.000000000","message":"Well, that applies to all NovaObject classes. They can all be created with missing attributes, still I see no reason to surround all attribute accesses with try..catch blocks.\n\nI have checked and from what I can tell all callers of this method does so with an instance object updated from db so if you don\u0027t feel strongly about it I see no reason to add a try..catch here.","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d987df984bb1cf3bb9b2b08dce3ad287306588b4","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_021489f3","line":183,"in_reply_to":"1a930d6b_192b2531","updated":"2015-01-23 15:42:38.000000000","message":"There is a possibility to see the host field not updated in the NovaObject. This way, it could raise an Exception if you were trying to call it. I agree with you that it would be really not possible to see this field not updated if the instance object would be updated thanks to the DB, but there is still a possibility to create an Instance object without calling the DB...","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"aa777420c406a33b63be92d68e50086b0a01dbda","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_9378e82c","line":183,"in_reply_to":"1a930d6b_5b53d3e1","updated":"2015-02-03 11:21:17.000000000","message":"The main problem I see with keeping your code is that it would change the error behaviour if *for some reason* an instance is not having the host field set\n\nIn that case, it would raise an Exception up to the API and would be shown up to the user while in the previous scenario, it was silently failing because it was buggy (host attribute was set to \u0027None\u0027 so the conditional below was not hitted...","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"8870e65f1f8be166229851e254e0c6eb5d1b9fef","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_192b2531","line":183,"in_reply_to":"1a930d6b_7f5a4e51","updated":"2015-01-23 12:59:07.000000000","message":"Why would these attributes not exist? We use this syntax all over without any issues. If the database entry that we query has a null value the object attribute will be None.","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b804b3a36b3ae6120ad90a41e3ba9e8022d7482c","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_7f5a4e51","line":183,"in_reply_to":"1a930d6b_9f78daf3","updated":"2015-01-23 11:46:07.000000000","message":"I\u0027m rather in favor of leaving the object attribute and maybe make sure that if there is an exception, then put it None\n\nIt should be better because we reduce the number of times we\u0027re keeping the dictionary compatiblity within the Instance object.","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"46f5283b17466c3ba8748d8c9d57c761d7c0bca3","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d instance.host"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_168dc631","line":183,"in_reply_to":"da86d52c_9378e82c","updated":"2015-02-03 11:48:26.000000000","message":"Ok, I see what you mean. Using getattr with a default of None would be the most objecty thing to do here I think then. Same below.","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"e0de85833fe36bdf22e497fe1e68c99ad07c1e54","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":187,"context_line":"        # record itself."},{"line_number":188,"context_line":"        return instance.availability_zone"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    cache_key \u003d _make_cache_key(host)"},{"line_number":191,"context_line":"    cache \u003d _get_cache()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a930d6b_ffcb9e1b","line":188,"updated":"2015-01-23 11:41:45.000000000","message":"Same question as above. else where previous code had .get to get the availability_zone attribute (see nova/api/openstack/compute/contrib/extended_availability_zone.py)","commit_id":"2762323baaf4a4a0b6448d601661e0bfeb0c3c52"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e919d2011390474143fd9b58aba45d0a957806e3","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d getattr(instance, \u0027host\u0027, None)"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_b6723547","line":183,"updated":"2015-02-03 13:51:30.000000000","message":"I\u0027m sorry but you would still get an Exception here:\n\n\u003e\u003e\u003e obj \u003d objects.Instance(id\u003d1)\n\u003e\u003e\u003e getattr(obj, \u0027host\u0027, None)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n  File \"/home/sbauza/git/openstack/nova/nova/objects/base.py\", line 72, in getter\n    self.obj_load_attr(name)\n  File \"/home/sbauza/git/openstack/nova/nova/objects/instance.py\", line 616, in obj_load_attr\n    reason\u003d\u0027attribute %s not lazy-loadable\u0027 % attrname)\nnova.exception.ObjectActionError: Object action obj_load_attr failed because: attribute host not lazy-loadable","commit_id":"66077c29f2acd4cc945652d660e8e0b112605c75"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"f8622fd25069373c5c3ce1de551feaada0874be4","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":182,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":183,"context_line":"    host \u003d getattr(instance, \u0027host\u0027, None)"},{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_acc0222b","line":183,"in_reply_to":"da86d52c_b6723547","updated":"2015-02-03 14:33:38.000000000","message":"Bah! Missed that one.\nThanks!","commit_id":"66077c29f2acd4cc945652d660e8e0b112605c75"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e919d2011390474143fd9b58aba45d0a957806e3","unresolved":false,"context_lines":[{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":187,"context_line":"        # record itself."},{"line_number":188,"context_line":"        az \u003d getattr(instance, \u0027availability_zone\u0027, None)"},{"line_number":189,"context_line":"        return az"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_56273912","line":187,"updated":"2015-02-03 13:51:30.000000000","message":"I appreciate the move, so the more I think about it, the more I guess that you should leave the dict notation on L183 and only fix what\u0027s wrong.\n\nSorry for cycling it around on your review, but now things are clear in my mind : stuff is broken, let\u0027s fix it but don\u0027t necessarly touch the object notations, that could be done on a separate patch","commit_id":"66077c29f2acd4cc945652d660e8e0b112605c75"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"f8622fd25069373c5c3ce1de551feaada0874be4","unresolved":false,"context_lines":[{"line_number":184,"context_line":"    if not host:"},{"line_number":185,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":187,"context_line":"        # record itself."},{"line_number":188,"context_line":"        az \u003d getattr(instance, \u0027availability_zone\u0027, None)"},{"line_number":189,"context_line":"        return az"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da86d52c_0c004e66","line":187,"in_reply_to":"da86d52c_56273912","updated":"2015-02-03 14:33:38.000000000","message":"Works for me.","commit_id":"66077c29f2acd4cc945652d660e8e0b112605c75"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"0c08e371c1fcf8d91798eaada9a1eb9affbd4c32","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":187,"context_line":"        # record itself."},{"line_number":188,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":189,"context_line":"        return az"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    cache_key \u003d _make_cache_key(host)"},{"line_number":192,"context_line":"    cache \u003d _get_cache()"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_e6eda9b8","line":189,"updated":"2015-02-04 13:28:05.000000000","message":"++ Thanks !","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"826d6a12e0f46189bc02c2419247be5a20161520","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    host \u003d instance.get(\u0027host\u0027)"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":174,"context_line":"        # record itself."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_fa21a34d","line":173,"updated":"2015-02-25 11:32:55.000000000","message":"The *may* means if specified on the boot arg, right? - Could be better to clearly explain it.","commit_id":"4da9148f89d09bd5a537ed64b36ef4726b510263"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"88dbf54ea212db015bda00004071204d955e94c4","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    host \u003d instance.get(\u0027host\u0027)"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":174,"context_line":"        # record itself."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_9b1fc96b","line":173,"in_reply_to":"ba7be1f8_154ac825","updated":"2015-02-25 13:02:55.000000000","message":"+1","commit_id":"4da9148f89d09bd5a537ed64b36ef4726b510263"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"f803feceebf0ce8acccb06583fe81d0581e9ad9c","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    host \u003d instance.get(\u0027host\u0027)"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":174,"context_line":"        # record itself."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_154ac825","line":173,"in_reply_to":"ba7be1f8_fa21a34d","updated":"2015-02-25 11:59:28.000000000","message":"This is just a copy/paste from code being moved. That said, I\u0027ll try to come up with something better. How about:\n\n\"...desired availability_zone stored in the instance record if the boot request specified one.\"","commit_id":"4da9148f89d09bd5a537ed64b36ef4726b510263"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4d7eed38d9ba28abc1621031928652c2b7062f39","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    host \u003d instance.get(\u0027host\u0027)"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":174,"context_line":"        # record itself."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"}],"source_content_type":"text/x-python","patch_set":8,"id":"da9b358b_95ff0b01","line":173,"updated":"2015-03-30 15:44:59.000000000","message":"You loose you change here https://review.openstack.org/#/c/149562/6/nova/availability_zones.py,cm - it was expected?\n\nTo have a note with *may* is probably not a good note, what do you think?","commit_id":"a22c95c101d2331ae3f7383f80e3d6d3e8bbee52"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"8af44543689bd7005272040ddfd95e5c26a6cb4b","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    host \u003d instance.get(\u0027host\u0027)"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone that *may* exist in the instance"},{"line_number":174,"context_line":"        # record itself."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"}],"source_content_type":"text/x-python","patch_set":8,"id":"da9b358b_6b1dc472","line":173,"in_reply_to":"da9b358b_95ff0b01","updated":"2015-03-30 16:41:32.000000000","message":"Not expected, but then I didn\u0027t even upload this patchset. I\u0027ll fix it right away. Thanks for pointing it out.","commit_id":"a22c95c101d2331ae3f7383f80e3d6d3e8bbee52"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b5c19fa9aa2cf5bc3c77be9070f20732987a8255","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":169,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":170,"context_line":"    host \u003d str(instance.get(\u0027host\u0027))"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        return None"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_34172553","side":"PARENT","line":170,"updated":"2015-05-06 21:17:26.000000000","message":"Is this not a bug fixed by this patch? If host isn\u0027t set, this would be the string \"None\" and go to the rest of the code.","commit_id":"e9d8d933859d2cbbab0f07194ca1f114e10bf38c"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"326c38cc61006306c946ed61681b292f19f4aa21","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":169,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":170,"context_line":"    host \u003d str(instance.get(\u0027host\u0027))"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        return None"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_81834bcc","side":"PARENT","line":170,"in_reply_to":"1aca2d91_34172553","updated":"2015-05-07 08:49:14.000000000","message":"Yeah, maybe we need to create a bug only for this, and update the commit msg. Just tbh, there are so much problems that this one is trivial IMHO.","commit_id":"e9d8d933859d2cbbab0f07194ca1f114e10bf38c"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"25cb115db0202cafa45f6346fb20e1afb35a5105","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"def get_instance_availability_zone(context, instance):"},{"line_number":169,"context_line":"    \"\"\"Return availability zone of specified instance.\"\"\""},{"line_number":170,"context_line":"    host \u003d str(instance.get(\u0027host\u0027))"},{"line_number":171,"context_line":"    if not host:"},{"line_number":172,"context_line":"        return None"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_dcbb38e4","side":"PARENT","line":170,"in_reply_to":"1aca2d91_81834bcc","updated":"2015-05-08 08:56:51.000000000","message":"Yes, this should have a bug or two associated with it.","commit_id":"e9d8d933859d2cbbab0f07194ca1f114e10bf38c"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"91e74901302992277b8d4ca0f09a7d0b04a8ab22","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5aa745d1_01f74ed4","line":175,"updated":"2015-04-16 13:48:35.000000000","message":"Is there anywhere left not passing around objects?","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"25cb115db0202cafa45f6346fb20e1afb35a5105","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_815b1d87","line":175,"in_reply_to":"1aca2d91_2100f733","updated":"2015-05-08 08:56:51.000000000","message":"In the cinder case, it also had the same code to display desired az, so that leaves the metadata service. Besides the service itself, metadata is also used by virt drivers, but at that point, host is set on the instance.\n\nI honestly don\u0027t know if it matters for the metadata service. As you point out, there is a bug in the code right now where host if not set becomes the string \"None\", which unless there is a host named \"None\" will lead to CONF.default_availability_zone being returned. This is clearly wrong and will change into something else once the instance actually gets placed.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"326c38cc61006306c946ed61681b292f19f4aa21","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_2100f733","line":175,"in_reply_to":"1aca2d91_cf761c3e","updated":"2015-05-07 08:49:14.000000000","message":"To be clear, instance AZ is not really useful.\n\nIf you see at my patch https://review.openstack.org/#/c/172906/3 I\u0027m trying to make sure that this information is good or not since the actual AZ is just given by the host.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"1741b24bc561505e6de0bc962c0763912cf5b490","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5aa745d1_75992c6e","line":175,"in_reply_to":"5aa745d1_01f74ed4","updated":"2015-04-16 14:57:08.000000000","message":"See discussion on patchset 1\u0026 2, it was agreed upon to not change the existing behavior here but rather in a separate patch.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b5c19fa9aa2cf5bc3c77be9070f20732987a8255","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_cf761c3e","line":175,"in_reply_to":"5aa745d1_75992c6e","updated":"2015-05-06 21:17:26.000000000","message":"Thinking more about it, this does seem like a significant change in behavior for callers other than the AZ API extension. In the extensions, they are falling back on the AZ from the instance to show \"desired AZ\" to the user if they list/show that instance. Maybe this is considered better than being technical about the fact that the instance isn\u0027t scheduled yet, for the sake of showing to users.\n\nHowever the other callers of get_instance_availability_zone are the metadata services and the cinder interface. Do they necessarily want to get an AZ for an instance before it\u0027s scheduled?","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"1bc506bd3fb8d25df0c7e1b365b8d113f9696c7a","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        # Likely hasn\u0027t reached a viable compute node yet so give back the"},{"line_number":173,"context_line":"        # desired availability_zone in the instance record if the boot request"},{"line_number":174,"context_line":"        # specified one."},{"line_number":175,"context_line":"        az \u003d instance.get(\u0027availability_zone\u0027)"},{"line_number":176,"context_line":"        return az"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    cache_key \u003d _make_cache_key(host)"}],"source_content_type":"text/x-python","patch_set":16,"id":"ba15a1d1_4281c38e","line":175,"updated":"2015-09-13 06:21:51.000000000","message":"is this not an instance object? can we use instance.availability_zone?","commit_id":"479005ce5a35039639fa809b1a4705c9d3dc28c4"}],"nova/tests/unit/api/openstack/compute/contrib/test_extended_availability_zone.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"de24c051180acc7745df0165f7cc2ca24769a8e9","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_795f1c76","line":117,"updated":"2015-02-08 10:17:50.000000000","message":"This test is changed means the behavior is changed.\n\nBut I\u0027m thinking of \u0027get_host_availability_zone\u0027 whether have chance return None.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"efce7c0ab08b8a5ee9fee96fa25422e1ac4c99ac","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_b4fd1b71","line":117,"in_reply_to":"da86d52c_795f1c76","updated":"2015-02-08 13:58:00.000000000","message":"I guess it could if CONF.default_availability_zone is set to None but that is not really the main issue here.\n\nThe thing is that no matter what the host az is set to, should we ever replace it with az from the instance, which is what this test checks? I think not because host az is always to be considered authoritative.\n\nInstance az gets set during boot if the user requests a specific az for it to boot in. If the instance is later associated with a host that says otherwise, the instance is undoubtedly in the az the host says, right?\n\nSo yes, this is a slight change of behavior. After this change, the only time we report back the instance az instead of the hosts az is during boot before the host has been set. At that time we know it is going to be booted on a host in the az as requested by the user.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2d9d6692f6e78f1e7e53966b5bf38eedb8a62aeb","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_ef117a54","line":117,"in_reply_to":"da86d52c_b4fd1b71","updated":"2015-02-08 15:00:53.000000000","message":"This testcase sound like tests a case that never happened.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":12861,"name":"Trung Trinh","email":"trungtt1981@gmail.com","username":"ttt"},"change_message_id":"2f954dd936213d70df08957b3d6182df1838a132","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_366c2608","line":117,"updated":"2015-02-12 08:10:20.000000000","message":"Can you explain why the expected az info returned is empty string?\n\nAccording to code, it should still return \u0027fakeaz\u0027.\nIn fact, at line #109, \"fake_compute_get_az\" returns a server instance whose keys \u0027host\u0027 and \u0027availability_zone\u0027 are set. Therefore, in this case, the returned az info must be \u0027fakeaz\u0027 according to source code of the function \"get_instance_availability_zone()\"","commit_id":"d1c6aa2cd7105b453132b20ae3035d35abb25fdc"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"72b1ba2a4c40f06783942ba2a0f39983dfc167de","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da86d52c_2965c9ae","line":117,"in_reply_to":"da86d52c_366c2608","updated":"2015-02-18 13:48:24.000000000","message":"Well, if host is set for an instance, then host az is what is returned by get_instance_availability_zone(). Since L110 returns None for host az, that is what will be returned.\n\nThe api code converts this to en empty string because we\u0027re not allowed to return None.\n\nInstance az is ignored here when host is set, which I believe is the correct behavior.","commit_id":"d1c6aa2cd7105b453132b20ae3035d35abb25fdc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d670688a03b9b7f96c24b09598531f28afa90236","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3acd31a7_434ca9c7","line":117,"updated":"2015-05-06 00:07:14.000000000","message":"In another comment you mention this patch isn\u0027t supposed to be changing behavior, but this shows it does. Is there a bug being fixed by this patch in addition to the code consolidation? If so, I think we should have a bug reference in the commit message.\n\nThis test is showing a scenario where an instance has a host, but the host has no availability zone, but the instance has an availability zone. This could happen (I think?) if an availability zone is removed from a compute host after an instance had already been created. Then when showing the instance, is it right to show the availability zone stored on the instance or empty string if the host it\u0027s on has had its availability zone removed? Point being it sounds like a bug fix.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"125522829b0f262b2d2b4840b812f1a0bd424e86","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_4169e18a","line":117,"in_reply_to":"1aca2d91_15d3273a","updated":"2015-05-06 10:53:17.000000000","message":"This test is problematic as it tests something that I believe should never happen. A host always belong to an az, if not from being associated with an aggregate, then the host will use CONF.default_availability_zone. This is what get_host_availability_zone() looks like:\n\n def get_host_availability_zone(context, host):\n     aggregates \u003d objects.AggregateList.get_by_host(context, host,\n                                                    key\u003d\u0027availability_zone\u0027)\n     if aggregates:\n         az \u003d aggregates[0].metadata[\u0027availability_zone\u0027]\n     else:\n         az \u003d CONF.default_availability_zone\n     return az\n\nThis test however tests when this method returns None, which it shouldn\u0027t unless CONF.default_availability_zone is explicitly set to None. Is that even possible? Maybe it is from a pure technical standpoint, but conceptually?\n\nIf it is, then I think we should be true to the user and display that the instance is on a host that is not part of any az, so yes this is a slight change of behavior for this edge case.\n\nTo add to what Sylvain are saying, the architecture behind availability zones is bad in some ways. Instance az should really go away since it is just a scheduler hint that for some reason gets saved to the instance table. Availability zone is really only decided by what host the instance is booted on.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e81e4f86f8759d1ba9c536775febe82916846302","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_82868811","line":117,"in_reply_to":"1aca2d91_4169e18a","updated":"2015-05-07 22:59:36.000000000","message":"Since the metadata server exposes the result of this function, this change means that instance metadata shows the hint before it\u0027s scheduled, where it used to show only None until it was placed. Now, that\u0027s not a huge exposure or probably that big of a deal, but saying the instance is in an az because we don\u0027t know yet seems wrong to me.\n\nMaking it factor in the config default does make sense to me.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"25cb115db0202cafa45f6346fb20e1afb35a5105","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_1c22d000","line":117,"in_reply_to":"1aca2d91_82868811","updated":"2015-05-08 08:56:51.000000000","message":"Well actually, due to a bug in the code right now, it will display CONF.default_availability_zone until placed, when it will pick up and display the correct host az.\n\nI think this all boils down to how trustworthy the hint is.\n\nOne solution would be to not consider the hint for other purposes than scheduling. That would mean not exposing it to users, metadata, cinder or whatever until we know the az for a fact.\n\nThe reason I think it is safe to assume the hint is correct however, is that the az is validated before being written to the instance record at build time. So as long as the instance fits on a host in that az, it should end up the same or else would fail to build altogether.","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f4b52b0214558ab8a5967ae1b4bfc970e9ed3cce","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        res \u003d self._make_request(url)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        self.assertEqual(res.status_int, 200)"},{"line_number":117,"context_line":"        self.assertAvailabilityZone(self._get_server(res.body), \u0027\u0027)"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def test_show_empty_host_az(self):"},{"line_number":120,"context_line":"        self.stubs.Set(compute.api.API, \u0027get\u0027, fake_compute_get_empty)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aca2d91_15d3273a","line":117,"in_reply_to":"3acd31a7_434ca9c7","updated":"2015-05-06 08:47:57.000000000","message":"Melanie, the whole AZ helper is quite buggy so we have to update it by some iterations. Here, Hans is trying to help us refactoring how the helper is working, and I\u0027m working to fix a bug which is quite painful :\nhttps://review.openstack.org/#/c/172906/\n\nSo, I would be in favor of iterating fast over all our changes, try to fix what has been clearly identified and then work on the left things.\n\nTbh, I\u0027m planning to talk on that during the Nova meetup on Friday just because AZs are like one of the most awful things we have (c)","commit_id":"a8a16f7b3eb255d2b14a1a1adc3562d1cf07f00f"}],"nova/tests/unit/test_availability_zones.py":[{"author":{"_account_id":12861,"name":"Trung Trinh","email":"trungtt1981@gmail.com","username":"ttt"},"change_message_id":"3ce2f7bc2c3671feaadd69587c8156990283bad8","unresolved":false,"context_lines":[{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def test_get_instance_availability_zone_default_value(self):"},{"line_number":238,"context_line":"        \"\"\"Test get right availability zone by given an instance.\"\"\""},{"line_number":239,"context_line":"        fake_inst \u003d objects.Instance(host\u003dself.host)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self.assertEqual(self.default_az,"},{"line_number":242,"context_line":"                az.get_instance_availability_zone(self.context, fake_inst))"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_2d827a52","line":239,"updated":"2015-02-10 07:05:25.000000000","message":"You can\u0027t use \"objects.Instance(...)\" to return a fake instance.\n\nYour current Unit-Test is all fine just because you are lucky.\n\nIn fact, this test-case and the one below are still fine because the key \"availability_zone\" is not accessed. Otherwise, you will get the following Traceback as it happened for my added Unit-Test test-case: http://paste.openstack.org/show/170478/\n\nFrom the Traceback, it can be seen that when accessing the key \"availability_zone\" then we\u0027ll get the Error: \"attribute availability_zone not lazy-loadable\".\n\nProof can be seen from source code as follows:\n\n    # nova\\objects\\instance.py\n    def obj_load_attr(self, attrname):\n        if attrname not in INSTANCE_OPTIONAL_ATTRS:\n            raise exception.ObjectActionError(\n                action\u003d\u0027obj_load_attr\u0027,\n                reason\u003d\u0027attribute %s not lazy-loadable\u0027 % attrname)\n\n\nThe Exception is raised because the attrname of \"availability_zone\" is not in the INSTANCE_OPTIONAL_ATTRS.\n\nSo, I strongly suggest that you use again the \"from nova.tests.unit.api.openstack import fakes\"","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":12861,"name":"Trung Trinh","email":"trungtt1981@gmail.com","username":"ttt"},"change_message_id":"9feabcc95c8d70c79a70dcabaff74a2dfd0948cd","unresolved":false,"context_lines":[{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def test_get_instance_availability_zone_default_value(self):"},{"line_number":238,"context_line":"        \"\"\"Test get right availability zone by given an instance.\"\"\""},{"line_number":239,"context_line":"        fake_inst \u003d objects.Instance(host\u003dself.host)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self.assertEqual(self.default_az,"},{"line_number":242,"context_line":"                az.get_instance_availability_zone(self.context, fake_inst))"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_04c0d04e","line":239,"in_reply_to":"da86d52c_106037a4","updated":"2015-02-12 02:55:05.000000000","message":"Hi Hans,\n\nThanks for your pointing out the \"objects.Instance(..., availability_zone\u003d...)\". I\u0027ll try this.","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"0d371e7c65cdaa22e52454feb11bf7dcf0d541b1","unresolved":false,"context_lines":[{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def test_get_instance_availability_zone_default_value(self):"},{"line_number":238,"context_line":"        \"\"\"Test get right availability zone by given an instance.\"\"\""},{"line_number":239,"context_line":"        fake_inst \u003d objects.Instance(host\u003dself.host)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        self.assertEqual(self.default_az,"},{"line_number":242,"context_line":"                az.get_instance_availability_zone(self.context, fake_inst))"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_106037a4","line":239,"in_reply_to":"da86d52c_2d827a52","updated":"2015-02-11 10:45:45.000000000","message":"Well, not exactly lucky ;-)\n\nSince the code being tested only involves certain attributes I only have to provide those. The fakes from before shouldn\u0027t really be used anymore since the code expects objects and therefore should be tested with objects, not dicts.\n\nIf you need other attributes for your changes to pass the tests, just add what you need like:\n\n  objects.Instance(..., availability_zone\u003d...)","commit_id":"dfc6a32fff8ffac28fe50047c761b72be1d34b0d"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"6dc31b44a0e72870f4337f6f3d925cc3b0a79c53","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    def test_get_instance_availability_zone_no_host(self):"},{"line_number":259,"context_line":"        \"\"\"Test get availability zone from instance if host not set.\"\"\""},{"line_number":260,"context_line":"        fake_inst \u003d objects.Instance(host\u003dNone, availability_zone\u003d\u0027inst-az\u0027)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        result \u003d az.get_instance_availability_zone(self.context, fake_inst)"},{"line_number":263,"context_line":"        self.assertEqual(\u0027inst-az\u0027, result)"}],"source_content_type":"text/x-python","patch_set":16,"id":"da20952f_aa6cfb36","line":260,"range":{"start_line":260,"start_character":66,"end_line":260,"end_character":75},"updated":"2015-09-09 15:22:38.000000000","message":"It is more readable to specify this value once as a variable, and then use that variable in the fake object creation and resulting test.","commit_id":"479005ce5a35039639fa809b1a4705c9d3dc28c4"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"c605afb2edae926b2164692c4aed4335055e9007","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"    def test_get_instance_availability_zone_no_host(self):"},{"line_number":259,"context_line":"        \"\"\"Test get availability zone from instance if host not set.\"\"\""},{"line_number":260,"context_line":"        fake_inst \u003d objects.Instance(host\u003dNone, availability_zone\u003d\u0027inst-az\u0027)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        result \u003d az.get_instance_availability_zone(self.context, fake_inst)"},{"line_number":263,"context_line":"        self.assertEqual(\u0027inst-az\u0027, result)"}],"source_content_type":"text/x-python","patch_set":16,"id":"da20952f_23a763a8","line":260,"range":{"start_line":260,"start_character":66,"end_line":260,"end_character":75},"in_reply_to":"da20952f_aa6cfb36","updated":"2015-09-11 12:42:00.000000000","message":"can we used a mock sentinel here? Magic strings were one of the reasons we got into some issue here in the first place.","commit_id":"479005ce5a35039639fa809b1a4705c9d3dc28c4"}]}