)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6aae68e088c9944b50d10fe40d3927628721d013","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add alternate hosts"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds alternate hosts to the return value from the scheduler. A"},{"line_number":10,"context_line":"later patch in the series will add allocation_candidates to the returned"},{"line_number":11,"context_line":"value. The patch after that will change the methods that call"},{"line_number":12,"context_line":"select_destinations() to properly handle the new data structure. Once"},{"line_number":13,"context_line":"that is done, we can begin work on implementing retrying builds in the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f5c4f37_fb724011","line":10,"range":{"start_line":10,"start_character":45,"end_line":10,"end_character":56},"updated":"2017-09-21 15:43:49.000000000","message":"s/candidates/requests/","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add alternate hosts"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds alternate hosts to the return value from the scheduler. A"},{"line_number":10,"context_line":"later patch in the series will add allocation_candidates to the returned"},{"line_number":11,"context_line":"value. The patch after that will change the methods that call"},{"line_number":12,"context_line":"select_destinations() to properly handle the new data structure. Once"},{"line_number":13,"context_line":"that is done, we can begin work on implementing retrying builds in the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"7f515b1d_551816d2","line":10,"range":{"start_line":10,"start_character":45,"end_line":10,"end_character":56},"in_reply_to":"9f5c4f37_fb724011","updated":"2017-09-21 22:17:35.000000000","message":"Done","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"508711f16475d336c1d2e5f9cdb9ed37711193ec","unresolved":false,"context_lines":[{"line_number":13,"context_line":"that is done, we can begin work on implementing retrying builds in the"},{"line_number":14,"context_line":"cell in the event of a failed build."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Blueprint: placement-allocation-requests"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I550c2b9b7f3eceac27fde253ba65fa6bb70fa875"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7f515b1d_d592c69f","line":16,"updated":"2017-10-03 16:34:32.000000000","message":"wrong blueprint, should be return-alternate-hosts","commit_id":"542a369fbfbcfe80610866f01a59e0a9077aba45"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add alternate hosts"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds alternate hosts to the return value from the scheduler. A"},{"line_number":10,"context_line":"later patch in the series will add allocation_requests to the returned"},{"line_number":11,"context_line":"value. The patch after that will change the methods that call"},{"line_number":12,"context_line":"select_destinations() to properly handle the new data structure. Once"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"3f4b6375_8c5ac4f0","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":65},"updated":"2017-10-20 13:44:52.000000000","message":"Not really, right? This does some work in the scheduler drivers but doesn\u0027t yet return it out of the select_destinations rpc call. That change requires the client asking for alternate hosts so it can expect that response format.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add alternate hosts"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds alternate hosts to the return value from the scheduler. A"},{"line_number":10,"context_line":"later patch in the series will add allocation_requests to the returned"},{"line_number":11,"context_line":"value. The patch after that will change the methods that call"},{"line_number":12,"context_line":"select_destinations() to properly handle the new data structure. Once"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"3f4b6375_84bcd4dc","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":65},"in_reply_to":"3f4b6375_8c5ac4f0","updated":"2017-10-20 19:03:21.000000000","message":"The earlier version did return alternates, but that was changed, and the commit message wasn\u0027t updated. I\u0027ll change it.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"}],"nova/conductor/manager.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"06ecf7ae8113177dec083c218dc8aa6ff56ac593","unresolved":false,"context_lines":[{"line_number":1054,"context_line":"                    # return the alternates, but that will be in the next patch"},{"line_number":1055,"context_line":"                    # in the series, where we also return the"},{"line_number":1056,"context_line":"                    # allocation_candidates. This way the RPC call is only"},{"line_number":1057,"context_line":"                    # changed once."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"    def _cleanup_build_artifacts(self, context, exc, instances, build_requests,"},{"line_number":1060,"context_line":"                                 request_specs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_ab1e9709","line":1057,"updated":"2017-07-24 17:41:59.000000000","message":"Not entirely following the above code comment. By \"above call\" what are you referring to exactly? Why would the call to build_and_run_instance() on the compute host return the set of alternates?","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d6335a2bd725893d6adcb413e687100c95964410","unresolved":false,"context_lines":[{"line_number":1054,"context_line":"                    # return the alternates, but that will be in the next patch"},{"line_number":1055,"context_line":"                    # in the series, where we also return the"},{"line_number":1056,"context_line":"                    # allocation_candidates. This way the RPC call is only"},{"line_number":1057,"context_line":"                    # changed once."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"    def _cleanup_build_artifacts(self, context, exc, instances, build_requests,"},{"line_number":1060,"context_line":"                                 request_specs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_c3dc1f76","line":1057,"in_reply_to":"ff346bd7_71b54031","updated":"2017-07-24 19:17:14.000000000","message":"gotcha.","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"aa1256be867c13e068bfedb8a3b76023955448d6","unresolved":false,"context_lines":[{"line_number":1054,"context_line":"                    # return the alternates, but that will be in the next patch"},{"line_number":1055,"context_line":"                    # in the series, where we also return the"},{"line_number":1056,"context_line":"                    # allocation_candidates. This way the RPC call is only"},{"line_number":1057,"context_line":"                    # changed once."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"    def _cleanup_build_artifacts(self, context, exc, instances, build_requests,"},{"line_number":1060,"context_line":"                                 request_specs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_71b54031","line":1057,"in_reply_to":"ff346bd7_ab1e9709","updated":"2017-07-24 18:40:16.000000000","message":"\"above call\" \u003d\u003d self.compute_rpcapi.build_and_run_instance()\n\nI should have said \"*send* the alternates\", along with the allocation candidates, to the target cell.","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"}],"nova/scheduler/chance.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            selected_hosts \u003d [random.choice(hosts)"},{"line_number":74,"context_line":"                    for i in range(num_instances)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_05a67d11","line":76,"updated":"2017-08-25 10:29:49.000000000","message":"would be good to move this particular comment to line 82...","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7a9f834dd233ac663624fac71fceaae1d85c16a0","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            selected_hosts \u003d [random.choice(hosts)"},{"line_number":74,"context_line":"                    for i in range(num_instances)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_6933b015","line":76,"in_reply_to":"7f287b81_05a67d11","updated":"2017-08-28 13:18:09.000000000","message":"Well, it does apply to here, too. Why else would we reduce the number of returned alternates?","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"5cd8974008785dd3fe43c0ef4c7fa51737a92f32","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            selected_hosts \u003d [random.choice(hosts)"},{"line_number":74,"context_line":"                    for i in range(num_instances)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_d666aac1","line":76,"in_reply_to":"7f287b81_6933b015","updated":"2017-08-29 07:29:22.000000000","message":"line 77 is simply making sure we don\u0027t return more alternates than we have hosts... it has nothing to do with returning dups as alternates.","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"91dba34e3355e3c3113d36c37740569d1edc50ba","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            selected_hosts \u003d [random.choice(hosts)"},{"line_number":74,"context_line":"                    for i in range(num_instances)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f2577a7_bac7a896","line":76,"in_reply_to":"7f287b81_d666aac1","updated":"2017-09-14 19:09:23.000000000","message":"It is the reason for limiting the number of alternatives. Otherwise, the while loop on L80 would never end.","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        # Don\u0027t change the return value in this patch. A later patch in this"},{"line_number":111,"context_line":"        # series will change all the method signatures to accept the new return"},{"line_number":112,"context_line":"        # data structure. This temporary value mimics the current return value"},{"line_number":113,"context_line":"        # of a list of hosts, one per instance."},{"line_number":114,"context_line":"        temp_ret \u003d [dest[0] for dest in dests]"},{"line_number":115,"context_line":"        return temp_ret"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_2588596f","line":113,"updated":"2017-08-25 10:29:49.000000000","message":"ack","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6aae68e088c9944b50d10fe40d3927628721d013","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":82,"context_line":"                if candidate in sel_plus_alts:"},{"line_number":83,"context_line":"                    continue"},{"line_number":84,"context_line":"                sel_plus_alts.append(candidate)"},{"line_number":85,"context_line":"            selected_host_lists.append(sel_plus_alts)"},{"line_number":86,"context_line":"        return selected_host_lists"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_5b5f0c41","line":84,"range":{"start_line":82,"start_character":16,"end_line":84,"end_character":47},"updated":"2017-09-21 15:43:49.000000000","message":"femto nit... could have done:\n\n if candidate not in sel_plus_alts:\n     sel_plus_alts.append(candidate)","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":82,"context_line":"                if candidate in sel_plus_alts:"},{"line_number":83,"context_line":"                    continue"},{"line_number":84,"context_line":"                sel_plus_alts.append(candidate)"},{"line_number":85,"context_line":"            selected_host_lists.append(sel_plus_alts)"},{"line_number":86,"context_line":"        return selected_host_lists"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_b5e3caa4","line":84,"range":{"start_line":82,"start_character":16,"end_line":84,"end_character":47},"in_reply_to":"9f5c4f37_5b5f0c41","updated":"2017-09-21 22:17:35.000000000","message":"Done","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c046631b077a0f1eed464361168fec35e29d9c1","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            host_list \u003d host_lists[idx]"},{"line_number":102,"context_line":"            host_states \u003d [host_cls(host, None, None)"},{"line_number":103,"context_line":"                    for host in host_list]"},{"line_number":104,"context_line":"            dests.append(host_states)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        if len(dests) \u003c num_instances:"},{"line_number":107,"context_line":"            reason \u003d _(\u0027There are not enough hosts available.\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"7f515b1d_a897133a","line":104,"updated":"2017-10-01 19:01:40.000000000","message":"cls, list, lists, states is all a bit confusing, especially the list lists. I kept going back and forth between this block and the earlier one.","commit_id":"542a369fbfbcfe80610866f01a59e0a9077aba45"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return hosts"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _schedule(self, context, topic, spec_obj, instance_uuids):"},{"line_number":54,"context_line":"        \"\"\"Picks a random list of hosts that are up.\"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        elevated \u003d context.elevated()"},{"line_number":57,"context_line":"        hosts \u003d self.hosts_up(elevated, topic)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_8c0f64e0","line":54,"updated":"2017-10-20 13:44:52.000000000","message":"nit: didn\u0027t really need to rephrase this as part of this change. If you\u0027re going to update the docstring, fill in the param and return descriptions.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        return hosts"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _schedule(self, context, topic, spec_obj, instance_uuids):"},{"line_number":54,"context_line":"        \"\"\"Picks a random list of hosts that are up.\"\"\""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        elevated \u003d context.elevated()"},{"line_number":57,"context_line":"        hosts \u003d self.hosts_up(elevated, topic)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_04a344a0","line":54,"in_reply_to":"3f4b6375_8c0f64e0","updated":"2017-10-20 19:03:21.000000000","message":"Done","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        num_instances \u003d len(instance_uuids)"},{"line_number":68,"context_line":"        selected_host_lists \u003d []"},{"line_number":69,"context_line":"        if len(hosts) \u003e\u003d num_instances:"},{"line_number":70,"context_line":"            selected_hosts \u003d random.sample(hosts, num_instances)"},{"line_number":71,"context_line":"        else:"},{"line_number":72,"context_line":"            # This will return dupes"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_4c95ec3f","line":69,"updated":"2017-10-20 13:44:52.000000000","message":"In general, a comment above this block of conditional code could be nice, or a :return: comment in the docstring if you\u0027re going to change the docstring anyway. Ultimately, what are we trying to return out of this - let\u0027s document that.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":73,"context_line":"            selected_hosts \u003d [random.choice(hosts)"},{"line_number":74,"context_line":"                    for i in range(num_instances)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_0c225401","line":76,"range":{"start_line":76,"start_character":8,"end_line":76,"end_character":46},"updated":"2017-10-20 13:44:52.000000000","message":"The dupes comment here with the one on L72 is a bit confusing. Might want to re-word one or both of those.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_8c008469","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":20},"updated":"2017-10-20 13:44:52.000000000","message":"Per L70, can\u0027t this be a list?\n\nselected_hosts \u003d random.sample(hosts, num_instances)","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_a7b3524d","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":20},"in_reply_to":"3f4b6375_8c008469","updated":"2017-10-20 19:03:21.000000000","message":"Yes, it is a list: one host per requested instance. Now we are iterating over that list of selected hosts, and one by one adding alternates.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"214ea5c998dfbc4e08ef6ffa7672a7c3cd5c6726","unresolved":false,"context_lines":[{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # We can\u0027t return dupes as alternates."},{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_08d8f27c","line":78,"range":{"start_line":78,"start_character":12,"end_line":78,"end_character":20},"in_reply_to":"3f4b6375_a7b3524d","updated":"2017-10-21 00:31:33.000000000","message":"Oh yeah, duh, for some reason I was thinking sel_host was a list, making selected_hosts a list of lists from L70 but it\u0027s not.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":82,"context_line":"                if candidate not in sel_plus_alts:"},{"line_number":83,"context_line":"                    sel_plus_alts.append(candidate)"},{"line_number":84,"context_line":"            selected_host_lists.append(sel_plus_alts)"},{"line_number":85,"context_line":"        return selected_host_lists"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_2c519063","line":83,"range":{"start_line":80,"start_character":12,"end_line":83,"end_character":51},"updated":"2017-10-20 13:44:52.000000000","message":"I was thinking we could just pull a subset of hosts of length alts_per_instance, but then that\u0027s not random alternates per selected host, right? And then we\u0027d be clobbering reschedules on the same alternates. Maybe add a comment to that effect as a reminder to why the alternates are chosen at random?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":82,"context_line":"                if candidate not in sel_plus_alts:"},{"line_number":83,"context_line":"                    sel_plus_alts.append(candidate)"},{"line_number":84,"context_line":"            selected_host_lists.append(sel_plus_alts)"},{"line_number":85,"context_line":"        return selected_host_lists"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_c791ae9b","line":83,"range":{"start_line":80,"start_character":12,"end_line":83,"end_character":51},"in_reply_to":"3f4b6375_2c519063","updated":"2017-10-20 19:03:21.000000000","message":"Earlier in the series, Dan wanted to ensure that none of the selected hosts appeared in any of the other instances\u0027 alternates, so that\u0027s why it isn\u0027t a purely random choice. I\u0027ll add a comment about that.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"214ea5c998dfbc4e08ef6ffa7672a7c3cd5c6726","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        alts_per_instance \u003d min(len(hosts), CONF.scheduler.max_attempts)"},{"line_number":78,"context_line":"        for sel_host in selected_hosts:"},{"line_number":79,"context_line":"            sel_plus_alts \u003d [sel_host]"},{"line_number":80,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":81,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":82,"context_line":"                if candidate not in sel_plus_alts:"},{"line_number":83,"context_line":"                    sel_plus_alts.append(candidate)"},{"line_number":84,"context_line":"            selected_host_lists.append(sel_plus_alts)"},{"line_number":85,"context_line":"        return selected_host_lists"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_e8e6b644","line":83,"range":{"start_line":80,"start_character":12,"end_line":83,"end_character":51},"in_reply_to":"3f4b6375_c791ae9b","updated":"2017-10-21 00:31:33.000000000","message":"Yup, cool.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            alloc_reqs_by_rp_uuid, provider_summaries):"},{"line_number":89,"context_line":"        \"\"\"Selects random destinations.\"\"\""},{"line_number":90,"context_line":"        num_instances \u003d spec_obj.num_instances"},{"line_number":91,"context_line":"        # NOTE(timello): Returns a list of list of dicts with \u0027host\u0027,"},{"line_number":92,"context_line":"        # \u0027nodename\u0027 and \u0027limits\u0027 as keys for compatibility with"},{"line_number":93,"context_line":"        # filter_scheduler."},{"line_number":94,"context_line":"        # TODO(danms): This needs to be extended to support multiple cells"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_8cd1a4d0","line":91,"range":{"start_line":91,"start_character":51,"end_line":91,"end_character":56},"updated":"2017-10-20 13:44:52.000000000","message":"It\u0027s not actually a dict, right? It\u0027s a HostState object.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f5539bede2f4078e5213bf610a385ee7a26ab40c","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            while len(sel_plus_alts) \u003c alts_per_instance:"},{"line_number":84,"context_line":"                candidate \u003d random.choice(hosts)"},{"line_number":85,"context_line":"                if (candidate not in sel_plus_alts) and ("},{"line_number":86,"context_line":"                        candidate not in selected_hosts):"},{"line_number":87,"context_line":"                    # We don\u0027t want to include a selected host as an alternate,"},{"line_number":88,"context_line":"                    # as it will have a high liklihood of not having enough"},{"line_number":89,"context_line":"                    # resources left after it has an instance built on it."}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_2874ce67","line":86,"range":{"start_line":86,"start_character":24,"end_line":86,"end_character":55},"updated":"2017-10-21 00:39:35.000000000","message":"This wasn\u0027t in PS20 - must have found a problem when adding the assertion in the test that duplicates wouldn\u0027t be in the alternates list?","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f5539bede2f4078e5213bf610a385ee7a26ab40c","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                if (candidate not in sel_plus_alts) and ("},{"line_number":86,"context_line":"                        candidate not in selected_hosts):"},{"line_number":87,"context_line":"                    # We don\u0027t want to include a selected host as an alternate,"},{"line_number":88,"context_line":"                    # as it will have a high liklihood of not having enough"},{"line_number":89,"context_line":"                    # resources left after it has an instance built on it."},{"line_number":90,"context_line":"                    sel_plus_alts.append(candidate)"},{"line_number":91,"context_line":"            selected_host_lists.append(sel_plus_alts)"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_685ac6f3","line":88,"range":{"start_line":88,"start_character":45,"end_line":88,"end_character":54},"updated":"2017-10-21 00:39:35.000000000","message":"likelihood","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"}],"nova/scheduler/filter_scheduler.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"06ecf7ae8113177dec083c218dc8aa6ff56ac593","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        for host_list in selected_hosts:"},{"line_number":242,"context_line":"            for alt_host in hosts:"},{"line_number":243,"context_line":"                target_cell_uuid \u003d cell_uuids[host_list[0]]"},{"line_number":244,"context_line":"                if alt_host.obj.cell_uuid \u003d\u003d target_cell_uuid:"},{"line_number":245,"context_line":"                    host_list.append(alt_host)"},{"line_number":246,"context_line":"                if len(host_list) \u003e\u003d CONF.scheduler.max_attempts:"},{"line_number":247,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_cb21db4f","line":244,"updated":"2017-07-24 17:41:59.000000000","message":"This will end up adding the selected host into the list of alternates. You\u0027ll need to filter out the one that was already claimed/selected. Also, I\u0027d recommend excluding hosts from the alternates list that may have failed the claim process for one reason or another (notably, if the claim failed because the host happened to be filled up by another process during scheduling, there\u0027s no point adding it to the list of alternates).\n\nI would recommend moving this section into the code right above line 207, but I realize you also need to do this for the caching scheduler, so doing it here is probably necessary. :(","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d6335a2bd725893d6adcb413e687100c95964410","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        for host_list in selected_hosts:"},{"line_number":242,"context_line":"            for alt_host in hosts:"},{"line_number":243,"context_line":"                target_cell_uuid \u003d cell_uuids[host_list[0]]"},{"line_number":244,"context_line":"                if alt_host.obj.cell_uuid \u003d\u003d target_cell_uuid:"},{"line_number":245,"context_line":"                    host_list.append(alt_host)"},{"line_number":246,"context_line":"                if len(host_list) \u003e\u003d CONF.scheduler.max_attempts:"},{"line_number":247,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_c3c53f90","line":244,"in_reply_to":"ff346bd7_71e30035","updated":"2017-07-24 19:17:14.000000000","message":"k.","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"aa1256be867c13e068bfedb8a3b76023955448d6","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        for host_list in selected_hosts:"},{"line_number":242,"context_line":"            for alt_host in hosts:"},{"line_number":243,"context_line":"                target_cell_uuid \u003d cell_uuids[host_list[0]]"},{"line_number":244,"context_line":"                if alt_host.obj.cell_uuid \u003d\u003d target_cell_uuid:"},{"line_number":245,"context_line":"                    host_list.append(alt_host)"},{"line_number":246,"context_line":"                if len(host_list) \u003e\u003d CONF.scheduler.max_attempts:"},{"line_number":247,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff346bd7_71e30035","line":244,"in_reply_to":"ff346bd7_cb21db4f","updated":"2017-07-24 18:40:16.000000000","message":"Since the \u0027hosts\u0027 list is ordered, the chosen host and any host that failed claims will be at the beginning of the list. I\u0027ll slice them off.","commit_id":"d3a90d7132e2f33a28bd59971ebf3307c4d91f60"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        \u0027chosen_host\u0027 has already had its resources claimed in Placement,"},{"line_number":52,"context_line":"        followed by zero or more alternates. The alternates are hosts that can"},{"line_number":53,"context_line":"        satisfy the request, and are included so that if the build for the"},{"line_number":54,"context_line":"        chosen host fails, the cell conductor can retry."},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        :param context: The RequestContext object"},{"line_number":57,"context_line":"        :param spec_obj: The RequestSpec object"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_85e70d41","line":54,"updated":"2017-08-25 10:29:49.000000000","message":"Should move this docstring change to the patch where you\u0027re actually returning the list of lists.","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            # host."},{"line_number":95,"context_line":"            for host_list in selected_host_lists:"},{"line_number":96,"context_line":"                # Do we need to update the alternates too? Or just the chosen"},{"line_number":97,"context_line":"                # (first) host?"},{"line_number":98,"context_line":"                host_list[0].updated \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_05cd5db7","line":97,"updated":"2017-08-25 10:29:49.000000000","message":"just the first host I would say.","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":262,"context_line":"            cell_uuid \u003d claimed_host.cell_uuid"},{"line_number":263,"context_line":"            # We won\u0027t consider any hosts prior to the claimed_host, as they"},{"line_number":264,"context_line":"            # have had their resources used up. So only consider alternates"},{"line_number":265,"context_line":"            # from the portion of the hosts list after the claimed_host."},{"line_number":266,"context_line":"            claim_pos \u003d hosts.index(claimed_host) + 1"},{"line_number":267,"context_line":"            for alt_host in hosts[claim_pos:]:"},{"line_number":268,"context_line":"                if alt_host.cell_uuid \u003d\u003d cell_uuid:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_c5f995c8","line":265,"updated":"2017-08-25 10:29:49.000000000","message":"++","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"717cdf8ecbd309a164e4cecc6218957b60d86d68","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    def select_destinations(self, context, spec_obj, instance_uuids,"},{"line_number":47,"context_line":"            alloc_reqs_by_rp_uuid, provider_summaries):"},{"line_number":48,"context_line":"        \"\"\"Returns a list of sorted lists of HostState objects (1 for each"},{"line_number":49,"context_line":"        instance) that satisfy the supplied request_spec. Each of those lists"},{"line_number":50,"context_line":"        consist of [chosen_host, alternate1, ..., alternateN], where the"},{"line_number":51,"context_line":"        \u0027chosen_host\u0027 has already had its resources claimed in Placement,"},{"line_number":52,"context_line":"        followed by zero or more alternates. The alternates are hosts that can"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_f68db77b","line":49,"range":{"start_line":49,"start_character":23,"end_line":49,"end_character":30},"updated":"2017-09-21 16:07:09.000000000","message":"satisfies or would satisfy","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    def select_destinations(self, context, spec_obj, instance_uuids,"},{"line_number":47,"context_line":"            alloc_reqs_by_rp_uuid, provider_summaries):"},{"line_number":48,"context_line":"        \"\"\"Returns a list of sorted lists of HostState objects (1 for each"},{"line_number":49,"context_line":"        instance) that satisfy the supplied request_spec. Each of those lists"},{"line_number":50,"context_line":"        consist of [chosen_host, alternate1, ..., alternateN], where the"},{"line_number":51,"context_line":"        \u0027chosen_host\u0027 has already had its resources claimed in Placement,"},{"line_number":52,"context_line":"        followed by zero or more alternates. The alternates are hosts that can"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_80364ab4","line":49,"range":{"start_line":49,"start_character":23,"end_line":49,"end_character":30},"in_reply_to":"9f5c4f37_f68db77b","updated":"2017-09-21 22:17:35.000000000","message":"Done","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"717cdf8ecbd309a164e4cecc6218957b60d86d68","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        # so prefer the length of instance_uuids unless it is None."},{"line_number":186,"context_line":"        num_instances \u003d (len(instance_uuids) if instance_uuids"},{"line_number":187,"context_line":"                         else spec_obj.num_instances)"},{"line_number":188,"context_line":"        for num in range(num_instances):"},{"line_number":189,"context_line":"            hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":190,"context_line":"            if not hosts:"},{"line_number":191,"context_line":"                # NOTE(jaypipes): If we get here, that means not all instances"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_f64b77ad","line":188,"updated":"2017-09-21 16:07:09.000000000","message":"So, I think that just iterating this list once is not going to give us the desired result. By going through this once, picking a primary and a set of alternates for each instance to be created, we\u0027re basically going to be guaranteed that alternates selected for early instances will be primaries for the later instances. This will almost always ensure that they won\u0027t be able to reschedule at all. Later instances will benefit from having alternates that weren\u0027t primary targets for any other instances.\n\nSo I think what we need to do is iterate this once, picking (and consuming) primary hosts for each of the instances to be created. Then, we can go back and pick the alternates for them all. The alternates can overlap with each other, but shouldn\u0027t overlap with any primaries.\n\nWe *could* just naively select max_attempts alternates from the remaining list when we\u0027re done and apply those to all the primaries. However (and maybe as an optimization later) it would probably be good to select several such slices of the remaining hosts, if we have enough.\n\nDoes the above make sense?","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"717cdf8ecbd309a164e4cecc6218957b60d86d68","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # We won\u0027t consider any hosts prior to the claimed_host, as they"},{"line_number":264,"context_line":"            # have had their resources used up. So only consider alternates"},{"line_number":265,"context_line":"            # from the portion of the hosts list after the claimed_host."},{"line_number":266,"context_line":"            claim_pos \u003d hosts.index(claimed_host) + 1"},{"line_number":267,"context_line":"            for alt_host in hosts[claim_pos:]:"},{"line_number":268,"context_line":"                if alt_host.cell_uuid \u003d\u003d cell_uuid:"},{"line_number":269,"context_line":"                    claim_and_alt.append(alt_host)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_56b1238f","line":266,"updated":"2017-09-21 16:07:09.000000000","message":"We should do this counting as part of the loop so we don\u0027t have to iterate the list just to find the position of the thing we already found. If we have this cranked up to return a bunch of hosts for each request, this list could be large.","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # We won\u0027t consider any hosts prior to the claimed_host, as they"},{"line_number":264,"context_line":"            # have had their resources used up. So only consider alternates"},{"line_number":265,"context_line":"            # from the portion of the hosts list after the claimed_host."},{"line_number":266,"context_line":"            claim_pos \u003d hosts.index(claimed_host) + 1"},{"line_number":267,"context_line":"            for alt_host in hosts[claim_pos:]:"},{"line_number":268,"context_line":"                if alt_host.cell_uuid \u003d\u003d cell_uuid:"},{"line_number":269,"context_line":"                    claim_and_alt.append(alt_host)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_e06f3e90","line":266,"in_reply_to":"9f5c4f37_56b1238f","updated":"2017-09-21 22:17:35.000000000","message":"OK, I\u0027ll see about re-working the iterations to take into account this and the comment above.","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d89870e7123e0d71a99ef509b2c17f698c903e97","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # The list to be returned, containing one item for each requested"},{"line_number":212,"context_line":"        # instance. Each of these items will be a list, containing the selected"},{"line_number":213,"context_line":"        # host + any alternates."},{"line_number":214,"context_line":"        selections_to_return \u003d []"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        for num in range(num_instances):"},{"line_number":217,"context_line":"            instance_uuid \u003d instance_uuids[num]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_ea514310","line":214,"updated":"2017-09-22 16:50:56.000000000","message":"You moved this down to the bottom, so you should remove this definition.","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f820f6fbfab6fa44d30f1078a7ba81f85d9ef19","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # The list to be returned, containing one item for each requested"},{"line_number":212,"context_line":"        # instance. Each of these items will be a list, containing the selected"},{"line_number":213,"context_line":"        # host + any alternates."},{"line_number":214,"context_line":"        selections_to_return \u003d []"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        for num in range(num_instances):"},{"line_number":217,"context_line":"            instance_uuid \u003d instance_uuids[num]"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_aaba2bdb","line":214,"in_reply_to":"7f515b1d_ea514310","updated":"2017-09-22 17:13:46.000000000","message":"Done","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d89870e7123e0d71a99ef509b2c17f698c903e97","unresolved":false,"context_lines":[{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":269,"context_line":"        # find alternates for each host."},{"line_number":270,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":271,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":272,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":273,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_8a976f91","line":270,"updated":"2017-09-22 16:50:56.000000000","message":"Can you add to this comment that this gives us a fresh view of the hosts that haven\u0027t already been claimed above? I spent an embarrassingly long time re-reading all this trying to articulate why you\u0027ve not done the thing I suggested in the previous set. I think this line is why you are, but it took me a while :P","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1935d43a6a4a7c7b78170f4ce55e8706306fd70f","unresolved":false,"context_lines":[{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":269,"context_line":"        # find alternates for each host."},{"line_number":270,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":271,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":272,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":273,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_add5a5bb","line":270,"in_reply_to":"7f515b1d_0a8a5f1a","updated":"2017-09-22 17:48:07.000000000","message":"Yeah, I wrote this before you answered me. I had missed the \"and not host in claim_hosts\" thing below.","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f820f6fbfab6fa44d30f1078a7ba81f85d9ef19","unresolved":false,"context_lines":[{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":269,"context_line":"        # find alternates for each host."},{"line_number":270,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":271,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":272,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":273,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_0a8a5f1a","line":270,"in_reply_to":"7f515b1d_8a976f91","updated":"2017-09-22 17:13:46.000000000","message":"Well, the hosts that have been claimed may still be in this list, but chances are that they will no longer be at the top.\n\nI can\u0027t just select a bunch of alternates and apply them as alternates for all the instances, because they could be in different cells. If it\u0027s OK to keep using the same hosts as alternates, I can remove the indexing.","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d89870e7123e0d71a99ef509b2c17f698c903e97","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                    break"},{"line_number":285,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in claimed_hosts:"},{"line_number":286,"context_line":"                    claimed_plus_alts.append(host)"},{"line_number":287,"context_line":"            selections_to_return.append(claimed_plus_alts)"},{"line_number":288,"context_line":"        return selections_to_return"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_ea3263a7","line":287,"updated":"2017-09-22 16:50:56.000000000","message":"This loop is really hard to follow and I\u0027m pretty sure I wouldn\u0027t understand it if I hadn\u0027t just read the old code. Maybe more words would help or maybe it could use some breaking up.\n\nThis is also giving each instance a unique slice of alternate hosts, correct? I feel like that\u0027s probably not going to work because it means you have to have num_instances*max_attempts hosts in your cluster in order for each instance to have the same chance of being rescheduled. If we _have_ that many hosts, it may be a good strategy, but I think what you have here will _require_ it, right?\n\nI was trying to say in my previous review that I think it\u0027s fine for all the alternates to overlap (maybe not always ideal, but reasonable). We could go though the list of instances, and if we have found a list of alternates for the cell we\u0027re targeting, then just re-use that list, else generate it once. What do you think of that?","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1935d43a6a4a7c7b78170f4ce55e8706306fd70f","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                    break"},{"line_number":285,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in claimed_hosts:"},{"line_number":286,"context_line":"                    claimed_plus_alts.append(host)"},{"line_number":287,"context_line":"            selections_to_return.append(claimed_plus_alts)"},{"line_number":288,"context_line":"        return selections_to_return"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_6dd7adc1","line":287,"in_reply_to":"7f515b1d_4afdf738","updated":"2017-09-22 17:48:07.000000000","message":"Yeah that sounds like a good plan.","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7f820f6fbfab6fa44d30f1078a7ba81f85d9ef19","unresolved":false,"context_lines":[{"line_number":284,"context_line":"                    break"},{"line_number":285,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in claimed_hosts:"},{"line_number":286,"context_line":"                    claimed_plus_alts.append(host)"},{"line_number":287,"context_line":"            selections_to_return.append(claimed_plus_alts)"},{"line_number":288,"context_line":"        return selections_to_return"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _cleanup_allocations(self, instance_uuids):"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f515b1d_4afdf738","line":287,"in_reply_to":"7f515b1d_ea3263a7","updated":"2017-09-22 17:13:46.000000000","message":"How about if I used itertools.cycle(hosts) to avoid running out?","commit_id":"6d044f852d146db65f8dc6408ca3261069df3e62"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"cfafa9bca5374487b91bffee7f644cd60105a3e2","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            # host."},{"line_number":95,"context_line":"            for host_list in selected_host_lists:"},{"line_number":96,"context_line":"                # Do we need to update the alternates too? Or just the chosen"},{"line_number":97,"context_line":"                # (first) host?"},{"line_number":98,"context_line":"                host_list[0].updated \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_346d6032","line":97,"updated":"2017-09-25 14:08:28.000000000","message":"Did this get resolved enough that the comment can be changed to an assertion not a question? If not, can we?","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f762a455ba34b22c519b2527842723a579e88f82","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            # host."},{"line_number":95,"context_line":"            for host_list in selected_host_lists:"},{"line_number":96,"context_line":"                # Do we need to update the alternates too? Or just the chosen"},{"line_number":97,"context_line":"                # (first) host?"},{"line_number":98,"context_line":"                host_list[0].updated \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_950f6be0","line":97,"in_reply_to":"7f515b1d_346d6032","updated":"2017-09-25 16:13:18.000000000","message":"Yes, I need to remove this comment. Only the selected needs to have updated cleared.","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"cfafa9bca5374487b91bffee7f644cd60105a3e2","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        if (instance_uuids is None or"},{"line_number":182,"context_line":"                not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":183,"context_line":"                alloc_reqs_by_rp_uuid is None):"},{"line_number":184,"context_line":"            # Unfortunately, we still need to deal with older conductors"},{"line_number":185,"context_line":"            # that may not be passing in a list of instance_uuids. In those"},{"line_number":186,"context_line":"            # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":187,"context_line":"            # have instance UUIDs to claim with, so we just grab the first"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_54bb3c71","line":184,"updated":"2017-09-25 14:08:28.000000000","message":"I\u0027m confused. In some places (e.g. a spec I commented on earlier today) we assert that conductor, scheduler and placement are going to upgrade in sync, but here we say we need to deal with older conductors.\n\nWhich is it?","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"f762a455ba34b22c519b2527842723a579e88f82","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        if (instance_uuids is None or"},{"line_number":182,"context_line":"                not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":183,"context_line":"                alloc_reqs_by_rp_uuid is None):"},{"line_number":184,"context_line":"            # Unfortunately, we still need to deal with older conductors"},{"line_number":185,"context_line":"            # that may not be passing in a list of instance_uuids. In those"},{"line_number":186,"context_line":"            # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":187,"context_line":"            # have instance UUIDs to claim with, so we just grab the first"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_35ad5731","line":184,"in_reply_to":"7f515b1d_54bb3c71","updated":"2017-09-25 16:13:18.000000000","message":"I think you\u0027re right about that. That was based on some discussion for which I don\u0027t recall the details. The other two cases are still valid, though.","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6b7cdede49a6f92aaa31763a1d20757041f04472","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        if (instance_uuids is None or"},{"line_number":182,"context_line":"                not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":183,"context_line":"                alloc_reqs_by_rp_uuid is None):"},{"line_number":184,"context_line":"            # Unfortunately, we still need to deal with older conductors"},{"line_number":185,"context_line":"            # that may not be passing in a list of instance_uuids. In those"},{"line_number":186,"context_line":"            # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":187,"context_line":"            # have instance UUIDs to claim with, so we just grab the first"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_98cc42bd","line":184,"in_reply_to":"7f515b1d_54bb3c71","updated":"2017-09-25 16:23:03.000000000","message":"You will note that this comment is from the existing code. Older conductors (prior to scheduler RPC version 4.4 [1]) did not send the instance_uuids parameter to select_destinations() scheduler RPC call and we use the instance_uuids parameter to build the alloc_reqs_by_rp_uuid variable.\n\nWe should be able to remove this block of code now that Pike conductors should be speaking the 4.4 scheduler RPC API. However, before we can do that, we should add a patch that demarcates 4.4 as the Pike release version.\n\n[1] https://github.com/openstack/nova/blob/master/nova/scheduler/rpcapi.py#L98","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"e2515775975560bb3aeb27cd04e5eac80ed8722e","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        if (instance_uuids is None or"},{"line_number":182,"context_line":"                not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":183,"context_line":"                alloc_reqs_by_rp_uuid is None):"},{"line_number":184,"context_line":"            # Unfortunately, we still need to deal with older conductors"},{"line_number":185,"context_line":"            # that may not be passing in a list of instance_uuids. In those"},{"line_number":186,"context_line":"            # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":187,"context_line":"            # have instance UUIDs to claim with, so we just grab the first"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_7fa848da","line":184,"in_reply_to":"7f515b1d_98cc42bd","updated":"2017-09-25 18:50:03.000000000","message":"Well, the other two cases still apply, no? So I\u0027ll remove the comments regarding old conductors, and remove the test for no instance_uuids.","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"91dec3931821d97896e18bca30b265162e98a68f","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            # to clean up once we no longer have to worry about older"},{"line_number":199,"context_line":"            # conductors."},{"line_number":200,"context_line":"            return self._legacy_find_hosts(num_instances, spec_obj, hosts,"},{"line_number":201,"context_line":"                    num_to_return)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"        # A list of the instance UUIDs that were successfully claimed against"},{"line_number":204,"context_line":"        # in the placement API. If we are not able to successfully claim for"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f515b1d_ae76a2d7","line":201,"updated":"2017-09-25 11:00:55.000000000","message":"ack. ++","commit_id":"c4e34aef8b360b369ac569c0440b7ec1776858ec"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"246f8ca551e958d160a6311b775d1abf5c4572dc","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":214,"context_line":"                break"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"            if (not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":217,"context_line":"                    alloc_reqs_by_rp_uuid is None):"},{"line_number":218,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":219,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":220,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":221,"context_line":"                # have instance UUIDs to claim with, so we just grab the first"},{"line_number":222,"context_line":"                # host in the list of sorted hosts. In addition to older"},{"line_number":223,"context_line":"                # conductors, we need to support the caching scheduler, which"},{"line_number":224,"context_line":"                # doesn\u0027t use the placement API (and has"},{"line_number":225,"context_line":"                # USES_ALLOCATION_CANDIDATE \u003d False) and therefore we skip all"},{"line_number":226,"context_line":"                # the claiming logic for that scheduler driver. Finally, if"},{"line_number":227,"context_line":"                # there was a problem communicating with the placement API,"},{"line_number":228,"context_line":"                # alloc_reqs_by_rp_uuid will be None, so we skip claiming in"},{"line_number":229,"context_line":"                # that case as well"},{"line_number":230,"context_line":"                claimed_host \u003d hosts[0]"},{"line_number":231,"context_line":"            else:"},{"line_number":232,"context_line":"                instance_uuid \u003d instance_uuids[num]"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"                # Attempt to claim the resources against one or more resource"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f515b1d_9c4aefa9","line":231,"range":{"start_line":216,"start_character":12,"end_line":231,"end_character":17},"updated":"2017-09-25 20:12:50.000000000","message":"this entire block is dead code now that you\u0027ve moved this code to line 181 above.","commit_id":"626f45e072b47de2627a47ead12a8c33ee452309"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"47aea9e26a30d01f0d7877e795d99a96a68b0cf9","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":214,"context_line":"                break"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"            if (not self.USES_ALLOCATION_CANDIDATES or"},{"line_number":217,"context_line":"                    alloc_reqs_by_rp_uuid is None):"},{"line_number":218,"context_line":"                # Unfortunately, we still need to deal with older conductors"},{"line_number":219,"context_line":"                # that may not be passing in a list of instance_uuids. In those"},{"line_number":220,"context_line":"                # cases, obviously we can\u0027t claim resources because we don\u0027t"},{"line_number":221,"context_line":"                # have instance UUIDs to claim with, so we just grab the first"},{"line_number":222,"context_line":"                # host in the list of sorted hosts. In addition to older"},{"line_number":223,"context_line":"                # conductors, we need to support the caching scheduler, which"},{"line_number":224,"context_line":"                # doesn\u0027t use the placement API (and has"},{"line_number":225,"context_line":"                # USES_ALLOCATION_CANDIDATE \u003d False) and therefore we skip all"},{"line_number":226,"context_line":"                # the claiming logic for that scheduler driver. Finally, if"},{"line_number":227,"context_line":"                # there was a problem communicating with the placement API,"},{"line_number":228,"context_line":"                # alloc_reqs_by_rp_uuid will be None, so we skip claiming in"},{"line_number":229,"context_line":"                # that case as well"},{"line_number":230,"context_line":"                claimed_host \u003d hosts[0]"},{"line_number":231,"context_line":"            else:"},{"line_number":232,"context_line":"                instance_uuid \u003d instance_uuids[num]"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"                # Attempt to claim the resources against one or more resource"}],"source_content_type":"text/x-python","patch_set":10,"id":"7f515b1d_d0f3a6e6","line":231,"range":{"start_line":216,"start_character":12,"end_line":231,"end_character":17},"in_reply_to":"7f515b1d_9c4aefa9","updated":"2017-09-25 22:36:43.000000000","message":"Ugh - rebase error.","commit_id":"626f45e072b47de2627a47ead12a8c33ee452309"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"6c046631b077a0f1eed464361168fec35e29d9c1","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            # host."},{"line_number":95,"context_line":"            for host_list in selected_host_lists:"},{"line_number":96,"context_line":"                # Do we need to update the alternates too? Or just the chosen"},{"line_number":97,"context_line":"                # (first) host?"},{"line_number":98,"context_line":"                host_list[0].updated \u003d None"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":12,"id":"7f515b1d_889a175f","line":97,"updated":"2017-10-01 19:01:40.000000000","message":"This comment still feels unfinished. Is it really a TODO?","commit_id":"542a369fbfbcfe80610866f01a59e0a9077aba45"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"f15b5e49624303f23868b544613846293a6d80fd","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        \"\"\"Returns a list of sorted lists of HostState objects (1 for each"},{"line_number":49,"context_line":"        instance) that would satisfy the supplied request_spec. Each of those"},{"line_number":50,"context_line":"        lists consist of [chosen_host, alternate1, ..., alternateN], where the"},{"line_number":51,"context_line":"        \u0027chosen_host\u0027 has already had its resources claimed in Placement,"},{"line_number":52,"context_line":"        followed by zero or more alternates. The alternates are hosts that can"},{"line_number":53,"context_line":"        satisfy the request, and are included so that if the build for the"},{"line_number":54,"context_line":"        chosen host fails, the cell conductor can retry."}],"source_content_type":"text/x-python","patch_set":19,"id":"5f4e5783_7496f195","line":51,"range":{"start_line":51,"start_character":52,"end_line":51,"end_character":59},"updated":"2017-10-13 18:36:51.000000000","message":"I wonder if we should start using \"allocated\" for this meaning?","commit_id":"e10d0b85d90d4ae3ebb95a83c33f859456231f32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"6e717f2078bb85b880f2690a6b2285341c1b87fe","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        \"\"\"Returns a list of sorted lists of HostState objects (1 for each"},{"line_number":49,"context_line":"        instance) that would satisfy the supplied request_spec. Each of those"},{"line_number":50,"context_line":"        lists consist of [chosen_host, alternate1, ..., alternateN], where the"},{"line_number":51,"context_line":"        \u0027chosen_host\u0027 has already had its resources claimed in Placement,"},{"line_number":52,"context_line":"        followed by zero or more alternates. The alternates are hosts that can"},{"line_number":53,"context_line":"        satisfy the request, and are included so that if the build for the"},{"line_number":54,"context_line":"        chosen host fails, the cell conductor can retry."}],"source_content_type":"text/x-python","patch_set":19,"id":"5f4e5783_ca52753b","line":51,"range":{"start_line":51,"start_character":52,"end_line":51,"end_character":59},"in_reply_to":"5f4e5783_7496f195","updated":"2017-10-16 13:19:09.000000000","message":"In the context of a new VM about to be built on a host, \"claimed\" is useful, as we want this to replace the current mess in the RT. Yes, \u0027allocated\u0027 is also correct, but I think that in this context, \u0027claimed\u0027 is more indicative of what\u0027s going on.","commit_id":"e10d0b85d90d4ae3ebb95a83c33f859456231f32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            # the resource consumed by instance in the process of selecting"},{"line_number":99,"context_line":"            # host."},{"line_number":100,"context_line":"            for host_list in selected_host_lists:"},{"line_number":101,"context_line":"                # Do we need to update the alternates too? Or just the chosen"},{"line_number":102,"context_line":"                # (first) host?"},{"line_number":103,"context_line":"                host_list[0].updated \u003d None"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_e7bef163","line":102,"range":{"start_line":101,"start_character":16,"end_line":102,"end_character":31},"updated":"2017-10-20 14:49:37.000000000","message":"I would assume just the chosen one. The Chosen One.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":118,"context_line":"            dict(request_spec\u003dspec_obj.to_legacy_request_spec_dict()))"},{"line_number":119,"context_line":"        # NOTE(edleafe) - In this patch we only create the lists of [chosen,"},{"line_number":120,"context_line":"        # alt1, alt2, etc.]. In a later patch we will change what we return, so"},{"line_number":121,"context_line":"        # for this patch just return the selected hosts."},{"line_number":122,"context_line":"        selected_hosts \u003d [sel_host[0] for sel_host in selected_host_lists]"},{"line_number":123,"context_line":"        return selected_hosts"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_420ed332","line":121,"range":{"start_line":121,"start_character":41,"end_line":121,"end_character":49},"updated":"2017-10-20 14:49:37.000000000","message":"nit: chosen - but this comment goes away soon so nvm","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        :param spec_obj: The RequestSpec object"},{"line_number":134,"context_line":"        :param instance_uuids: List of instance UUIDs to place or move."},{"line_number":135,"context_line":"        :param alloc_reqs_by_rp_uuid: Optional dict, keyed by resource provider"},{"line_number":136,"context_line":"                                      UUID, of the allocation_requests that may"},{"line_number":137,"context_line":"                                      be used to claim resources against"},{"line_number":138,"context_line":"                                      matched hosts. If None, indicates either"},{"line_number":139,"context_line":"                                      the placement API wasn\u0027t reachable or"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_0218db72","line":136,"range":{"start_line":136,"start_character":51,"end_line":136,"end_character":70},"updated":"2017-10-20 14:49:37.000000000","message":"Not really a necessary change in this patch. It just clutters up the review.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        :param spec_obj: The RequestSpec object"},{"line_number":134,"context_line":"        :param instance_uuids: List of instance UUIDs to place or move."},{"line_number":135,"context_line":"        :param alloc_reqs_by_rp_uuid: Optional dict, keyed by resource provider"},{"line_number":136,"context_line":"                                      UUID, of the allocation_requests that may"},{"line_number":137,"context_line":"                                      be used to claim resources against"},{"line_number":138,"context_line":"                                      matched hosts. If None, indicates either"},{"line_number":139,"context_line":"                                      the placement API wasn\u0027t reachable or"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_c76d2e99","line":136,"range":{"start_line":136,"start_character":51,"end_line":136,"end_character":70},"in_reply_to":"3f4b6375_0218db72","updated":"2017-10-20 19:03:21.000000000","message":"I had gotten criticism that I should use the proper name of the class, so that\u0027s why this change was made. I\u0027ll break them out into a separate patch.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            # driver. Also, if there was a problem communicating with the"},{"line_number":193,"context_line":"            # placement API, alloc_reqs_by_rp_uuid will be None, so we skip"},{"line_number":194,"context_line":"            # claiming in that case as well. In the case where instance_uuids"},{"line_number":195,"context_line":"            # is None, that indicates and older conductor, so we need to return"},{"line_number":196,"context_line":"            # the older-style HostState objects without alternates."},{"line_number":197,"context_line":"            # NOTE(edleafe): moving this logic into a separate method, as this"},{"line_number":198,"context_line":"            # method is already way too long. It will also make it easier to"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_a2a58706","line":195,"range":{"start_line":195,"start_character":38,"end_line":195,"end_character":41},"updated":"2017-10-20 14:49:37.000000000","message":"an","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            # driver. Also, if there was a problem communicating with the"},{"line_number":193,"context_line":"            # placement API, alloc_reqs_by_rp_uuid will be None, so we skip"},{"line_number":194,"context_line":"            # claiming in that case as well. In the case where instance_uuids"},{"line_number":195,"context_line":"            # is None, that indicates and older conductor, so we need to return"},{"line_number":196,"context_line":"            # the older-style HostState objects without alternates."},{"line_number":197,"context_line":"            # NOTE(edleafe): moving this logic into a separate method, as this"},{"line_number":198,"context_line":"            # method is already way too long. It will also make it easier to"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_e7f38ae6","line":195,"range":{"start_line":195,"start_character":38,"end_line":195,"end_character":41},"in_reply_to":"3f4b6375_a2a58706","updated":"2017-10-20 19:03:21.000000000","message":"Done","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            # placement API, alloc_reqs_by_rp_uuid will be None, so we skip"},{"line_number":194,"context_line":"            # claiming in that case as well. In the case where instance_uuids"},{"line_number":195,"context_line":"            # is None, that indicates and older conductor, so we need to return"},{"line_number":196,"context_line":"            # the older-style HostState objects without alternates."},{"line_number":197,"context_line":"            # NOTE(edleafe): moving this logic into a separate method, as this"},{"line_number":198,"context_line":"            # method is already way too long. It will also make it easier to"},{"line_number":199,"context_line":"            # clean up once we no longer have to worry about older conductors."}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_6298afbf","line":196,"range":{"start_line":196,"start_character":18,"end_line":196,"end_character":67},"updated":"2017-10-20 14:49:37.000000000","message":"Later in the series, conductor is going to be explicitly requesting alternates, right? Which is going to be a parameter on the select_destinations rpcapi method, so I\u0027d think that gets plumbed down here so we don\u0027t have to make an assumption based on instance_uuids being provided or not.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            # placement API, alloc_reqs_by_rp_uuid will be None, so we skip"},{"line_number":194,"context_line":"            # claiming in that case as well. In the case where instance_uuids"},{"line_number":195,"context_line":"            # is None, that indicates and older conductor, so we need to return"},{"line_number":196,"context_line":"            # the older-style HostState objects without alternates."},{"line_number":197,"context_line":"            # NOTE(edleafe): moving this logic into a separate method, as this"},{"line_number":198,"context_line":"            # method is already way too long. It will also make it easier to"},{"line_number":199,"context_line":"            # clean up once we no longer have to worry about older conductors."}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_e7ca6a6c","line":196,"range":{"start_line":196,"start_character":18,"end_line":196,"end_character":67},"in_reply_to":"3f4b6375_6298afbf","updated":"2017-10-20 19:03:21.000000000","message":"If an older conductor calls select_destinations(), it will not pass instance_uuids, so we need to catch that case as well as the not USES_ALLOCATION_CANDIDATES case and when we fail to contact placement for the alloc_reqs_by_rp_uuid.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"214ea5c998dfbc4e08ef6ffa7672a7c3cd5c6726","unresolved":false,"context_lines":[{"line_number":193,"context_line":"            # placement API, alloc_reqs_by_rp_uuid will be None, so we skip"},{"line_number":194,"context_line":"            # claiming in that case as well. In the case where instance_uuids"},{"line_number":195,"context_line":"            # is None, that indicates and older conductor, so we need to return"},{"line_number":196,"context_line":"            # the older-style HostState objects without alternates."},{"line_number":197,"context_line":"            # NOTE(edleafe): moving this logic into a separate method, as this"},{"line_number":198,"context_line":"            # method is already way too long. It will also make it easier to"},{"line_number":199,"context_line":"            # clean up once we no longer have to worry about older conductors."}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_a8d2fe5a","line":196,"range":{"start_line":196,"start_character":18,"end_line":196,"end_character":67},"in_reply_to":"3f4b6375_e7ca6a6c","updated":"2017-10-21 00:31:33.000000000","message":"OK","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":266,"context_line":"        # find alternates for each host."},{"line_number":267,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":268,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":269,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":270,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_42237306","line":267,"range":{"start_line":267,"start_character":8,"end_line":267,"end_character":60},"updated":"2017-10-20 14:49:37.000000000","message":"Why do we have to go through filtering and weighing again?\n\nCan\u0027t we just exclude any hosts in the claimed_host list from our list of alternates?\n\nI worry about introducing a performance regression here by running through all of the filters and weighers a second time when we already have the list of filtered/weighed hosts.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":266,"context_line":"        # find alternates for each host."},{"line_number":267,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":268,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":269,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":270,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_678d5a30","line":267,"range":{"start_line":267,"start_character":8,"end_line":267,"end_character":60},"in_reply_to":"3f4b6375_42237306","updated":"2017-10-20 19:03:21.000000000","message":"For the same reason that we call this after every instance: because the consume_from_request() call above changes how suitable a host is. E.g., the last instance selected could have consumed enough RAM that the host selected for it is no longer a good candidate as an alternate. Alternates are useless if we know ahead of time that they will not be able to fit an instance on them.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"214ea5c998dfbc4e08ef6ffa7672a7c3cd5c6726","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":266,"context_line":"        # find alternates for each host."},{"line_number":267,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":268,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":269,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":270,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_88d50246","line":267,"range":{"start_line":267,"start_character":8,"end_line":267,"end_character":60},"in_reply_to":"3f4b6375_678d5a30","updated":"2017-10-21 00:31:33.000000000","message":"Yeah, good point, and I didn\u0027t think about how we call this during each instance in the for loop. My original question included something like, \"I suppose because of the consume_from_request thing on a chosen host\" but didn\u0027t connect the dots with the loop.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            for host in hosts:"},{"line_number":282,"context_line":"                if len(claimed_plus_alts) \u003e\u003d num_to_return:"},{"line_number":283,"context_line":"                    break"},{"line_number":284,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in claimed_hosts:"},{"line_number":285,"context_line":"                    claimed_plus_alts.append(host)"},{"line_number":286,"context_line":"            selections_to_return.append(claimed_plus_alts)"},{"line_number":287,"context_line":"        return selections_to_return"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_02721b12","line":284,"range":{"start_line":284,"start_character":63,"end_line":284,"end_character":76},"updated":"2017-10-20 14:49:37.000000000","message":"We should have a comment explaining why this isn\u0027t claimed_plus_alts - same as mentioned below for the legacy code. The point being we don\u0027t want primary chosen hosts to be alternates because rescheduling on something we know was chosen for an instance build means it\u0027s going to have less capacity if it\u0027s used as a reschedule target later.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":281,"context_line":"            for host in hosts:"},{"line_number":282,"context_line":"                if len(claimed_plus_alts) \u003e\u003d num_to_return:"},{"line_number":283,"context_line":"                    break"},{"line_number":284,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in claimed_hosts:"},{"line_number":285,"context_line":"                    claimed_plus_alts.append(host)"},{"line_number":286,"context_line":"            selections_to_return.append(claimed_plus_alts)"},{"line_number":287,"context_line":"        return selections_to_return"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_e7c6ea49","line":284,"range":{"start_line":284,"start_character":63,"end_line":284,"end_character":76},"in_reply_to":"3f4b6375_02721b12","updated":"2017-10-20 19:03:21.000000000","message":"Added an explanation to the comments at the top of the loop.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":338,"context_line":"            alloc_req, project_id, user_id)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def _legacy_find_hosts(self, num_instances, spec_obj, hosts,"},{"line_number":341,"context_line":"            num_to_return, return_old):"},{"line_number":342,"context_line":"        \"\"\"Some schedulers do not do claiming, or we can sometimes not be able"},{"line_number":343,"context_line":"        to if the Placement service is not reachable. Additionally, we may be"},{"line_number":344,"context_line":"        working with older conductors that don\u0027t pass in instance_uuids."}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_225517c7","line":341,"range":{"start_line":341,"start_character":27,"end_line":341,"end_character":37},"updated":"2017-10-20 14:49:37.000000000","message":"Seems we could use a better variable name for this, maybe something like return_alternates? Because \"old\" doesn\u0027t have much meaning here.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":338,"context_line":"            alloc_req, project_id, user_id)"},{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    def _legacy_find_hosts(self, num_instances, spec_obj, hosts,"},{"line_number":341,"context_line":"            num_to_return, return_old):"},{"line_number":342,"context_line":"        \"\"\"Some schedulers do not do claiming, or we can sometimes not be able"},{"line_number":343,"context_line":"        to if the Placement service is not reachable. Additionally, we may be"},{"line_number":344,"context_line":"        working with older conductors that don\u0027t pass in instance_uuids."}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_872bb6e9","line":341,"range":{"start_line":341,"start_character":27,"end_line":341,"end_character":37},"in_reply_to":"3f4b6375_225517c7","updated":"2017-10-20 19:03:21.000000000","message":"ok, changed it to \u0027include_alternates\u0027, and reversed the logic.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":347,"context_line":"        selected_hosts \u003d []"},{"line_number":348,"context_line":"        # This the overall list of values to be returned. There will be one"},{"line_number":349,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":350,"context_line":"        # representing the selected host along with alternates from the same"},{"line_number":351,"context_line":"        # cell."},{"line_number":352,"context_line":"        selections_to_return \u003d []"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_c23ac396","line":350,"range":{"start_line":350,"start_character":47,"end_line":350,"end_character":62},"updated":"2017-10-20 14:49:37.000000000","message":"On L196 you said we wouldn\u0027t return alternates for the legacy case. Maybe need to adjust the comment here a bit to mention the return_old variable since that determines if alternates are returned.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":347,"context_line":"        selected_hosts \u003d []"},{"line_number":348,"context_line":"        # This the overall list of values to be returned. There will be one"},{"line_number":349,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":350,"context_line":"        # representing the selected host along with alternates from the same"},{"line_number":351,"context_line":"        # cell."},{"line_number":352,"context_line":"        selections_to_return \u003d []"},{"line_number":353,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_4735be45","line":350,"range":{"start_line":350,"start_character":47,"end_line":350,"end_character":62},"in_reply_to":"3f4b6375_c23ac396","updated":"2017-10-20 19:03:21.000000000","message":"Done","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":356,"context_line":"            if not hosts:"},{"line_number":357,"context_line":"                return []"},{"line_number":358,"context_line":"            selected_host \u003d hosts[0]"},{"line_number":359,"context_line":"            selected_host.consume_from_request(spec_obj)"},{"line_number":360,"context_line":"            selected_hosts.append(selected_host)"},{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_82128b08","line":359,"updated":"2017-10-20 14:49:37.000000000","message":"Duplicate the debug message from above?\n\nLOG.debug(\"Selected host: %(host)s\", {\u0027host\u0027: selected_host})","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                return []"},{"line_number":358,"context_line":"            selected_host \u003d hosts[0]"},{"line_number":359,"context_line":"            selected_host.consume_from_request(spec_obj)"},{"line_number":360,"context_line":"            selected_hosts.append(selected_host)"},{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"},{"line_number":363,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_221cf710","line":360,"updated":"2017-10-20 14:49:37.000000000","message":"You\u0027re missing all of the instance group affinity stuff:\n\nif spec_obj.instance_group is not None:\n    spec_obj.instance_group.hosts.append(claimed_host.host)\n    # hosts has to be not part of the updates when saving\n    spec_obj.instance_group.obj_reset_changes([\u0027hosts\u0027])","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                return []"},{"line_number":358,"context_line":"            selected_host \u003d hosts[0]"},{"line_number":359,"context_line":"            selected_host.consume_from_request(spec_obj)"},{"line_number":360,"context_line":"            selected_hosts.append(selected_host)"},{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"},{"line_number":363,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_47d13ecd","line":360,"in_reply_to":"3f4b6375_221cf710","updated":"2017-10-20 19:03:21.000000000","message":"OK, I moved all that stuff into a separate staticmethod, and call it from both places.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"},{"line_number":363,"context_line":"                continue"},{"line_number":364,"context_line":"            cell_uuid \u003d selected_host.uuid"},{"line_number":365,"context_line":"            host_and_alts \u003d [selected_host]"},{"line_number":366,"context_line":"            for host in hosts:"},{"line_number":367,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_c2bf63ce","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":42},"updated":"2017-10-20 14:49:37.000000000","message":"This isn\u0027t the cell uuid, this is the compute node uuid:\n\nhttps://github.com/openstack/nova/blob/8d21d711000fff80eb367692b157d09b6532923f/nova/scheduler/host_manager.py#L157\n\nhttps://github.com/openstack/nova/blob/8d21d711000fff80eb367692b157d09b6532923f/nova/scheduler/host_manager.py#L200","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"214ea5c998dfbc4e08ef6ffa7672a7c3cd5c6726","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"},{"line_number":363,"context_line":"                continue"},{"line_number":364,"context_line":"            cell_uuid \u003d selected_host.uuid"},{"line_number":365,"context_line":"            host_and_alts \u003d [selected_host]"},{"line_number":366,"context_line":"            for host in hosts:"},{"line_number":367,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_a8379eb0","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":42},"in_reply_to":"3f4b6375_87f05669","updated":"2017-10-21 00:31:33.000000000","message":"Why didn\u0027t unit tests fail?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":361,"context_line":"            if return_old:"},{"line_number":362,"context_line":"                # Skip the alternates"},{"line_number":363,"context_line":"                continue"},{"line_number":364,"context_line":"            cell_uuid \u003d selected_host.uuid"},{"line_number":365,"context_line":"            host_and_alts \u003d [selected_host]"},{"line_number":366,"context_line":"            for host in hosts:"},{"line_number":367,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_87f05669","line":364,"range":{"start_line":364,"start_character":24,"end_line":364,"end_character":42},"in_reply_to":"3f4b6375_c2bf63ce","updated":"2017-10-20 19:03:21.000000000","message":"Done","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9a5b6b5c930174013506d69c5fda34d2a47ff4ae","unresolved":false,"context_lines":[{"line_number":366,"context_line":"            for host in hosts:"},{"line_number":367,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"},{"line_number":368,"context_line":"                    break"},{"line_number":369,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in selected_hosts:"},{"line_number":370,"context_line":"                    host_and_alts.append(host)"},{"line_number":371,"context_line":"            selections_to_return.append(host_and_alts)"},{"line_number":372,"context_line":"        if return_old:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_429ed354","line":369,"range":{"start_line":369,"start_character":63,"end_line":369,"end_character":77},"updated":"2017-10-20 14:49:37.000000000","message":"I was thinking this should be host_and_alts but I guess the point is chosen (primary) hosts should never be alternates, because by the time we reschedule and try that alternate, if it was a primary there is a good chance it might not be free anymore. We should have a comment in here explaining that logic.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":366,"context_line":"            for host in hosts:"},{"line_number":367,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"},{"line_number":368,"context_line":"                    break"},{"line_number":369,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in selected_hosts:"},{"line_number":370,"context_line":"                    host_and_alts.append(host)"},{"line_number":371,"context_line":"            selections_to_return.append(host_and_alts)"},{"line_number":372,"context_line":"        if return_old:"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_c7fd2e3b","line":369,"range":{"start_line":369,"start_character":63,"end_line":369,"end_character":77},"in_reply_to":"3f4b6375_429ed354","updated":"2017-10-20 19:03:21.000000000","message":"Done","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c65edbdd16dbae268b34a30d7da39822fc4bfbec","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        claimed_hosts \u003d []"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        for num in range(num_instances):"},{"line_number":212,"context_line":"            instance_uuid \u003d instance_uuids[num]"},{"line_number":213,"context_line":"            hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":214,"context_line":"            if not hosts:"},{"line_number":215,"context_line":"                # NOTE(jaypipes): If we get here, that means not all instances"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_fb0d5e35","line":212,"range":{"start_line":212,"start_character":12,"end_line":212,"end_character":47},"updated":"2017-10-21 02:50:47.000000000","message":"This isn\u0027t used.","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fa97422835d15bdd68fec52581eb1659f2b8e6b1","unresolved":false,"context_lines":[{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        # We have selected and claimed hosts for each instance. Now we need to"},{"line_number":259,"context_line":"        # find alternates for each host."},{"line_number":260,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":261,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":262,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":263,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_7becce28","line":260,"range":{"start_line":260,"start_character":56,"end_line":260,"end_character":59},"updated":"2017-10-21 03:17:15.000000000","message":"Do we need to increment this index value? Looks like it doesn\u0027t matter with how run_filter_once_per_request works.","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76c73b8a071aacab33c68c124f154f74f5cffacd","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        selections_to_return \u003d []"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        for num in range(num_instances):"},{"line_number":353,"context_line":"            hosts \u003d self._get_sorted_hosts(spec_obj, hosts, num)"},{"line_number":354,"context_line":"            if not hosts:"},{"line_number":355,"context_line":"                return []"},{"line_number":356,"context_line":"            selected_host \u003d hosts[0]"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_1b983abe","line":353,"range":{"start_line":353,"start_character":25,"end_line":353,"end_character":42},"updated":"2017-10-21 03:06:47.000000000","message":"In the non-legacy case, we re-sort the hosts in each iteration of the instance for loop, and then once again separately before we start picking alternates from the list of selected hosts - why aren\u0027t we doing that here?","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e408c2dfd9d336a08935661477a929e5d2190eb1","unresolved":false,"context_lines":[{"line_number":359,"context_line":"            if not include_alternates:"},{"line_number":360,"context_line":"                # Skip the alternates"},{"line_number":361,"context_line":"                continue"},{"line_number":362,"context_line":"            cell_uuid \u003d selected_host.cell_uuid"},{"line_number":363,"context_line":"            host_and_alts \u003d [selected_host]"},{"line_number":364,"context_line":"            for host in hosts:"},{"line_number":365,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_48ea6aac","line":362,"range":{"start_line":362,"start_character":24,"end_line":362,"end_character":47},"updated":"2017-10-21 01:07:45.000000000","message":"So this wasn\u0027t caught because we don\u0027t have a test for the legacy path with multiple cells - we should add one in test_caching_scheduler, that\u0027s probably easiest. I tried copying one of the filter scheduler tests and move it over there but hit some weird issues.","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f5539bede2f4078e5213bf610a385ee7a26ab40c","unresolved":false,"context_lines":[{"line_number":365,"context_line":"                if len(host_and_alts) \u003e\u003d num_to_return:"},{"line_number":366,"context_line":"                    break"},{"line_number":367,"context_line":"                # Note that we exclude any host already selected from"},{"line_number":368,"context_line":"                # considertation as an alternate because it will have had its"},{"line_number":369,"context_line":"                # resources reduced and will have a much lower chance of being"},{"line_number":370,"context_line":"                # able to fit another instance on it."},{"line_number":371,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in selected_hosts:"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_08f472ce","line":368,"range":{"start_line":368,"start_character":18,"end_line":368,"end_character":32},"updated":"2017-10-21 00:39:35.000000000","message":"consideration","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"76c73b8a071aacab33c68c124f154f74f5cffacd","unresolved":false,"context_lines":[{"line_number":368,"context_line":"                # considertation as an alternate because it will have had its"},{"line_number":369,"context_line":"                # resources reduced and will have a much lower chance of being"},{"line_number":370,"context_line":"                # able to fit another instance on it."},{"line_number":371,"context_line":"                if host.cell_uuid \u003d\u003d cell_uuid and host not in selected_hosts:"},{"line_number":372,"context_line":"                    host_and_alts.append(host)"},{"line_number":373,"context_line":"            selections_to_return.append(host_and_alts)"},{"line_number":374,"context_line":"        if include_alternates:"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_fb92fe9e","line":371,"range":{"start_line":371,"start_character":47,"end_line":371,"end_character":77},"updated":"2017-10-21 03:06:47.000000000","message":"Since we\u0027re not picking alternates after picking primary selected hosts for each instance, couldn\u0027t we pick an alternate for an instance early in the loop and then that alternate host is actually a chosen selected host for an instance later in the loop?","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f508df23498d856687c62e442d9bf0a54b4e361e","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        # The selected_hosts have all had resources \u0027claimed\u0027 via"},{"line_number":356,"context_line":"        # _consume_selected_host, so we need to filter/weigh and sort the"},{"line_number":357,"context_line":"        # hosts again to get an accurate count for alternates."},{"line_number":358,"context_line":"        hosts \u003d self._get_sorted_hosts(spec_obj, hosts, index)"},{"line_number":359,"context_line":"        # This is the overall list of values to be returned. There will be one"},{"line_number":360,"context_line":"        # item per instance, and that item will be a list of HostState objects"},{"line_number":361,"context_line":"        # representing the selected host along with alternates from the same"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f4b6375_d9e4eaae","line":358,"updated":"2017-10-21 13:12:06.000000000","message":"Something for a follow up that I was thinking about, but if we\u0027re just scheduling for a single instance, we shouldn\u0027t need to re-filter/weigh/sort the hosts, because we\u0027re going to filter out the one selected host from the alternates anyway.\n\nWe can easily bypass the unnecessary _get_sorted_hosts call by checking if index \u003d\u003d 0.\n\nI just want to be conscious of any new performance impact this is going to have since we already get dinged about the scheduler taking too long.","commit_id":"70e858d6a1db782d4a7e6d30edd0ec2c98a73a62"}],"nova/scheduler/utils.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0828a4a9f6b696310cd493a3f0c1be3f8b38960d","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        limits \u003d host_state.limits"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    # Adds a retry entry for the selected compute host and node:"},{"line_number":286,"context_line":"    _add_retry_host(filter_properties, host, nodename)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    # Adds oversubscription policy"},{"line_number":289,"context_line":"    if not filter_properties.get(\u0027force_hosts\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_65ec21f3","line":286,"updated":"2017-08-25 10:29:49.000000000","message":"Eventually we\u0027ll be getting rid of this, yes?","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7a9f834dd233ac663624fac71fceaae1d85c16a0","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        limits \u003d host_state.limits"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    # Adds a retry entry for the selected compute host and node:"},{"line_number":286,"context_line":"    _add_retry_host(filter_properties, host, nodename)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    # Adds oversubscription policy"},{"line_number":289,"context_line":"    if not filter_properties.get(\u0027force_hosts\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7f287b81_691cd077","line":286,"in_reply_to":"7f287b81_65ec21f3","updated":"2017-08-28 13:18:09.000000000","message":"After we implement retries in the cell conductor, yes.","commit_id":"2fcc49a36c11969f79ef95ace25a6a575762a666"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6aae68e088c9944b50d10fe40d3927628721d013","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, dict):"},{"line_number":354,"context_line":"        host \u003d host_state[\u0027host\u0027]"},{"line_number":355,"context_line":"        nodename \u003d host_state[\u0027nodename\u0027]"},{"line_number":356,"context_line":"        limits \u003d host_state[\u0027limits\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_363d8fbb","line":353,"updated":"2017-09-21 15:43:49.000000000","message":"We can remove the dict block now, right? There are no longer any schedulers calling this method and supplying a host_state param that is a dict.","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9cf500f89b5bc0436a8da2c763035360e120a80c","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, dict):"},{"line_number":354,"context_line":"        host \u003d host_state[\u0027host\u0027]"},{"line_number":355,"context_line":"        nodename \u003d host_state[\u0027nodename\u0027]"},{"line_number":356,"context_line":"        limits \u003d host_state[\u0027limits\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_0d3719b3","line":353,"in_reply_to":"7f515b1d_ad5b652c","updated":"2017-09-22 17:28:38.000000000","message":"Bingo :) Glad removing this identified those tests!","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"259c5d7d52814bfbb00d585b0b066576913ee220","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, dict):"},{"line_number":354,"context_line":"        host \u003d host_state[\u0027host\u0027]"},{"line_number":355,"context_line":"        nodename \u003d host_state[\u0027nodename\u0027]"},{"line_number":356,"context_line":"        limits \u003d host_state[\u0027limits\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_ad5b652c","line":353,"in_reply_to":"7f515b1d_b5918af4","updated":"2017-09-22 17:23:25.000000000","message":"Ugh, turns out that some tests still pass host dicts.","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, dict):"},{"line_number":354,"context_line":"        host \u003d host_state[\u0027host\u0027]"},{"line_number":355,"context_line":"        nodename \u003d host_state[\u0027nodename\u0027]"},{"line_number":356,"context_line":"        limits \u003d host_state[\u0027limits\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_b5918af4","line":353,"in_reply_to":"9f5c4f37_363d8fbb","updated":"2017-09-21 22:17:35.000000000","message":"Done","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, list):"},{"line_number":354,"context_line":"        host_state \u003d host_state[0]"},{"line_number":355,"context_line":"    if isinstance(host_state, dict):"},{"line_number":356,"context_line":"        # TODO(edleafe): remove support for dicts"},{"line_number":357,"context_line":"        host \u003d host_state[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_6ced488a","line":354,"range":{"start_line":353,"start_character":4,"end_line":354,"end_character":34},"updated":"2017-10-20 13:44:52.000000000","message":"Add a comment about when/why this can be a list?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c0b5c912dad37c9a2f91bb1c7c761e24926e6ce","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, list):"},{"line_number":354,"context_line":"        host_state \u003d host_state[0]"},{"line_number":355,"context_line":"    if isinstance(host_state, dict):"},{"line_number":356,"context_line":"        # TODO(edleafe): remove support for dicts"},{"line_number":357,"context_line":"        host \u003d host_state[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_a2b8e775","line":354,"range":{"start_line":353,"start_character":4,"end_line":354,"end_character":34},"in_reply_to":"3f4b6375_6ced488a","updated":"2017-10-20 14:50:58.000000000","message":"Why do we even need this in this patch?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":350,"context_line":"    \"\"\"Add additional information to the filter properties after a node has"},{"line_number":351,"context_line":"    been selected by the scheduling process."},{"line_number":352,"context_line":"    \"\"\""},{"line_number":353,"context_line":"    if isinstance(host_state, list):"},{"line_number":354,"context_line":"        host_state \u003d host_state[0]"},{"line_number":355,"context_line":"    if isinstance(host_state, dict):"},{"line_number":356,"context_line":"        # TODO(edleafe): remove support for dicts"},{"line_number":357,"context_line":"        host \u003d host_state[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_242be030","line":354,"range":{"start_line":353,"start_character":4,"end_line":354,"end_character":34},"in_reply_to":"3f4b6375_a2b8e775","updated":"2017-10-20 19:03:21.000000000","message":"Left over from earlier versions of the patch, where the scheduler returned the alternates. Removed.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"}],"nova/tests/unit/scheduler/test_chance_scheduler.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6aae68e088c9944b50d10fe40d3927628721d013","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    def test_schedule_success_single(self, mock_hosts_up):"},{"line_number":82,"context_line":"        hosts \u003d [\"host%s\" % i for i in range(20)]"},{"line_number":83,"context_line":"        mock_hosts_up.return_value \u003d hosts"},{"line_number":84,"context_line":"        spec_obj \u003d objects.RequestSpec(num_instances\u003d1, ignore_hosts\u003dNone)"},{"line_number":85,"context_line":"        spec_obj.instance_uuid \u003d uuids.instance"},{"line_number":86,"context_line":"        # Set the max_attempts to 2"},{"line_number":87,"context_line":"        attempts \u003d 2"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_96f2fb05","line":84,"range":{"start_line":84,"start_character":54,"end_line":84,"end_character":73},"updated":"2017-09-21 15:43:49.000000000","message":"here and below, is there a reason to pass ignore_hosts\u003dNone? Just copy/paste or is there a valid purpose to passing it?","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"0894a06c2a6e5e84ac9c9ebed08013dfb0f44605","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    def test_schedule_success_single(self, mock_hosts_up):"},{"line_number":82,"context_line":"        hosts \u003d [\"host%s\" % i for i in range(20)]"},{"line_number":83,"context_line":"        mock_hosts_up.return_value \u003d hosts"},{"line_number":84,"context_line":"        spec_obj \u003d objects.RequestSpec(num_instances\u003d1, ignore_hosts\u003dNone)"},{"line_number":85,"context_line":"        spec_obj.instance_uuid \u003d uuids.instance"},{"line_number":86,"context_line":"        # Set the max_attempts to 2"},{"line_number":87,"context_line":"        attempts \u003d 2"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f515b1d_80d1aa5a","line":84,"range":{"start_line":84,"start_character":54,"end_line":84,"end_character":73},"in_reply_to":"9f5c4f37_96f2fb05","updated":"2017-09-21 22:17:35.000000000","message":"Just copy/paste.","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6aae68e088c9944b50d10fe40d3927628721d013","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                instance_uuids)"},{"line_number":123,"context_line":"        self.assertEqual(num_instances, len(result))"},{"line_number":124,"context_line":"        for host_list in result:"},{"line_number":125,"context_line":"            self.assertEqual(attempts, len(host_list))"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f5c4f37_d6d19359","line":125,"updated":"2017-09-21 15:43:49.000000000","message":"++ good tests, and thanks for de-moxing :)","commit_id":"60e3b20eefe39debc40e06c1615dffb1316c7d5e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                          mock.sentinel.provider_summaries)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    @mock.patch(\"nova.scheduler.chance.ChanceScheduler.hosts_up\")"},{"line_number":81,"context_line":"    def test_schedule_success_single(self, mock_hosts_up):"},{"line_number":82,"context_line":"        hosts \u003d [\"host%s\" % i for i in range(20)]"},{"line_number":83,"context_line":"        mock_hosts_up.return_value \u003d hosts"},{"line_number":84,"context_line":"        spec_obj \u003d objects.RequestSpec(num_instances\u003d1, ignore_hosts\u003dNone)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_6cbb0872","line":81,"range":{"start_line":81,"start_character":30,"end_line":81,"end_character":36},"updated":"2017-10-20 13:44:52.000000000","message":"single_instance?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                [spec_obj.instance_uuid])"},{"line_number":92,"context_line":"        self.assertEqual(1, len(result))"},{"line_number":93,"context_line":"        for host_list in result:"},{"line_number":94,"context_line":"            self.assertEqual(expected, len(host_list))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # Now set max_attempts to a number larger than the available hosts. It"},{"line_number":97,"context_line":"        # should return a host_list containing only as many hosts as there are"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_0c2ff4a1","line":94,"range":{"start_line":94,"start_character":12,"end_line":94,"end_character":54},"updated":"2017-10-20 13:44:52.000000000","message":"Do we assert anywhere in the tests that the alternates aren\u0027t dupes?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5bf10507ed4bb9b4eca1c941bc3b37322f98d32a","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                [spec_obj.instance_uuid])"},{"line_number":92,"context_line":"        self.assertEqual(1, len(result))"},{"line_number":93,"context_line":"        for host_list in result:"},{"line_number":94,"context_line":"            self.assertEqual(expected, len(host_list))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        # Now set max_attempts to a number larger than the available hosts. It"},{"line_number":97,"context_line":"        # should return a host_list containing only as many hosts as there are"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_072586c2","line":94,"range":{"start_line":94,"start_character":12,"end_line":94,"end_character":54},"in_reply_to":"3f4b6375_0c2ff4a1","updated":"2017-10-20 19:03:21.000000000","message":"Guess not. I added that test to the multiple instance test below.","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"73a92182618813c4ae0414cd0503469716635554","unresolved":false,"context_lines":[{"line_number":106,"context_line":"            self.assertEqual(expected, len(host_list))"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    @mock.patch(\"nova.scheduler.chance.ChanceScheduler.hosts_up\")"},{"line_number":109,"context_line":"    def test_schedule_success_multiple(self, mock_hosts_up):"},{"line_number":110,"context_line":"        hosts \u003d [\"host%s\" % i for i in range(20)]"},{"line_number":111,"context_line":"        mock_hosts_up.return_value \u003d hosts"},{"line_number":112,"context_line":"        num_instances \u003d 4"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f4b6375_eca6d853","line":109,"range":{"start_line":109,"start_character":30,"end_line":109,"end_character":38},"updated":"2017-10-20 13:44:52.000000000","message":"multiple_instances?","commit_id":"7a9db7a621afd1e42ecd63ab557d4e40005d9c32"}],"nova/tests/unit/scheduler/test_filter_scheduler.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a90efbe345fde627eedd980bc14c651c227b412","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                \u0027_get_all_host_states\u0027)"},{"line_number":99,"context_line":"    @mock.patch(\u0027nova.scheduler.filter_scheduler.FilterScheduler.\u0027"},{"line_number":100,"context_line":"                \u0027_get_sorted_hosts\u0027)"},{"line_number":101,"context_line":"    def test_schedule_old_conductor(self, mock_get_hosts,"},{"line_number":102,"context_line":"            mock_get_all_states, mock_claim):"},{"line_number":103,"context_line":"        \"\"\"Old conductor can call scheduler without the instance_uuids"},{"line_number":104,"context_line":"        parameter. When this happens, we need to ensure we do not attempt to"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_0a037411","side":"PARENT","line":101,"updated":"2017-10-04 20:23:58.000000000","message":"Sorry if I missed this, but why are you removing this test? Unless you bump the minimum rpc version, this just breaks our api contract, even if it shouldn\u0027t happen in real life.","commit_id":"43e03b5d5b079b0ef54ede0ec0d4beb5865ea7e7"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c4e25e75b601613e4fe9949fbf44af999ec85de9","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                \u0027_get_all_host_states\u0027)"},{"line_number":99,"context_line":"    @mock.patch(\u0027nova.scheduler.filter_scheduler.FilterScheduler.\u0027"},{"line_number":100,"context_line":"                \u0027_get_sorted_hosts\u0027)"},{"line_number":101,"context_line":"    def test_schedule_old_conductor(self, mock_get_hosts,"},{"line_number":102,"context_line":"            mock_get_all_states, mock_claim):"},{"line_number":103,"context_line":"        \"\"\"Old conductor can call scheduler without the instance_uuids"},{"line_number":104,"context_line":"        parameter. When this happens, we need to ensure we do not attempt to"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_bb44b475","side":"PARENT","line":101,"in_reply_to":"7f515b1d_0a037411","updated":"2017-10-04 22:42:20.000000000","message":"I was under the impression that this was no longer needed. I\u0027ll return it.","commit_id":"43e03b5d5b079b0ef54ede0ec0d4beb5865ea7e7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a90efbe345fde627eedd980bc14c651c227b412","unresolved":false,"context_lines":[{"line_number":551,"context_line":"        for num in range(10):"},{"line_number":552,"context_line":"            host_name \u003d \"host%s\" % num"},{"line_number":553,"context_line":"            hs \u003d host_manager.HostState(host_name, \"node%s\" % num,"},{"line_number":554,"context_line":"                    uuids.cell)"},{"line_number":555,"context_line":"            hs.uuid \u003d getattr(uuids, host_name)"},{"line_number":556,"context_line":"            all_host_states.append(hs)"},{"line_number":557,"context_line":"            alloc_reqs[hs.uuid] \u003d {}"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_25466151","line":554,"range":{"start_line":554,"start_character":19,"end_line":554,"end_character":30},"updated":"2017-10-04 20:23:58.000000000","message":"These all use the same cell, which means you\u0027re not testing that the logic to find things in the same cell actually works, AFAICT.","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c4e25e75b601613e4fe9949fbf44af999ec85de9","unresolved":false,"context_lines":[{"line_number":551,"context_line":"        for num in range(10):"},{"line_number":552,"context_line":"            host_name \u003d \"host%s\" % num"},{"line_number":553,"context_line":"            hs \u003d host_manager.HostState(host_name, \"node%s\" % num,"},{"line_number":554,"context_line":"                    uuids.cell)"},{"line_number":555,"context_line":"            hs.uuid \u003d getattr(uuids, host_name)"},{"line_number":556,"context_line":"            all_host_states.append(hs)"},{"line_number":557,"context_line":"            alloc_reqs[hs.uuid] \u003d {}"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_5b7260e5","line":554,"range":{"start_line":554,"start_character":19,"end_line":554,"end_character":30},"in_reply_to":"7f515b1d_25466151","updated":"2017-10-04 22:42:20.000000000","message":"OK, added a test for that.","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a90efbe345fde627eedd980bc14c651c227b412","unresolved":false,"context_lines":[{"line_number":576,"context_line":""},{"line_number":577,"context_line":"        dests \u003d self.driver._schedule(self.context, spec_obj,"},{"line_number":578,"context_line":"                instance_uuids, alloc_reqs, None)"},{"line_number":579,"context_line":"        self.assertEqual(num_instances, len(dests))"},{"line_number":580,"context_line":"        for dest in dests:"},{"line_number":581,"context_line":"            self.assertEqual(total_returned, len(dest))"},{"line_number":582,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_059a45c4","line":579,"updated":"2017-10-04 20:23:58.000000000","message":"This doesn\u0027t assert that there were no primary overlaps right?","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c4e25e75b601613e4fe9949fbf44af999ec85de9","unresolved":false,"context_lines":[{"line_number":576,"context_line":""},{"line_number":577,"context_line":"        dests \u003d self.driver._schedule(self.context, spec_obj,"},{"line_number":578,"context_line":"                instance_uuids, alloc_reqs, None)"},{"line_number":579,"context_line":"        self.assertEqual(num_instances, len(dests))"},{"line_number":580,"context_line":"        for dest in dests:"},{"line_number":581,"context_line":"            self.assertEqual(total_returned, len(dest))"},{"line_number":582,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_7620914a","line":579,"in_reply_to":"7f515b1d_059a45c4","updated":"2017-10-04 22:42:20.000000000","message":"Done","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6a90efbe345fde627eedd980bc14c651c227b412","unresolved":false,"context_lines":[{"line_number":578,"context_line":"                instance_uuids, alloc_reqs, None)"},{"line_number":579,"context_line":"        self.assertEqual(num_instances, len(dests))"},{"line_number":580,"context_line":"        for dest in dests:"},{"line_number":581,"context_line":"            self.assertEqual(total_returned, len(dest))"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    def test_alternates_returned(self):"},{"line_number":584,"context_line":"        self._test_alternates_returned(num_instances\u003d1, num_alternates\u003d1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_25b42135","line":581,"updated":"2017-10-04 20:23:58.000000000","message":"This should probably assert that all the alternates are unique and not like (host1, host1, host2) right?","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"c4e25e75b601613e4fe9949fbf44af999ec85de9","unresolved":false,"context_lines":[{"line_number":578,"context_line":"                instance_uuids, alloc_reqs, None)"},{"line_number":579,"context_line":"        self.assertEqual(num_instances, len(dests))"},{"line_number":580,"context_line":"        for dest in dests:"},{"line_number":581,"context_line":"            self.assertEqual(total_returned, len(dest))"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    def test_alternates_returned(self):"},{"line_number":584,"context_line":"        self._test_alternates_returned(num_instances\u003d1, num_alternates\u003d1)"}],"source_content_type":"text/x-python","patch_set":13,"id":"7f515b1d_561d950d","line":581,"in_reply_to":"7f515b1d_25b42135","updated":"2017-10-04 22:42:20.000000000","message":"Done","commit_id":"9de7b5af09382213a6e78ff0455d9932c493bf86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"146b2ca9c7a4823e286cf755772b4633d4e04466","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            alloc_reqs[hs.uuid] \u003d {}"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"        mock_get_all_hosts.return_value \u003d all_host_states"},{"line_number":698,"context_line":"        mock_sorted.side_effect \u003d [all_host_states, reversed(all_host_states),"},{"line_number":699,"context_line":"                all_host_states]"},{"line_number":700,"context_line":"        mock_claim.return_value \u003d True"},{"line_number":701,"context_line":"        total_returned \u003d 3"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f4b6375_1baf1ae4","line":698,"range":{"start_line":698,"start_character":52,"end_line":698,"end_character":60},"updated":"2017-10-21 03:01:28.000000000","message":"This returns an iterator but don\u0027t we want a list here?","commit_id":"c438ab3a8732510b610bca82a7e4fa8f676d7976"}]}