)]}'
{"nova/scheduler/filter_scheduler.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"007c3f092ccb3c716eef455026ac9d8128515de2","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        # Couldn\u0027t fulfill the request_spec"},{"line_number":70,"context_line":"        if len(selected_hosts) \u003c num_instances:"},{"line_number":71,"context_line":"            # NOTE(Rui Chen): If multiple creating failed, force to update"},{"line_number":72,"context_line":"            # HostStates in order to release the resource consumed by"},{"line_number":73,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_fbfd34b3","line":71,"updated":"2015-01-15 12:45:11.000000000","message":"creating \u003d\u003e creates","commit_id":"21740a801a94c66ea4384594d906c1e3380b1daf"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"964ac7135e5c6aa6edeb87c60f03783d0f534b5d","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        # Couldn\u0027t fulfill the request_spec"},{"line_number":70,"context_line":"        if len(selected_hosts) \u003c num_instances:"},{"line_number":71,"context_line":"            # NOTE(Rui Chen): If multiple creating failed, force to update"},{"line_number":72,"context_line":"            # HostStates in order to release the resource consumed by"},{"line_number":73,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_76b15990","line":71,"in_reply_to":"3a961159_fbfd34b3","updated":"2015-01-15 13:32:09.000000000","message":"Done","commit_id":"21740a801a94c66ea4384594d906c1e3380b1daf"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"007c3f092ccb3c716eef455026ac9d8128515de2","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            # HostStates in order to release the resource consumed by"},{"line_number":73,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            self.host_manager.get_all_host_states(context, force_update\u003dTrue)"},{"line_number":76,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":77,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":78,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_7bd86452","line":75,"updated":"2015-01-15 12:45:11.000000000","message":"i am not sure about this - so each time there is a failure, which in the worst case scenario when booting multiple instances we will invoke get_all_host_states","commit_id":"21740a801a94c66ea4384594d906c1e3380b1daf"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"964ac7135e5c6aa6edeb87c60f03783d0f534b5d","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            # HostStates in order to release the resource consumed by"},{"line_number":73,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            self.host_manager.get_all_host_states(context, force_update\u003dTrue)"},{"line_number":76,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":77,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":78,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_36bb5176","line":75,"in_reply_to":"3a961159_7bd86452","updated":"2015-01-15 13:32:09.000000000","message":"Yes, it is, we must refresh the host states from db, otherwise we will hit the bug/1408859. BTW I think we don\u0027t need to invoke get_all_host_states if num_instances \u003d\u003d 1. I will push a patch to improve it.","commit_id":"21740a801a94c66ea4384594d906c1e3380b1daf"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"b445543199fd7b87e1087235ab08e482b3475167","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            if num_instances \u003e 1:"},{"line_number":76,"context_line":"                self.host_manager.get_all_host_states(context,"},{"line_number":77,"context_line":"                                                      force_update\u003dTrue)"},{"line_number":78,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":79,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":80,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_80b70685","line":77,"updated":"2015-01-19 12:18:44.000000000","message":"I agree it\u0027s a good catch, the\nconsume_from_instance function should be reverted because\nthe instance is consumed \n\nhowever, the get_all_host_states might be too aggressive since it will update all hosts, can we only update the affected hosts?","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"41388cb1883a93d04aa37b25680c9199da6fc123","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            if num_instances \u003e 1:"},{"line_number":76,"context_line":"                self.host_manager.get_all_host_states(context,"},{"line_number":77,"context_line":"                                                      force_update\u003dTrue)"},{"line_number":78,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":79,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":80,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_d43c57e4","line":77,"in_reply_to":"3a961159_015b1823","updated":"2015-01-20 05:32:29.000000000","message":"Changing \u0027updated\u0027 is the way of mini impacting code, but I don\u0027t think it is good idea. It makes some confusions.","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"da59e4a6e463987907ee74a2a94982ee9b962737","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            if num_instances \u003e 1:"},{"line_number":76,"context_line":"                self.host_manager.get_all_host_states(context,"},{"line_number":77,"context_line":"                                                      force_update\u003dTrue)"},{"line_number":78,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":79,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":80,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_015b1823","line":77,"in_reply_to":"3a961159_80b70685","updated":"2015-01-19 13:24:23.000000000","message":"Agree with jichenjc, update all the hosts is expensive. How about we just change \u0027HostStat.updated\u0027 to a old time. Then the host stats will be updated by next request.","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"41388cb1883a93d04aa37b25680c9199da6fc123","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":75,"context_line":"            if num_instances \u003e 1:"},{"line_number":76,"context_line":"                self.host_manager.get_all_host_states(context,"},{"line_number":77,"context_line":"                                                      force_update\u003dTrue)"},{"line_number":78,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":79,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":80,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cfa6840f","line":77,"in_reply_to":"3a961159_80b70685","updated":"2015-01-20 05:32:29.000000000","message":"Retrieving all compute nodes from db can not be avoided, it\u0027s different that only if consumed hosts was reverted in scheduler ram cache but not all. I can try to add the method \u0027revert_host_states\u0027 in HostManager, and just refresh the affected hosts. Thanks.","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"622545d700f7e1c266dc7c67ee02b81cae9c047d","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                self.host_manager.force_update_host_states(context,"},{"line_number":85,"context_line":"                                                           selected_hosts)"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_6f6ace1b","line":85,"updated":"2015-01-22 11:10:39.000000000","message":"see below comments\n\nif we got the array I mentioned, maybe we will duplicate update the host manager cache here?","commit_id":"d1d9b206b9b812f42d7bedb4aa205a6989bb4137"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"622545d700f7e1c266dc7c67ee02b81cae9c047d","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                scheduler_host_subset_size \u003d 1"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            chosen_host \u003d random.choice("},{"line_number":165,"context_line":"                weighed_hosts[0:scheduler_host_subset_size])"},{"line_number":166,"context_line":"            selected_hosts.append(chosen_host)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            # Now consume the resources so the filter/weights"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_0f84ead2","line":165,"updated":"2015-01-22 11:10:39.000000000","message":"if we want to deploy 10 instances\nthen we might find same host here, right? \nso selected_hosts might be [A,B,C,A, A ...]","commit_id":"d1d9b206b9b812f42d7bedb4aa205a6989bb4137"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"716fd301028e496c0a4dc9d2820a24525c3fafa8","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                scheduler_host_subset_size \u003d 1"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            chosen_host \u003d random.choice("},{"line_number":165,"context_line":"                weighed_hosts[0:scheduler_host_subset_size])"},{"line_number":166,"context_line":"            selected_hosts.append(chosen_host)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"            # Now consume the resources so the filter/weights"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_e3d6dac2","line":165,"in_reply_to":"1a930d6b_0f84ead2","updated":"2015-01-22 11:43:53.000000000","message":"ok, I get it, but I don\u0027t think it will be happen, the selected_hosts array had been converted to map in HostManager.force_update_host_states, the key of map is (host, nodename), so the duplicate hosts had been removed in the process. [A,B,C,A, A ...] \u003d\u003e {(hosta, nodea):A, (hostb, nodeb):B, (hostc, nodec):C}\nSee details in https://review.openstack.org/#/c/147048/6/nova/scheduler/host_manager.py","commit_id":"d1d9b206b9b812f42d7bedb4aa205a6989bb4137"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"564abbeda815b275f91f488c9fdc64e5ef052f65","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                           dict(request_spec\u003drequest_spec))"},{"line_number":73,"context_line":"        return dests"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _check_fulfill_for_multiple_create(self, context, num_instances,"},{"line_number":76,"context_line":"                                           selected_hosts):"},{"line_number":77,"context_line":"        # Couldn\u0027t fulfill the request_spec"},{"line_number":78,"context_line":"        if len(selected_hosts) \u003c num_instances:"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_8787f911","line":75,"updated":"2015-02-09 10:09:15.000000000","message":"the name of the method is confusing\n\nthis should be something like:\n_validate_host_selection(..)","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"e3cc1065e8f32564792910598105672190aebbd0","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                           dict(request_spec\u003drequest_spec))"},{"line_number":73,"context_line":"        return dests"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _check_fulfill_for_multiple_create(self, context, num_instances,"},{"line_number":76,"context_line":"                                           selected_hosts):"},{"line_number":77,"context_line":"        # Couldn\u0027t fulfill the request_spec"},{"line_number":78,"context_line":"        if len(selected_hosts) \u003c num_instances:"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_2ebfdd61","line":75,"in_reply_to":"da86d52c_8787f911","updated":"2015-02-10 02:56:50.000000000","message":"Done","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"564abbeda815b275f91f488c9fdc64e5ef052f65","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":82,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                self.host_manager.force_update_host_states(context,"},{"line_number":85,"context_line":"                                                           selected_hosts)"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_e786f512","line":84,"updated":"2015-02-09 10:09:15.000000000","message":"why do we need this?","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"e3cc1065e8f32564792910598105672190aebbd0","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            # instance in the process of selecting host. See details:"},{"line_number":82,"context_line":"            # https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                self.host_manager.force_update_host_states(context,"},{"line_number":85,"context_line":"                                                           selected_hosts)"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_59ca1da2","line":84,"in_reply_to":"da86d52c_e786f512","updated":"2015-02-10 02:56:50.000000000","message":"In multiple create instance case, scheduler select host one by one, and consume chosen host each time, when there is no enough  suitable hosts for request, we must revoke the chosen host that had been consumed.","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"9779d28feb2c77fb237407b2455beddd65e19184","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            # refreshed according to database in next schedule, and release"},{"line_number":81,"context_line":"            # the resource consumed by instance in the process of selecting"},{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_9d656e1f","line":83,"updated":"2015-02-16 13:32:54.000000000","message":"this is checking is useless. We can run \u0027for host in selected_hosts\u0027 directly.  When selected_hosts is [], it will just skip.","commit_id":"347621ca9dc1e681733f90ff58fdb9c443d74cf0"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"fff9285ef07c015ff15f918674f07e99a930c053","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            # refreshed according to database in next schedule, and release"},{"line_number":81,"context_line":"            # the resource consumed by instance in the process of selecting"},{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_848ef325","line":83,"in_reply_to":"da86d52c_9d656e1f","updated":"2015-02-16 15:20:31.000000000","message":"This bug will be triggered when multiple-create.","commit_id":"347621ca9dc1e681733f90ff58fdb9c443d74cf0"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"9779d28feb2c77fb237407b2455beddd65e19184","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                        filter_properties[\u0027group_hosts\u0027])"},{"line_number":178,"context_line":"                filter_properties[\u0027group_hosts\u0027].add(chosen_host.obj.host)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self._validate_host_selection(context, num_instances, selected_hosts)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return selected_hosts"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_bd9eb205","line":180,"updated":"2015-02-16 13:32:54.000000000","message":"why not keep this at line 67? I saw there are some unitest changed is because you move the validation at here.","commit_id":"347621ca9dc1e681733f90ff58fdb9c443d74cf0"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"fff9285ef07c015ff15f918674f07e99a930c053","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                        filter_properties[\u0027group_hosts\u0027])"},{"line_number":178,"context_line":"                filter_properties[\u0027group_hosts\u0027].add(chosen_host.obj.host)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        self._validate_host_selection(context, num_instances, selected_hosts)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"        return selected_hosts"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"da86d52c_44b87b2a","line":180,"in_reply_to":"da86d52c_bd9eb205","updated":"2015-02-16 15:20:31.000000000","message":"I want to keep the method \u0027select_destinations\u0027 as simple as i can.","commit_id":"347621ca9dc1e681733f90ff58fdb9c443d74cf0"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"41eef2b7d0a137008986af4c9d8fb488cfe1aa45","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            # of selected HostState to None so that these HostStates are"},{"line_number":80,"context_line":"            # refreshed according to database in next schedule, and release"},{"line_number":81,"context_line":"            # the resource consumed by instance in the process of selecting"},{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_7e18de84","line":82,"updated":"2015-03-23 10:56:11.000000000","message":"nit : I don\u0027t like putting URLs in comments while we can just blame this file and see within the commit msg the related bug. If you need another rebase of your patch, please remove that, but unless it, that\u0027s fine.","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"a63e1f8a23838a297ad0bf3315320d81537f22d9","unresolved":false,"context_lines":[{"line_number":79,"context_line":"            # of selected HostState to None so that these HostStates are"},{"line_number":80,"context_line":"            # refreshed according to database in next schedule, and release"},{"line_number":81,"context_line":"            # the resource consumed by instance in the process of selecting"},{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_9ca379b1","line":82,"in_reply_to":"1a6ced46_7e18de84","updated":"2015-03-25 13:40:59.000000000","message":"Done","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"46727d41dd8b4c1e1b7da9e248b322d213c1587a","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":26,"id":"9a80dd14_29df523b","line":85,"updated":"2015-03-15 23:11:48.000000000","message":"The key for this review is setting updated to None. I don\u0027t see an explicit mock/test that does this. Can you please point me to it?","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"82fa57a49fd22e9f1d6af73a77ca147ec1f87dfe","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host. See details: https://bugs.launchpad.net/nova/+bug/1408859"},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_1b5f0610","line":85,"in_reply_to":"9a80dd14_29df523b","updated":"2015-03-23 03:58:32.000000000","message":"I add test logic in test_filter_scheduler.py, see my inline.\n\nThanks.","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f08f39f23acaf009191e298fba17f8c2b64f944","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host."},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":36,"id":"dac4157f_b244481b","line":85,"updated":"2015-04-21 14:30:17.000000000","message":"Sorry, I don\u0027t understand how this works.\n\nEach request gets its own new copy of the host list:\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L131\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L532\n\nIn the caching scheduler it is saved between requests, but I don\u0027t think setting the updated flag makes any difference:\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/caching_scheduler.py\n\nI am assuming I am missing something here?","commit_id":"df541a0d7b9ace656a2f4133a7fa123a8c241979"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"fde2626058b87ecdda3d86e8a8cf711d58694037","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host."},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":36,"id":"dac4157f_b4164fef","line":85,"in_reply_to":"dac4157f_55437b42","updated":"2015-04-22 10:15:54.000000000","message":"From the comment of CachingScheduler:\n\n\"In a similar way, if you have a high number of server deletes, the\nextra capacity from those deletes will not show up until the cache is\nrefreshed.\"\n\nThink about similar case, we boot up a lot of instance, but they failed at compute node, then CachingScheduler didn\u0027t have chance to know that.\n\nFor this case, although it failed in scheduler, but I\u0027m not sure we should trigger another host state buliding, because the design of CachingScheduler want to reduce the host state building time.\n\nSo this should be acceptable for CachingScheduler?","commit_id":"df541a0d7b9ace656a2f4133a7fa123a8c241979"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"22627ef09111fd300da2ea01e9554f455500101f","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host."},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":36,"id":"dac4157f_55437b42","line":85,"in_reply_to":"dac4157f_b244481b","updated":"2015-04-22 04:12:19.000000000","message":"In FilterScheduler case, the host_manager.get_all_host_states is called in each request, if we set the HostState.updated as None, it will be updated at next request in HostState.update_from_compute_node.\n\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L185\n\nIn CachingScheduler case, the HostState list is cached in caching scheduler, so the HostState with \u0027None\u0027 updated flag will be updated at next period. You are right, it maybe break in a cycle until the next periodic_tasks is executed.\n\nI have another implementation in patch set 19, a force_update will be triggered when no enough host is selected. It will update the HostState that cached in CachingScheduler.","commit_id":"df541a0d7b9ace656a2f4133a7fa123a8c241979"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"442651b8cb99c298469ae82faa6b538a10fd99a9","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host."},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":36,"id":"dac4157f_b7a9b17d","line":85,"in_reply_to":"dac4157f_b4164fef","updated":"2015-04-22 10:45:21.000000000","message":"I tend to agree with Alex. We know that the CachingScheduler can predict bad results given some stale data with the trade-off of more retries. Here, it wouldn\u0027t really impact the existing behaviour for the CachingScheduler (because it would just keep the situation as it is) but would improve the FilterScheduler.","commit_id":"df541a0d7b9ace656a2f4133a7fa123a8c241979"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"67bd7594ce9b2b67abfb29d8575395d024a4aa53","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            # host."},{"line_number":83,"context_line":"            if num_instances \u003e 1:"},{"line_number":84,"context_line":"                for host in selected_hosts:"},{"line_number":85,"context_line":"                    host.obj.updated \u003d None"},{"line_number":86,"context_line":"            # Log the details but don\u0027t put those into the reason since"},{"line_number":87,"context_line":"            # we don\u0027t want to give away too much information about our"},{"line_number":88,"context_line":"            # actual environment."}],"source_content_type":"text/x-python","patch_set":36,"id":"9abe1de1_77cef084","line":85,"in_reply_to":"dac4157f_b7a9b17d","updated":"2015-04-27 08:40:42.000000000","message":"@Rui Thanks for you note about this line, thats the main bit, I don\u0027t remember seeing that land:\nhttps://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L185\n\nSo this doesn\u0027t impact the caching scheduler at all, my bad, its the reverse problem to what I first thought. This change doesn\u0027t help with the caching scheduler, and thats OK.","commit_id":"df541a0d7b9ace656a2f4133a7fa123a8c241979"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"b445543199fd7b87e1087235ab08e482b3475167","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            state_key \u003d (host, node)"},{"line_number":415,"context_line":"            host_state \u003d self.host_state_map.get(state_key)"},{"line_number":416,"context_line":"            if host_state:"},{"line_number":417,"context_line":"                host_state.update_from_compute_node(compute, force_update)"},{"line_number":418,"context_line":"            else:"},{"line_number":419,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":420,"context_line":"                self.host_state_map[state_key] \u003d host_state"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_d2f72459","line":417,"updated":"2015-01-19 12:18:44.000000000","message":"use force_update\u003dforce_update will be better","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"41388cb1883a93d04aa37b25680c9199da6fc123","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            state_key \u003d (host, node)"},{"line_number":415,"context_line":"            host_state \u003d self.host_state_map.get(state_key)"},{"line_number":416,"context_line":"            if host_state:"},{"line_number":417,"context_line":"                host_state.update_from_compute_node(compute, force_update)"},{"line_number":418,"context_line":"            else:"},{"line_number":419,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":420,"context_line":"                self.host_state_map[state_key] \u003d host_state"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_aa95a633","line":417,"in_reply_to":"3a961159_d2f72459","updated":"2015-01-20 05:32:29.000000000","message":"Done","commit_id":"5a5714d815eb073cbf71bfd0d75910aff4d01f0d"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"e0c0fd644a97681c96fefea204c3a1fa15e6cc36","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        consumed_hosts_map \u003d {(x.obj.host, x.obj.nodename): x.obj"},{"line_number":440,"context_line":"                              for x in consumed_hosts}"},{"line_number":441,"context_line":"        compute_nodes \u003d db.compute_node_get_all(context)"},{"line_number":442,"context_line":"        for compute in compute_nodes:"},{"line_number":443,"context_line":"            service \u003d compute[\u0027service\u0027]"},{"line_number":444,"context_line":"            if not service:"},{"line_number":445,"context_line":"                LOG.warning(_LW(\"No service for compute ID %s\"), compute[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_b1ffc4aa","line":442,"updated":"2015-01-20 16:27:39.000000000","message":"I think the compute_nodes is an array and will it be helpful to make it a set? thus we don\u0027t need the duplicate update","commit_id":"e155e859846648c9478408a69b44073567656720"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"189621d689c8c49bbc088b1287409c1202d09f94","unresolved":false,"context_lines":[{"line_number":439,"context_line":"        consumed_hosts_map \u003d {(x.obj.host, x.obj.nodename): x.obj"},{"line_number":440,"context_line":"                              for x in consumed_hosts}"},{"line_number":441,"context_line":"        compute_nodes \u003d db.compute_node_get_all(context)"},{"line_number":442,"context_line":"        for compute in compute_nodes:"},{"line_number":443,"context_line":"            service \u003d compute[\u0027service\u0027]"},{"line_number":444,"context_line":"            if not service:"},{"line_number":445,"context_line":"                LOG.warning(_LW(\"No service for compute ID %s\"), compute[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a930d6b_7eb6308c","line":442,"in_reply_to":"3a961159_b1ffc4aa","updated":"2015-01-21 07:57:43.000000000","message":"Hi, @jichenjc, I don\u0027t know if the duplicate update will happen in any case, can you get me a sample? thanks","commit_id":"e155e859846648c9478408a69b44073567656720"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"716fd301028e496c0a4dc9d2820a24525c3fafa8","unresolved":false,"context_lines":[{"line_number":437,"context_line":"            return"},{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        consumed_hosts_map \u003d {(x.obj.host, x.obj.nodename): x.obj"},{"line_number":440,"context_line":"                              for x in consumed_hosts}"},{"line_number":441,"context_line":"        compute_nodes \u003d db.compute_node_get_all(context)"},{"line_number":442,"context_line":"        for compute in compute_nodes:"},{"line_number":443,"context_line":"            service \u003d compute[\u0027service\u0027]"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_43896e25","line":440,"updated":"2015-01-22 11:43:53.000000000","message":"converts list to map so that avoiding duplicate updating. the duplicate consumed hosts has same key (host, nodename) in the map.","commit_id":"d1d9b206b9b812f42d7bedb4aa205a6989bb4137"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"f31c344f3107b45cdcdb673d491d9d5562537d5e","unresolved":false,"context_lines":[{"line_number":455,"context_line":"            if state_key in consumed_hosts_map:"},{"line_number":456,"context_line":"                consumed_hosts_map.get("},{"line_number":457,"context_line":"                    state_key).update_from_compute_node(compute,"},{"line_number":458,"context_line":"                                                        force_update\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_976d5407","line":458,"updated":"2015-02-09 18:11:15.000000000","message":"I\u0027m really against this method because here you force a DB call for refreshing each HostState while it doesn\u0027t make sense because HostState is *not* persisted.\n\nHostState is actually recreated each time a new select_destinations() call is made because it actually queries the _schedule() method which calls the HostManager.get_filtered_hosts which generates it.","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"e3cc1065e8f32564792910598105672190aebbd0","unresolved":false,"context_lines":[{"line_number":455,"context_line":"            if state_key in consumed_hosts_map:"},{"line_number":456,"context_line":"                consumed_hosts_map.get("},{"line_number":457,"context_line":"                    state_key).update_from_compute_node(compute,"},{"line_number":458,"context_line":"                                                        force_update\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_6e12b50a","line":458,"in_reply_to":"da86d52c_976d5407","updated":"2015-02-10 02:56:50.000000000","message":"Actually, HostState is not recreated each time, it\u0027s cached in HostManager.host_state_map, when select_destinations() is called, HostManager update the HostState according to db in get_all_host_states(), but there is a trick in currently master code, when the update time of HostState is greater than db, it\u0027s not updated, so I must force update it when this bug occur.\n\nhttp://git.openstack.org/cgit/openstack/nova/tree/nova/scheduler/host_manager.py#n174","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"dad118c05cfc395e2b5a7a33cf1e28270da3c892","unresolved":false,"context_lines":[{"line_number":424,"context_line":"                host_state.update_from_compute_node(compute)"},{"line_number":425,"context_line":"            else:"},{"line_number":426,"context_line":"                host_state \u003d self.host_state_cls(host, node, compute\u003dcompute)"},{"line_number":427,"context_line":"                self.host_state_map[state_key] \u003d host_state"},{"line_number":428,"context_line":"            host_state.update_service(dict(service.iteritems()))"},{"line_number":429,"context_line":"            seen_nodes.add(state_key)"},{"line_number":430,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"da86d52c_e0655faf","line":427,"updated":"2015-02-12 09:22:52.000000000","message":"I got your comment on PS16, but I think it\u0027s overkill to create a new handler for updating the local HostStates.\n\nHere, it verifies if HostState is existing for the current node and updates it if necessary, or creates a new one if not yet there.\n\nIMHO, I think you only need to make sure that if a condition is there, you\u0027ll update the local HostState using the same update_from_compute_node() helper","commit_id":"54f4072dc33fec935a7448f34e166ba1612015d6"}],"nova/tests/unit/scheduler/test_filter_scheduler.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"564abbeda815b275f91f488c9fdc64e5ef052f65","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        request_spec \u003d {\u0027instance_type\u0027: {\u0027memory_mb\u0027: 512, \u0027root_gb\u0027: 512,"},{"line_number":303,"context_line":"                                          \u0027ephemeral_gb\u0027: 0,"},{"line_number":304,"context_line":"                                          \u0027vcpus\u0027: 1},"},{"line_number":305,"context_line":"                        \u0027instance_properties\u0027: {\u0027project_id\u0027: 1,"},{"line_number":306,"context_line":"                                                \u0027root_gb\u0027: 512,"},{"line_number":307,"context_line":"                                                \u0027memory_mb\u0027: 512,"},{"line_number":308,"context_line":"                                                \u0027ephemeral_gb\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_07014939","line":305,"updated":"2015-02-09 10:09:15.000000000","message":"is this not meant to be an instance object?","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"e3cc1065e8f32564792910598105672190aebbd0","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        request_spec \u003d {\u0027instance_type\u0027: {\u0027memory_mb\u0027: 512, \u0027root_gb\u0027: 512,"},{"line_number":303,"context_line":"                                          \u0027ephemeral_gb\u0027: 0,"},{"line_number":304,"context_line":"                                          \u0027vcpus\u0027: 1},"},{"line_number":305,"context_line":"                        \u0027instance_properties\u0027: {\u0027project_id\u0027: 1,"},{"line_number":306,"context_line":"                                                \u0027root_gb\u0027: 512,"},{"line_number":307,"context_line":"                                                \u0027memory_mb\u0027: 512,"},{"line_number":308,"context_line":"                                                \u0027ephemeral_gb\u0027: 0,"}],"source_content_type":"text/x-python","patch_set":16,"id":"da86d52c_abb9fa7a","line":305,"in_reply_to":"da86d52c_07014939","updated":"2015-02-10 02:56:50.000000000","message":"No, it\u0027s a dict.\n\n(Pdb) type(request_spec)\n\u003ctype \u0027dict\u0027\u003e\n(Pdb) type(request_spec[\u0027instance_properties\u0027])\n\u003ctype \u0027dict\u0027\u003e","commit_id":"84cf64dddbfdd8953c8bcca060e67ed918343684"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"41eef2b7d0a137008986af4c9d8fb488cfe1aa45","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                    \u0027get_filtered_hosts\u0027,"},{"line_number":336,"context_line":"                    side_effect\u003d[fake_hosts, None]) as m_get_filtered:"},{"line_number":337,"context_line":"                with mock.patch.object(self.driver.host_manager,"},{"line_number":338,"context_line":"                        \u0027get_weighed_hosts\u0027,"},{"line_number":339,"context_line":"                        return_value\u003dfake_hosts) as m_get_weighed:"},{"line_number":340,"context_line":"                    try:"},{"line_number":341,"context_line":"                        self.driver.select_destinations(self.context,"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_a1eebb20","line":338,"updated":"2015-03-23 10:56:11.000000000","message":"nit : I don\u0027t like those nested context managers, you should rather put them as decorators for the unittest.","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"a63e1f8a23838a297ad0bf3315320d81537f22d9","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                    \u0027get_filtered_hosts\u0027,"},{"line_number":336,"context_line":"                    side_effect\u003d[fake_hosts, None]) as m_get_filtered:"},{"line_number":337,"context_line":"                with mock.patch.object(self.driver.host_manager,"},{"line_number":338,"context_line":"                        \u0027get_weighed_hosts\u0027,"},{"line_number":339,"context_line":"                        return_value\u003dfake_hosts) as m_get_weighed:"},{"line_number":340,"context_line":"                    try:"},{"line_number":341,"context_line":"                        self.driver.select_destinations(self.context,"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_e2f12802","line":338,"in_reply_to":"1a6ced46_a1eebb20","updated":"2015-03-25 13:40:59.000000000","message":"Done","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"41eef2b7d0a137008986af4c9d8fb488cfe1aa45","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        # Make sure that we provided a reason why"},{"line_number":346,"context_line":"                        # NoValidHost."},{"line_number":347,"context_line":"                        self.assertEqual(2, m_get_filtered.call_count)"},{"line_number":348,"context_line":"                        self.assertEqual(1, m_get_weighed.call_count)"},{"line_number":349,"context_line":"                        selected_host \u003d fake_hosts[0].obj"},{"line_number":350,"context_line":"                        self.assertEqual(1, selected_host."},{"line_number":351,"context_line":"                                         consume_from_instance.call_count)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_81d0df54","line":348,"updated":"2015-03-23 10:56:11.000000000","message":"please don\u0027t check on the number of calls but rather use self.assert_call_with twice, because that\u0027s also checking the passed args.","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"a63e1f8a23838a297ad0bf3315320d81537f22d9","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                        # Make sure that we provided a reason why"},{"line_number":346,"context_line":"                        # NoValidHost."},{"line_number":347,"context_line":"                        self.assertEqual(2, m_get_filtered.call_count)"},{"line_number":348,"context_line":"                        self.assertEqual(1, m_get_weighed.call_count)"},{"line_number":349,"context_line":"                        selected_host \u003d fake_hosts[0].obj"},{"line_number":350,"context_line":"                        self.assertEqual(1, selected_host."},{"line_number":351,"context_line":"                                         consume_from_instance.call_count)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_2d9891bf","line":348,"in_reply_to":"1a6ced46_81d0df54","updated":"2015-03-25 13:40:59.000000000","message":"Done","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"82fa57a49fd22e9f1d6af73a77ca147ec1f87dfe","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                        selected_host \u003d fake_hosts[0].obj"},{"line_number":350,"context_line":"                        self.assertEqual(1, selected_host."},{"line_number":351,"context_line":"                                         consume_from_instance.call_count)"},{"line_number":352,"context_line":"                        self.assertIsNone(selected_host.updated)"},{"line_number":353,"context_line":"                        self.assertIn(\u0027reason\u0027, e.kwargs)"},{"line_number":354,"context_line":"                        self.assertTrue(len(e.kwargs[\u0027reason\u0027]) \u003e 0)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_1b48e65a","line":352,"updated":"2015-03-23 03:58:32.000000000","message":"Here.","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":5638,"name":"Davanum Srinivas","email":"davanum@gmail.com","username":"dims-v"},"change_message_id":"5e95d449d1d2b628d9249c528d1468a54ebc778e","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                        selected_host \u003d fake_hosts[0].obj"},{"line_number":350,"context_line":"                        self.assertEqual(1, selected_host."},{"line_number":351,"context_line":"                                         consume_from_instance.call_count)"},{"line_number":352,"context_line":"                        self.assertIsNone(selected_host.updated)"},{"line_number":353,"context_line":"                        self.assertIn(\u0027reason\u0027, e.kwargs)"},{"line_number":354,"context_line":"                        self.assertTrue(len(e.kwargs[\u0027reason\u0027]) \u003e 0)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a6ced46_af14e7f5","line":352,"in_reply_to":"1a6ced46_1b48e65a","updated":"2015-03-23 09:54:00.000000000","message":"thanks","commit_id":"7383a7a77a86d23d38543a3517e512f9bd878b39"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9fb778dd558d487647e231e3c6af875f503bc02e","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                                                \u0027uuid\u0027: \u0027fake-uuid\u0027},"},{"line_number":316,"context_line":"                        \u0027num_instances\u0027: 1}"},{"line_number":317,"context_line":"        with mock.patch.object(self.driver, \u0027_get_all_host_states\u0027,"},{"line_number":318,"context_line":"                               return_value\u003dlist()):"},{"line_number":319,"context_line":"            self.assertRaises(exception.NoValidHost,"},{"line_number":320,"context_line":"                              self.driver.select_destinations, self.context,"},{"line_number":321,"context_line":"                              request_spec, {})"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_89d22f5a","line":318,"updated":"2015-03-26 10:49:35.000000000","message":"nit: that\u0027s better for performance to return [] and not call list() instead, but that\u0027s fine...","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"29d3cd929c95b6bf85065ce176f7e252d4df8b79","unresolved":false,"context_lines":[{"line_number":315,"context_line":"                                                \u0027uuid\u0027: \u0027fake-uuid\u0027},"},{"line_number":316,"context_line":"                        \u0027num_instances\u0027: 1}"},{"line_number":317,"context_line":"        with mock.patch.object(self.driver, \u0027_get_all_host_states\u0027,"},{"line_number":318,"context_line":"                               return_value\u003dlist()):"},{"line_number":319,"context_line":"            self.assertRaises(exception.NoValidHost,"},{"line_number":320,"context_line":"                              self.driver.select_destinations, self.context,"},{"line_number":321,"context_line":"                              request_spec, {})"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_eca2d149","line":318,"in_reply_to":"1a6ced46_89d22f5a","updated":"2015-03-26 11:33:44.000000000","message":"Done","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9fb778dd558d487647e231e3c6af875f503bc02e","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                               return_value\u003dlist()):"},{"line_number":319,"context_line":"            self.assertRaises(exception.NoValidHost,"},{"line_number":320,"context_line":"                              self.driver.select_destinations, self.context,"},{"line_number":321,"context_line":"                              request_spec, {})"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @mock.patch.object(filter_scheduler.FilterScheduler,"},{"line_number":324,"context_line":"                       \u0027_get_all_host_states\u0027)"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_e9ae4397","line":321,"updated":"2015-03-26 10:49:35.000000000","message":"I don\u0027t like the change you\u0027re doing by implicitely testing the _schedule method.\n\nInstead you should keep mocking _schedule in that test and the test below, and only add unittests for the new method you\u0027re adding. Ideally, a unittest should only focus on the section code of the method it\u0027s testing by mocking all the callables done in the tested method; and test separately the callable methods with other unittests which would call them directly.","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"29d3cd929c95b6bf85065ce176f7e252d4df8b79","unresolved":false,"context_lines":[{"line_number":318,"context_line":"                               return_value\u003dlist()):"},{"line_number":319,"context_line":"            self.assertRaises(exception.NoValidHost,"},{"line_number":320,"context_line":"                              self.driver.select_destinations, self.context,"},{"line_number":321,"context_line":"                              request_spec, {})"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"    @mock.patch.object(filter_scheduler.FilterScheduler,"},{"line_number":324,"context_line":"                       \u0027_get_all_host_states\u0027)"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_6c8ee1d1","line":321,"in_reply_to":"1a6ced46_e9ae4397","updated":"2015-03-26 11:33:44.000000000","message":"Done","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9fb778dd558d487647e231e3c6af875f503bc02e","unresolved":false,"context_lines":[{"line_number":339,"context_line":"                                                \u0027ephemeral_gb\u0027: 0,"},{"line_number":340,"context_line":"                                                \u0027vcpus\u0027: 1,"},{"line_number":341,"context_line":"                                                \u0027os_type\u0027: \u0027Linux\u0027,"},{"line_number":342,"context_line":"                                                \u0027uuid\u0027: \u0027fake-uuid\u0027},"},{"line_number":343,"context_line":"                        \u0027num_instances\u0027: 2}"},{"line_number":344,"context_line":"        fake_hosts \u003d [mock.MagicMock()]"},{"line_number":345,"context_line":"        m_get_all.return_value \u003d fake_hosts"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_496af781","line":342,"updated":"2015-03-26 10:49:35.000000000","message":"I don\u0027t really think it\u0027s necessary to modify the request_spec dictionary by explicitely defining the attributes, it can create confusion on what is readed and needs some update if we want to update the dictionary. \n\nHere, you only introspect the num_instances field in the method you\u0027re about to validate, so please only leave this.","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"29d3cd929c95b6bf85065ce176f7e252d4df8b79","unresolved":false,"context_lines":[{"line_number":339,"context_line":"                                                \u0027ephemeral_gb\u0027: 0,"},{"line_number":340,"context_line":"                                                \u0027vcpus\u0027: 1,"},{"line_number":341,"context_line":"                                                \u0027os_type\u0027: \u0027Linux\u0027,"},{"line_number":342,"context_line":"                                                \u0027uuid\u0027: \u0027fake-uuid\u0027},"},{"line_number":343,"context_line":"                        \u0027num_instances\u0027: 2}"},{"line_number":344,"context_line":"        fake_hosts \u003d [mock.MagicMock()]"},{"line_number":345,"context_line":"        m_get_all.return_value \u003d fake_hosts"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a6ced46_acc379eb","line":342,"in_reply_to":"1a6ced46_496af781","updated":"2015-03-26 11:33:44.000000000","message":"Done","commit_id":"b0655d2bda474ad70dc95aac0b473212f4669a18"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6262c29920830de530beafd7ed427fefdcb2c163","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Tests For Filter Scheduler."},{"line_number":17,"context_line":"\"\"\""},{"line_number":18,"context_line":"from nova import objects"},{"line_number":19,"context_line":"objects.register_all()"},{"line_number":20,"context_line":"import mock"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from nova import exception"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa963961_cd4631bf","line":19,"updated":"2015-03-27 10:02:53.000000000","message":"That\u0027s unnecessary, see\n\nhttps://github.com/openstack/nova/blob/master/nova/test.py#L63-L66","commit_id":"0b7955add5868893b4879bba53f9fe25574ae7f3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6262c29920830de530beafd7ed427fefdcb2c163","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                               side_effect\u003dexception.NoValidHost(reason\u003d\"\")):"},{"line_number":309,"context_line":"            self.assertRaises(exception.NoValidHost,"},{"line_number":310,"context_line":"                              self.driver.select_destinations, self.context,"},{"line_number":311,"context_line":"                              {}, {})"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    @mock.patch.object(filter_scheduler.FilterScheduler,"},{"line_number":314,"context_line":"                       \u0027_get_all_host_states\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa963961_8dc149e7","line":311,"updated":"2015-03-27 10:02:53.000000000","message":"nit: well, this test is maybe a no-brainer but that\u0027s fine...","commit_id":"0b7955add5868893b4879bba53f9fe25574ae7f3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6262c29920830de530beafd7ed427fefdcb2c163","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                       \u0027_get_all_host_states\u0027)"},{"line_number":315,"context_line":"    @mock.patch.object(host_manager.HostManager, \u0027get_filtered_hosts\u0027)"},{"line_number":316,"context_line":"    @mock.patch.object(host_manager.HostManager, \u0027get_weighed_hosts\u0027)"},{"line_number":317,"context_line":"    def test_schedule_no_valid_host_not_enough(self, m_get_weighed,"},{"line_number":318,"context_line":"                                               m_get_filtered, m_get_all):"},{"line_number":319,"context_line":"        # Tests that we have fewer hosts available than number of instances"},{"line_number":320,"context_line":"        # requested to build."}],"source_content_type":"text/x-python","patch_set":28,"id":"fa963961_ed4e95c0","line":317,"updated":"2015-03-27 10:02:53.000000000","message":"nit: I would prefer that you explicitely rename the unittest to test_validate_host_selection_with_multiple_hosts","commit_id":"0b7955add5868893b4879bba53f9fe25574ae7f3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6262c29920830de530beafd7ed427fefdcb2c163","unresolved":false,"context_lines":[{"line_number":326,"context_line":"        m_get_filtered.side_effect \u003d [fake_hosts, None]"},{"line_number":327,"context_line":"        m_get_weighed.return_value \u003d fake_hosts"},{"line_number":328,"context_line":"        try:"},{"line_number":329,"context_line":"            self.driver._schedule(self.context, request_spec, {})"},{"line_number":330,"context_line":"            self.fail(\u0027Expected NoValidHost to be raised.\u0027)"},{"line_number":331,"context_line":"        except exception.NoValidHost as e:"},{"line_number":332,"context_line":"            # Make sure that we provided a reason why"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa963961_cd9071ef","line":329,"updated":"2015-03-27 10:02:53.000000000","message":"you don\u0027t need to test against _schedule. Here, you need to cover _validate_host_selection","commit_id":"0b7955add5868893b4879bba53f9fe25574ae7f3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6262c29920830de530beafd7ed427fefdcb2c163","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":28,"id":"fa963961_2d035d44","line":344,"updated":"2015-03-27 10:02:53.000000000","message":"IIRC, two tests are missing for covering validate_host_selection() :\n\n - one testing num_instances \u003d\u003d 1\n - one testing selected_hosts \u003c num_instances","commit_id":"0b7955add5868893b4879bba53f9fe25574ae7f3"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ced91f565447d152b5200b226e38969fa8b197a2","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                                                 fake_hosts)"},{"line_number":309,"context_line":"            selected_host \u003d fake_hosts[0].obj"},{"line_number":310,"context_line":"            self.assertIsNotNone(selected_host.updated)"},{"line_number":311,"context_line":"        except Exception:"},{"line_number":312,"context_line":"            self.fail(\u0027Expected no exception to be raised.\u0027)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_validate_host_selection_no_valid_host_not_enough(self):"}],"source_content_type":"text/x-python","patch_set":30,"id":"da9b358b_42e859b4","line":311,"updated":"2015-04-01 08:05:28.000000000","message":"mmm, if an Exception is raised, the test will fail anyway. I would rather check that LOG.debug is not called or I would only catch exception.NoValidHost","commit_id":"b9dc3cb160a1dd70ff0362780d40ecc866e637f8"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"e7aa24dca4888c98f06d071f8c05928746d065fb","unresolved":false,"context_lines":[{"line_number":308,"context_line":"                                                 fake_hosts)"},{"line_number":309,"context_line":"            selected_host \u003d fake_hosts[0].obj"},{"line_number":310,"context_line":"            self.assertIsNotNone(selected_host.updated)"},{"line_number":311,"context_line":"        except Exception:"},{"line_number":312,"context_line":"            self.fail(\u0027Expected no exception to be raised.\u0027)"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_validate_host_selection_no_valid_host_not_enough(self):"}],"source_content_type":"text/x-python","patch_set":30,"id":"da9b358b_6599271f","line":311,"in_reply_to":"da9b358b_42e859b4","updated":"2015-04-01 08:50:51.000000000","message":"Done","commit_id":"b9dc3cb160a1dd70ff0362780d40ecc866e637f8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"67bd7594ce9b2b67abfb29d8575395d024a4aa53","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                self.driver.select_destinations, self.context,"},{"line_number":312,"context_line":"                {\u0027num_instances\u0027: 1}, {})"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_select_destinations_no_valid_host_not_enough(self):"},{"line_number":315,"context_line":"        # Tests that we have fewer hosts available than number of instances"},{"line_number":316,"context_line":"        # requested to build."},{"line_number":317,"context_line":"        with mock.patch.object(self.driver, \u0027_schedule\u0027,"}],"source_content_type":"text/x-python","patch_set":36,"id":"9abe1de1_174694bf","side":"PARENT","line":314,"updated":"2015-04-27 08:40:42.000000000","message":"Why have you deleted these tests?\n\nI don\u0027t think the others replace what these tests are doing? Should these tests not also pass after this change?\n\nHowever, the ones you have added are great as they clearly test the new code you are adding here.","commit_id":"dc06cf40c44eabcd99ad97486914515a839d2fc4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b1c02277e772042bc451da2d41c3d8eea6e4b34e","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                self.driver.select_destinations, self.context,"},{"line_number":312,"context_line":"                {\u0027num_instances\u0027: 1}, {})"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_select_destinations_no_valid_host_not_enough(self):"},{"line_number":315,"context_line":"        # Tests that we have fewer hosts available than number of instances"},{"line_number":316,"context_line":"        # requested to build."},{"line_number":317,"context_line":"        with mock.patch.object(self.driver, \u0027_schedule\u0027,"}],"source_content_type":"text/x-python","patch_set":36,"id":"9abe1de1_7d6daf31","side":"PARENT","line":314,"in_reply_to":"9abe1de1_174694bf","updated":"2015-04-27 10:08:32.000000000","message":"Hell yeah, John is right, you shouldn\u0027t cut cover like that IMHO.","commit_id":"dc06cf40c44eabcd99ad97486914515a839d2fc4"},{"author":{"_account_id":8276,"name":"kiwik","email":"chenrui.momo@gmail.com","username":"kiwik"},"change_message_id":"26e3b9ca874500ec2af630c745489f6271b7193a","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                self.driver.select_destinations, self.context,"},{"line_number":312,"context_line":"                {\u0027num_instances\u0027: 1}, {})"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    def test_select_destinations_no_valid_host_not_enough(self):"},{"line_number":315,"context_line":"        # Tests that we have fewer hosts available than number of instances"},{"line_number":316,"context_line":"        # requested to build."},{"line_number":317,"context_line":"        with mock.patch.object(self.driver, \u0027_schedule\u0027,"}],"source_content_type":"text/x-python","patch_set":36,"id":"9abe1de1_2b5ff10f","side":"PARENT","line":314,"in_reply_to":"9abe1de1_174694bf","updated":"2015-04-27 10:06:10.000000000","message":"I move the \u0027check-enough-host\u0027 logic into method \u0027_schedule\u0027, so I think the other test cases in test_filter_scheduler had covered my logic by default, these test cases that deleted by me was not need anymore.\n\nAnd I think the new test cases can covered the raising NoValidHost logic.","commit_id":"dc06cf40c44eabcd99ad97486914515a839d2fc4"}]}