)]}'
{"nova/scheduler/filter_scheduler.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"15da9fac05461e4b70255e159962f801ad4e596e","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        num_instances \u003d spec_obj.num_instances"},{"line_number":101,"context_line":"        for num in range(num_instances):"},{"line_number":102,"context_line":"            sorted_hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":103,"context_line":"            if not sorted_hosts:"},{"line_number":104,"context_line":"                break"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            chosen_host \u003d sorted_hosts[0]"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_8e506fd6","line":103,"updated":"2017-07-14 15:10:20.000000000","message":"Hmm, the original code passes the return value from get_filtered_hosts() back to get_filtered_hosts() on the next iteration of the loop... Perhaps that is the culprit here.","commit_id":"84b8f7b69acdde7ea23d3a789aff595c6fc5bab2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f099d38392991626635d4c12e1421eadd005ff69","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        if not filtered_hosts:"},{"line_number":128,"context_line":"            return []"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        LOG.debug(\"Filtered %(hosts)s\", {\u0027hosts\u0027: filtered_hosts})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        weighed_hosts \u003d self.host_manager.get_weighed_hosts(filtered_hosts,"},{"line_number":133,"context_line":"            spec_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_5a410c39","line":130,"updated":"2017-07-13 21:54:27.000000000","message":"Since you\u0027re changing this anyway, you might want to move this logging above the if statement on line 127 to help operators debug NoValidHost conditions.","commit_id":"84b8f7b69acdde7ea23d3a789aff595c6fc5bab2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"197b2a69f416cf3b2d56b57387eca14842921b58","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        if not filtered_hosts:"},{"line_number":128,"context_line":"            return []"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        LOG.debug(\"Filtered %(hosts)s\", {\u0027hosts\u0027: filtered_hosts})"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        weighed_hosts \u003d self.host_manager.get_weighed_hosts(filtered_hosts,"},{"line_number":133,"context_line":"            spec_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_d33cd0c0","line":130,"in_reply_to":"1f1a1f67_5a410c39","updated":"2017-07-14 14:38:50.000000000","message":"ack. will do.","commit_id":"84b8f7b69acdde7ea23d3a789aff595c6fc5bab2"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f099d38392991626635d4c12e1421eadd005ff69","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":147,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":148,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":149,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":152,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_da4d1c73","line":149,"updated":"2017-07-13 21:54:27.000000000","message":"The conditional based on subset size isn\u0027t really needed. Something like this would work just fine:\n\n  pre_weigh, post_weigh \u003d (weighed_hosts[:host_subset_size],\n          weighed_hosts[host_subset_size:])\n  random.shuffle(pre_weigh)\n  return pre_weigh + post_weigh\n\nIt randomizes the first few hosts, based on subset_size, but I don\u0027t think that contradicts the spirit of what\u0027s being done here.","commit_id":"84b8f7b69acdde7ea23d3a789aff595c6fc5bab2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"197b2a69f416cf3b2d56b57387eca14842921b58","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":147,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":148,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":149,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":152,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1a1f67_539fa0b3","line":149,"in_reply_to":"1f1a1f67_da4d1c73","updated":"2017-07-14 14:38:50.000000000","message":"Sure, though I think what I\u0027ve got above is a bit more readable?","commit_id":"84b8f7b69acdde7ea23d3a789aff595c6fc5bab2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3583a5c05a351b0bc7e60536b371aea6a1ea848","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        filtered_hosts \u003d self.host_manager.get_filtered_hosts(host_states,"},{"line_number":126,"context_line":"            spec_obj, index)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        LOG.debug(\"Filtered %(hosts)s\", {\u0027hosts\u0027: filtered_hosts})"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        if not filtered_hosts:"},{"line_number":131,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_c9cc0abe","line":128,"updated":"2017-07-15 20:13:25.000000000","message":"This used to go after we decided if we got anything.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"35f00278875472d9725fcd8bfe6e42308ee95116","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        filtered_hosts \u003d self.host_manager.get_filtered_hosts(host_states,"},{"line_number":126,"context_line":"            spec_obj, index)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        LOG.debug(\"Filtered %(hosts)s\", {\u0027hosts\u0027: filtered_hosts})"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        if not filtered_hosts:"},{"line_number":131,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_1e91ff4e","line":128,"in_reply_to":"1f1a1f67_c9cc0abe","updated":"2017-07-17 13:08:42.000000000","message":"Yes, I had asked in the previous PS to move it. It would help in debugging NoValidHost situations.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e5b90a0d7a0178db98a5196a49d65f300980528a","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # congestion where the same host is consistently selected among"},{"line_number":142,"context_line":"        # numerous potential hosts for similar request specs."},{"line_number":143,"context_line":"        host_subset_size \u003d CONF.filter_scheduler.host_subset_size"},{"line_number":144,"context_line":"        if host_subset_size \u003c len(weighed_hosts):"},{"line_number":145,"context_line":"            weighed_subset \u003d weighed_hosts[0:host_subset_size]"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_127119aa","line":148,"range":{"start_line":144,"start_character":0,"end_line":148,"end_character":51},"updated":"2017-07-14 20:10:42.000000000","message":"OK, if you don\u0027t like the code I posted before, you can keep the readability of this and get rid of the \u0027if\u0027 conditional:\n\n  host_subset_size \u003d CONF.filter_scheduler.host_subset_size\n  chosen_host \u003d random.choice(weighed_hosts[:host_subset_size])\n\nSee http://paste.openstack.org/show/615427/ if you don\u0027t believe me.\n\nI think I complained about this several years ago when it was first introduced. :)","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"35f00278875472d9725fcd8bfe6e42308ee95116","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # congestion where the same host is consistently selected among"},{"line_number":142,"context_line":"        # numerous potential hosts for similar request specs."},{"line_number":143,"context_line":"        host_subset_size \u003d CONF.filter_scheduler.host_subset_size"},{"line_number":144,"context_line":"        if host_subset_size \u003c len(weighed_hosts):"},{"line_number":145,"context_line":"            weighed_subset \u003d weighed_hosts[0:host_subset_size]"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_7e759331","line":148,"range":{"start_line":144,"start_character":0,"end_line":148,"end_character":51},"in_reply_to":"1f1a1f67_0db33cec","updated":"2017-07-17 13:08:42.000000000","message":"If it wasn\u0027t I wouldn\u0027t have mentioned it.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e3cff4f56f08cfe7bd04e9dc6e217712b941ee6e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        # congestion where the same host is consistently selected among"},{"line_number":142,"context_line":"        # numerous potential hosts for similar request specs."},{"line_number":143,"context_line":"        host_subset_size \u003d CONF.filter_scheduler.host_subset_size"},{"line_number":144,"context_line":"        if host_subset_size \u003c len(weighed_hosts):"},{"line_number":145,"context_line":"            weighed_subset \u003d weighed_hosts[0:host_subset_size]"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_0db33cec","line":148,"range":{"start_line":144,"start_character":0,"end_line":148,"end_character":51},"in_reply_to":"1f1a1f67_127119aa","updated":"2017-07-14 20:55:42.000000000","message":"Is this really a useful complaint to -1 this set over at this point the schedule? Honestly...","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3583a5c05a351b0bc7e60536b371aea6a1ea848","unresolved":false,"context_lines":[{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_29693e8a","line":149,"range":{"start_line":149,"start_character":8,"end_line":149,"end_character":41},"updated":"2017-07-15 20:13:25.000000000","message":"Why is this done? I\u0027m assuming so we can put it at the beginning of the returned list so that the caller can pick that one from the 0th index. A comment would be nice, or putting a comment in the docstring saying that the 0-index weighed host in the returned list is the chosen host.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3583a5c05a351b0bc7e60536b371aea6a1ea848","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":153,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_89f972c9","line":150,"range":{"start_line":150,"start_character":29,"end_line":150,"end_character":44},"updated":"2017-07-15 20:13:25.000000000","message":"As far as I can tell this isn\u0027t even used, so why return it? Is this here for using later with alternatives?","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7bcece08a9be173ff05547ddef1888323bb6eb83","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":153,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_d43da3c4","line":150,"range":{"start_line":150,"start_character":29,"end_line":150,"end_character":44},"in_reply_to":"1f1a1f67_7e46b3c2","updated":"2017-07-17 15:52:18.000000000","message":"That list of weighed hosts can be very large in memory, I\u0027ll provide a follow-up for just returning an iterator instead.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"35f00278875472d9725fcd8bfe6e42308ee95116","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":153,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_7e46b3c2","line":150,"range":{"start_line":150,"start_character":29,"end_line":150,"end_character":44},"in_reply_to":"1f1a1f67_89f972c9","updated":"2017-07-17 13:08:42.000000000","message":"Not just for that reason. We haven\u0027t claimed anything yet, so if claiming the chosen host fails, we can continue to try the remaining hosts.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ba952e82579d0ae34f442e64fcb847bbd4f69053","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":153,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_f4b41fd3","line":150,"range":{"start_line":150,"start_character":29,"end_line":150,"end_character":44},"in_reply_to":"1f1a1f67_d43da3c4","updated":"2017-07-17 16:05:51.000000000","message":"As discussed in IRC, this change is no different from how this worked before.\n\nself.host_manager.get_filtered_hosts returns an iterator of HostState objects, which are passed to self.host_manager.get_weighed_hosts which converts them to a sorted list of WeighedObject objects. So at that point you no longer have an iterator anyway, so this is really no different from before, it\u0027s just moving code around.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"97672c50b4915408537bd87de13a09dd4d4eaed2","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            weighed_subset \u003d weighed_hosts"},{"line_number":148,"context_line":"        chosen_host \u003d random.choice(weighed_subset)"},{"line_number":149,"context_line":"        weighed_hosts.remove(chosen_host)"},{"line_number":150,"context_line":"        return [chosen_host] + weighed_hosts"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_all_host_states(self, context, spec_obj, provider_summaries):"},{"line_number":153,"context_line":"        \"\"\"Template method, so a subclass can implement caching.\"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_34c5f7f1","line":150,"range":{"start_line":150,"start_character":29,"end_line":150,"end_character":44},"in_reply_to":"1f1a1f67_f4b41fd3","updated":"2017-07-17 16:18:05.000000000","message":"Yeah, we\u0027re already playing with full objects in memory before that change, yeepeee... :-(\n\nAnyway, not worth a problem with that change, I totally agree.","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f4472459ae2cd3b550c867e1db63dae46bbec5ff","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # pass None when we want to grab all compute nodes (for instance, when"},{"line_number":156,"context_line":"        # using the caching scheduler. We pass an empty dict when the Placement"},{"line_number":157,"context_line":"        # API found no providers that match the requested constraints."},{"line_number":158,"context_line":"        compute_uuids \u003d None"},{"line_number":159,"context_line":"        if provider_summaries is not None:"},{"line_number":160,"context_line":"            compute_uuids \u003d list(provider_summaries.keys())"},{"line_number":161,"context_line":"        return self.host_manager.get_host_states_by_uuids(context,"},{"line_number":162,"context_line":"                                                          compute_uuids,"},{"line_number":163,"context_line":"                                                          spec_obj)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f1a1f67_e9e0e6ce","line":163,"range":{"start_line":158,"start_character":8,"end_line":163,"end_character":67},"updated":"2017-07-15 20:39:11.000000000","message":"I ran the coverage report and it looks like with the test changes in this patch we no longer have test coverage of this method, at least with unit tests (I didn\u0027t check coverage via functional tests).","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ddc71ee9b2eb765a70f5ef310a59e84db003d57a","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # pass None when we want to grab all compute nodes (for instance, when"},{"line_number":156,"context_line":"        # using the caching scheduler. We pass an empty dict when the Placement"},{"line_number":157,"context_line":"        # API found no providers that match the requested constraints."},{"line_number":158,"context_line":"        compute_uuids \u003d None"},{"line_number":159,"context_line":"        if provider_summaries is not None:"},{"line_number":160,"context_line":"            compute_uuids \u003d list(provider_summaries.keys())"},{"line_number":161,"context_line":"        return self.host_manager.get_host_states_by_uuids(context,"},{"line_number":162,"context_line":"                                                          compute_uuids,"},{"line_number":163,"context_line":"                                                          spec_obj)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f287b81_3419e6d6","line":163,"range":{"start_line":158,"start_character":8,"end_line":163,"end_character":67},"in_reply_to":"1f1a1f67_e9e0e6ce","updated":"2017-08-23 17:53:01.000000000","message":"https://review.openstack.org/#/c/496803/","commit_id":"853e0ee88d18f52b8ede12d00fe1c1e90c7e553b"}]}