)]}'
{"nova/scheduler/filters/numa_topology_filter.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8f784735d24c470c028731d9d7a2e91fe0ca15a5","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_85c14683","side":"PARENT","line":62,"updated":"2017-01-13 08:22:59.000000000","message":"I don\u0027t think it\u0027s specific to the NUMAtopologyFilter so we should fix the root cause of this issue which is when we pass spec_obj to the method \u0027host_passes\u0027 so every filters could take advantage of this fix.\n\nI don\u0027t think there is a case where we want a filter to modify that object when passing a host.","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bb7c3b9880ea4a9c162c6689ec4a30bb4ae6acfa","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_f931c4ed","side":"PARENT","line":62,"in_reply_to":"7a3c09a3_074c686f","updated":"2017-01-13 15:19:53.000000000","message":"\u003e \u003e We could just drop any changes made when each host_passes returns, and log a warning to indicate that the filter modified the input.\n\n\u003e When you say \"drop any changes\", what do you mean? We can\u0027t wipe the pinning/NUMA topology information as this is useful (for downstream, at least). I figured copying the object did just that.\n\n*We can\u0027t wipe any pinning/NUMA topology information provided in the initial RequestSpec object...","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6ac6b19783d74f99f96f6799d83ee29c64b2dc43","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_074c686f","side":"PARENT","line":62,"in_reply_to":"7a3c09a3_4c04d77d","updated":"2017-01-13 15:03:22.000000000","message":"\u003e On the other hand, it\u0027s really rude of this method to modify the things it is passed, and no other filter does/should do that. There is definitely overhead in duplicating the object for every filter, for every scheduler run.\n\nTrue. A longer term fix would be to modify the offending function to not modify the provided object, but that would be a far larger change and much harder to backport. I can do this separately, however.\n\n \u003e We could just drop any changes made when each host_passes returns, and log a warning to indicate that the filter modified the input.\n\nWhen you say \"drop any changes\", what do you mean? We can\u0027t wipe the pinning/NUMA topology information as this is useful (for downstream, at least). I figured copying the object did just that.","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5b9f4259da85acd8f0bc9b5f909730bc48d5f2c3","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_4c04d77d","side":"PARENT","line":62,"in_reply_to":"7a3c09a3_5c11f4ab","updated":"2017-01-13 14:52:41.000000000","message":"On the other hand, it\u0027s really rude of this method to modify the things it is passed, and no other filter does/should do that. There is definitely overhead in duplicating the object for every filter, for every scheduler run.\n\nWe could just drop any changes made when each host_passes returns, and log a warning to indicate that the filter modified the input.","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f80d35a3bfe51f4003c5ba5b0430ccab21bc1517","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_5c11f4ab","side":"PARENT","line":62,"in_reply_to":"7a3c09a3_85c14683","updated":"2017-01-13 11:40:12.000000000","message":"Good call - I\u0027ll do that","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3c765cdc75327810e029ab77ff7602ade75eab76","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        return True"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":63,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":64,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":65,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a3905b3_ee32440f","side":"PARENT","line":62,"in_reply_to":"7a3c09a3_f931c4ed","updated":"2017-01-23 15:14:16.000000000","message":"I\u0027m just going to fix this in NUMATopologyFilter like I did in PS2, then look at a larger, less-backportable rework of the offending patch after","commit_id":"aacb8daa2cd489ffa44eee4268b8887d602f1f37"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d7e167dbce5a7e9f30625eb09f34f809b094377f","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        # doing this. That\u0027s a large, non-backportable cleanup however, so for"},{"line_number":68,"context_line":"        # now we just duplicate spec_obj to prevent changes propagating to"},{"line_number":69,"context_line":"        # future filter calls."},{"line_number":70,"context_line":"        spec_obj \u003d spec_obj.obj_clone()"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":73,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_ecbb2ce3","line":70,"updated":"2017-01-23 15:32:52.000000000","message":"This should be trivial to test in a unit test, which I think we should do.","commit_id":"29bc0390e5828a1df40a09732e40a21c8aa8e3c9"}],"nova/tests/unit/scheduler/filters/test_numa_topology_filters.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"43bc613cca0f990b9b5e63f74c9179bb0d1820eb","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            spec_obj.flavor \u003d fake_flavor"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            assertion(self.filt_cls.host_passes(host, spec_obj))"},{"line_number":175,"context_line":"            self.assertIsNone(spec_obj.numa_topology.cells[0].cpu_pinning)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_numa_topology_filter_fail_cpu_thread_policy_require(self):"},{"line_number":178,"context_line":"        cpu_policy \u003d fields.CPUAllocationPolicy.DEDICATED"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_4946e94f","line":175,"updated":"2017-01-23 17:50:10.000000000","message":"This is kindof an indirect test for the actual change, checking the symptom and not the actual code. But, I pulled this down and tested that it does in fact fail without the change to the filter, so okay.","commit_id":"bff2030ecea8a1d21e03c61a7ece02f40dc25c5d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e88066ea1b5d405f8f0d99a1e755586da1cba3f2","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            spec_obj.flavor \u003d fake_flavor"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            assertion(self.filt_cls.host_passes(host, spec_obj))"},{"line_number":175,"context_line":"            self.assertIsNone(spec_obj.numa_topology.cells[0].cpu_pinning)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_numa_topology_filter_fail_cpu_thread_policy_require(self):"},{"line_number":178,"context_line":"        cpu_policy \u003d fields.CPUAllocationPolicy.DEDICATED"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_cd80058e","line":175,"in_reply_to":"5a3905b3_4946e94f","updated":"2017-01-24 09:48:26.000000000","message":"I did this on purpose :) I figured the reason we were doing the copy was more important than how we did said copy.","commit_id":"bff2030ecea8a1d21e03c61a7ece02f40dc25c5d"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"94e1b9ecd9160d6e6c3d6541518935963c3910b2","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            spec_obj.flavor \u003d fake_flavor"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"            assertion(self.filt_cls.host_passes(host, spec_obj))"},{"line_number":175,"context_line":"            self.assertIsNone(spec_obj.numa_topology.cells[0].cpu_pinning)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def test_numa_topology_filter_fail_cpu_thread_policy_require(self):"},{"line_number":178,"context_line":"        cpu_policy \u003d fields.CPUAllocationPolicy.DEDICATED"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a3905b3_107e9369","line":175,"in_reply_to":"5a3905b3_cd80058e","updated":"2017-01-24 15:31:25.000000000","message":"I would have preferred to have that new assertion into a separate test that would only verify that symptom, exactly like we do for regression tests but that\u0027s a nit.","commit_id":"bff2030ecea8a1d21e03c61a7ece02f40dc25c5d"}]}