)]}'
{"nova/scheduler/client/__init__.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e1590bc289cf9ec14d0d20b974b4c8994e26199","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from nova.scheduler import utils"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SchedulerClient(object):"},{"line_number":22,"context_line":"    \"\"\"Client library for placing calls to the scheduler.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_1e1ea60e","line":21,"updated":"2018-11-07 15:42:55.000000000","message":"nit: add a TODO to remove this proxy and just have callers hit the necessary clients directly.","commit_id":"5d1a50018510b2b281ad33895ae2d9555f5d5b05"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"525f84ebe70e2b31eacfab732e1e122e0c82ce23","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from nova.scheduler import utils"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SchedulerClient(object):"},{"line_number":22,"context_line":"    \"\"\"Client library for placing calls to the scheduler.\"\"\""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_de8eee31","line":21,"in_reply_to":"3f79a3b5_1e1ea60e","updated":"2018-11-07 15:44:39.000000000","message":"Yup, follow-up.","commit_id":"5d1a50018510b2b281ad33895ae2d9555f5d5b05"}],"nova/tests/unit/conductor/tasks/test_live_migrate.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3abe2b0e0ff5b331f6bf4bad84ce428c3e2a0d35","unresolved":false,"context_lines":[{"line_number":635,"context_line":"        \"\"\""},{"line_number":636,"context_line":"        with mock.patch.object("},{"line_number":637,"context_line":"                self.task.scheduler_client.reportclient,"},{"line_number":638,"context_line":"                \u0027remove_provider_from_instance_allocation\u0027) as remove_provider:"},{"line_number":639,"context_line":"            self.task._remove_host_allocations(\u0027host\u0027, \u0027node\u0027)"},{"line_number":640,"context_line":"        remove_provider.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_67fbbc47","line":638,"range":{"start_line":638,"start_character":16,"end_line":638,"end_character":58},"updated":"2018-11-05 23:53:17.000000000","message":"This was missed in [1], since LazyLoad has a __getattr__ that will happily pretend any random method exists, and the assertion is that that mock isn\u0027t called, which it wouldn\u0027t be, since it\u0027s a method that doesn\u0027t exist.\n\n[1] I2af45a9540e7ccd60ace80d9fcadc79972da7df7","commit_id":"430777ee11bc8d60e613373656329d21c63e6da3"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"33dff166e762158b9c3d465a3b8a645d2bdd3eeb","unresolved":false,"context_lines":[{"line_number":633,"context_line":"        \"\"\"Tests that failing to find a ComputeNode will not blow up"},{"line_number":634,"context_line":"        the _remove_host_allocations method."},{"line_number":635,"context_line":"        \"\"\""},{"line_number":636,"context_line":"        method \u003d \u0027remove_provider_tree_from_instance_allocation\u0027"},{"line_number":637,"context_line":"        with mock.patch.object(self.task.scheduler_client.reportclient,"},{"line_number":638,"context_line":"                               method) as remove_provider:"},{"line_number":639,"context_line":"            self.task._remove_host_allocations(\u0027host\u0027, \u0027node\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c4796381","line":636,"updated":"2018-11-06 10:52:37.000000000","message":"Was this something your editor happened to do for you, or was there a more specific you did it?\n\nPeople might declare it unrelated, but meh, it\u0027s a nice cleanup.","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b070be415aab4bfc37df0cfca0b8fb72266d1c80","unresolved":false,"context_lines":[{"line_number":633,"context_line":"        \"\"\"Tests that failing to find a ComputeNode will not blow up"},{"line_number":634,"context_line":"        the _remove_host_allocations method."},{"line_number":635,"context_line":"        \"\"\""},{"line_number":636,"context_line":"        method \u003d \u0027remove_provider_tree_from_instance_allocation\u0027"},{"line_number":637,"context_line":"        with mock.patch.object(self.task.scheduler_client.reportclient,"},{"line_number":638,"context_line":"                               method) as remove_provider:"},{"line_number":639,"context_line":"            self.task._remove_host_allocations(\u0027host\u0027, \u0027node\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c55076d4","line":636,"in_reply_to":"3f79a3b5_3c7947fa","updated":"2018-11-06 17:10:44.000000000","message":"Done","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a0175a0b2f6eef63be81ef357e47873e9de85ebe","unresolved":false,"context_lines":[{"line_number":633,"context_line":"        \"\"\"Tests that failing to find a ComputeNode will not blow up"},{"line_number":634,"context_line":"        the _remove_host_allocations method."},{"line_number":635,"context_line":"        \"\"\""},{"line_number":636,"context_line":"        method \u003d \u0027remove_provider_tree_from_instance_allocation\u0027"},{"line_number":637,"context_line":"        with mock.patch.object(self.task.scheduler_client.reportclient,"},{"line_number":638,"context_line":"                               method) as remove_provider:"},{"line_number":639,"context_line":"            self.task._remove_host_allocations(\u0027host\u0027, \u0027node\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3c7947fa","line":636,"in_reply_to":"3f79a3b5_b0525d69","updated":"2018-11-06 15:40:09.000000000","message":"@Chris, I had to pull it out for line length. The actual change is just s/remove_provider_from_instance_allocation/remove_provider_tree_from_instance_allocation/. There were probably other ways to do it.\n\n@Jay: There\u0027s a difference between mock.patch(\u0027path.to.meth\u0027) (which mocks any call to the method) and mock.patch.object(inst, \u0027meth\u0027) which mocks only the call to the method through that specific inst. In a lot of cases (as probably here), the difference is irrelevant. I\u0027m also not 100% sure the former will work through the lazy loader shim, but can try it.\n\nI did make an attempt to change as little as possible here, especially since it\u0027s a fix related to a completely separate patch.","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"244ec4e996c47213d5c5a55dc776e6cb900a8c5e","unresolved":false,"context_lines":[{"line_number":633,"context_line":"        \"\"\"Tests that failing to find a ComputeNode will not blow up"},{"line_number":634,"context_line":"        the _remove_host_allocations method."},{"line_number":635,"context_line":"        \"\"\""},{"line_number":636,"context_line":"        method \u003d \u0027remove_provider_tree_from_instance_allocation\u0027"},{"line_number":637,"context_line":"        with mock.patch.object(self.task.scheduler_client.reportclient,"},{"line_number":638,"context_line":"                               method) as remove_provider:"},{"line_number":639,"context_line":"            self.task._remove_host_allocations(\u0027host\u0027, \u0027node\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b0525d69","line":636,"in_reply_to":"3f79a3b5_c4796381","updated":"2018-11-06 14:06:56.000000000","message":"There\u0027s actually no reason to have an inner context manager patcher. The test method itself could just be decorated:\n\n @mock.patch(\u0027nova.scheduler.client.report.SchedulerReportClient.\u0027\n             \u0027remove_provider_tree_from_instance_allocation\u0027)\n def test_remove_host_allocations_compute_host_not_found(self, get_cn,\n         mock_remove_provider):\n\nThat way, we have all mocks simply decorating the test method instead of one mock decorating the test method and another mock set up using a context manager inside the method for some reason.","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"}],"nova/tests/unit/scheduler/test_client.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"33dff166e762158b9c3d465a3b8a645d2bdd3eeb","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    @mock.patch.object(scheduler_query_client.SchedulerQueryClient,"},{"line_number":39,"context_line":"                       \u0027select_destinations\u0027)"},{"line_number":40,"context_line":"    def test_select_destinations(self, mock_select_destinations):"},{"line_number":41,"context_line":"        fake_spec \u003d objects.RequestSpec()"},{"line_number":42,"context_line":"        fake_spec.instance_uuid \u003d uuids.instance"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64c08f7e","line":40,"updated":"2018-11-06 10:52:37.000000000","message":"You could probably kill this test entirely, as all it seems to be doing is confirming that python works, unless we really care to confirm that keyword args actually happen?\n\nI suppose this and the other modified test are basically confirming \"yes, my proxy methods do in fact proxy\".\n\nI suppose that\u0027s valid but suggests a future refactoring where get rid of the proxy/facade entirely.","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"244ec4e996c47213d5c5a55dc776e6cb900a8c5e","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    @mock.patch.object(scheduler_query_client.SchedulerQueryClient,"},{"line_number":39,"context_line":"                       \u0027select_destinations\u0027)"},{"line_number":40,"context_line":"    def test_select_destinations(self, mock_select_destinations):"},{"line_number":41,"context_line":"        fake_spec \u003d objects.RequestSpec()"},{"line_number":42,"context_line":"        fake_spec.instance_uuid \u003d uuids.instance"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b0aedd2e","line":40,"in_reply_to":"3f79a3b5_64c08f7e","updated":"2018-11-06 14:06:56.000000000","message":"+10.\n\nOr simply remove the entire abstraction layer of a base \"scheduler client\" (which isn\u0027t used at all anyway and just adds needless redirection).","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a0175a0b2f6eef63be81ef357e47873e9de85ebe","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    @mock.patch.object(scheduler_query_client.SchedulerQueryClient,"},{"line_number":39,"context_line":"                       \u0027select_destinations\u0027)"},{"line_number":40,"context_line":"    def test_select_destinations(self, mock_select_destinations):"},{"line_number":41,"context_line":"        fake_spec \u003d objects.RequestSpec()"},{"line_number":42,"context_line":"        fake_spec.instance_uuid \u003d uuids.instance"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_bcd237ce","line":40,"in_reply_to":"3f79a3b5_b0aedd2e","updated":"2018-11-06 15:40:09.000000000","message":"@Chris: We do have other dumb unit tests like this one that just test that the shim is calling the right client with the right args. If nothing else, the cover job would miss them if they were gone.\n\n@Jay: I have actually looked into removing the scheduler client before, and it was going to wind up being a nontrivial thing. In this case I was just trying to get to a point where I could access a (non-method) attribute of the report client for subsequent patches. But yeah, at some point I can dig back into getting rid of the thing.","commit_id":"4ebd2504bb27afac7184d254aa6556fe61493a5d"}]}