)]}'
{"nova/scheduler/utils.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"3d6f2105aaf9a5b83d526182370b4755120c6516","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    def _clean_empties(self):"},{"line_number":244,"context_line":"        \"\"\"Get rid of any empty ResourceGroup instances.\"\"\""},{"line_number":245,"context_line":"        for ident, rg in list(self._rg_by_id.items()):"},{"line_number":246,"context_line":"            if not any((rg.resources, rg.required_traits, rg.forbidden_traits)):"},{"line_number":247,"context_line":"                self._rg_by_id.pop(ident)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def strip_zeros(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_cf3db9bb","line":246,"updated":"2019-03-27 02:57:12.000000000","message":"This line must be 79 characters or less.","commit_id":"2233d69fdb89f3b146553c059de57cd425bc29f8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"1f247ce8eac09f7a31d01203043d042f40ffbbe6","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    def _clean_empties(self):"},{"line_number":244,"context_line":"        \"\"\"Get rid of any empty ResourceGroup instances.\"\"\""},{"line_number":245,"context_line":"        for ident, rg in list(self._rg_by_id.items()):"},{"line_number":246,"context_line":"            if not any((rg.resources, rg.required_traits, rg.forbidden_traits)):"},{"line_number":247,"context_line":"                self._rg_by_id.pop(ident)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def strip_zeros(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_06cc0233","line":246,"in_reply_to":"5fc1f717_83c8e454","updated":"2019-03-27 13:45:41.000000000","message":"If this is true, aggregate information (member_of) would need to be included as well, for the same reason.","commit_id":"2233d69fdb89f3b146553c059de57cd425bc29f8"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9b00440e2e042a9b46e977a158e50f3d7fed8d58","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    def _clean_empties(self):"},{"line_number":244,"context_line":"        \"\"\"Get rid of any empty ResourceGroup instances.\"\"\""},{"line_number":245,"context_line":"        for ident, rg in list(self._rg_by_id.items()):"},{"line_number":246,"context_line":"            if not any((rg.resources, rg.required_traits, rg.forbidden_traits)):"},{"line_number":247,"context_line":"                self._rg_by_id.pop(ident)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def strip_zeros(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_f459698d","line":246,"in_reply_to":"5fc1f717_cf3db9bb","updated":"2019-03-27 08:45:21.000000000","message":"If a group only has required or forbidden traits but not resources then that group is not valid from placement perspective so I don\u0027t know how adding forbidden_traits here fixes the issue. \n\nCould you add some functional test that reproduce the issue first and then show that this change fixes the problem?","commit_id":"2233d69fdb89f3b146553c059de57cd425bc29f8"},{"author":{"_account_id":23922,"name":"Magnus Bergman","email":"mb@citynetwork.se","username":"magnusbe"},"change_message_id":"91bfc1c47e1dc42a9ad6e3a616cfec11fd3ef777","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    def _clean_empties(self):"},{"line_number":244,"context_line":"        \"\"\"Get rid of any empty ResourceGroup instances.\"\"\""},{"line_number":245,"context_line":"        for ident, rg in list(self._rg_by_id.items()):"},{"line_number":246,"context_line":"            if not any((rg.resources, rg.required_traits, rg.forbidden_traits)):"},{"line_number":247,"context_line":"                self._rg_by_id.pop(ident)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def strip_zeros(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_63fa98bd","line":246,"in_reply_to":"5fc1f717_cf3db9bb","updated":"2019-03-27 12:57:35.000000000","message":"Sorry about that, will correct it.","commit_id":"2233d69fdb89f3b146553c059de57cd425bc29f8"},{"author":{"_account_id":23922,"name":"Magnus Bergman","email":"mb@citynetwork.se","username":"magnusbe"},"change_message_id":"91bfc1c47e1dc42a9ad6e3a616cfec11fd3ef777","unresolved":false,"context_lines":[{"line_number":243,"context_line":"    def _clean_empties(self):"},{"line_number":244,"context_line":"        \"\"\"Get rid of any empty ResourceGroup instances.\"\"\""},{"line_number":245,"context_line":"        for ident, rg in list(self._rg_by_id.items()):"},{"line_number":246,"context_line":"            if not any((rg.resources, rg.required_traits, rg.forbidden_traits)):"},{"line_number":247,"context_line":"                self._rg_by_id.pop(ident)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def strip_zeros(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_83c8e454","line":246,"in_reply_to":"5fc1f717_f459698d","updated":"2019-03-27 12:57:35.000000000","message":"At tthe time the resource request is passed to strip_zeros and then _clean_empties it seems to only contain the extra_specs and not yet resources:\n\nhttps://github.com/openstack/nova/blob/771e102654b54d257f437f16be2aeeef51fa0783/nova/scheduler/utils.py#L459","commit_id":"2233d69fdb89f3b146553c059de57cd425bc29f8"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":4523,"context_line":""},{"line_number":4524,"context_line":"    def test_flavor_traits_based_scheduling(self):"},{"line_number":4525,"context_line":"        \"\"\"Tests that a server create request using a required trait on flavor"},{"line_number":4526,"context_line":"        ends up in ERROR state when the resource provider with the required"},{"line_number":4527,"context_line":"        trait in placement is disabled."},{"line_number":4528,"context_line":"        \"\"\""},{"line_number":4529,"context_line":""},{"line_number":4530,"context_line":"        # Decorate compute1 resource provider with that same trait."}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_1fce5c2c","line":4527,"range":{"start_line":4526,"start_character":31,"end_line":4527,"end_character":38},"updated":"2019-03-29 09:47:55.000000000","message":"To be precise: \"when the compute host is disabled which has the required trait in placement.\"","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":4548,"context_line":"        self.assertIn(\u0027No valid host\u0027, server[\u0027fault\u0027][\u0027message\u0027])"},{"line_number":4549,"context_line":""},{"line_number":4550,"context_line":"    def test_flavor_forbidden_traits_based_scheduling(self):"},{"line_number":4551,"context_line":"        \"\"\"Tests that a server create request using a forbidden trait on flavor"},{"line_number":4552,"context_line":"        ends up in ERROR state when the resource provider without the"},{"line_number":4553,"context_line":"        forbidden trait in placement is disabled."},{"line_number":4554,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_5ff1446a","line":4551,"range":{"start_line":4551,"start_character":70,"end_line":4551,"end_character":72},"updated":"2019-03-29 09:47:55.000000000","message":"in the flavor","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":4549,"context_line":""},{"line_number":4550,"context_line":"    def test_flavor_forbidden_traits_based_scheduling(self):"},{"line_number":4551,"context_line":"        \"\"\"Tests that a server create request using a forbidden trait on flavor"},{"line_number":4552,"context_line":"        ends up in ERROR state when the resource provider without the"},{"line_number":4553,"context_line":"        forbidden trait in placement is disabled."},{"line_number":4554,"context_line":"        \"\"\""},{"line_number":4555,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_3fea3873","line":4552,"range":{"start_line":4552,"start_character":40,"end_line":4552,"end_character":57},"updated":"2019-03-29 09:47:55.000000000","message":"compute","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":4569,"context_line":"        )"},{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"        # The server should go to ERROR state because there is no valid host."},{"line_number":4572,"context_line":"        server \u003d self._wait_for_state_change(self.admin_api, server, \u0027ERROR\u0027)"},{"line_number":4573,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":4574,"context_line":"        # Make sure the failure was due to NoValidHost by checking the fault."},{"line_number":4575,"context_line":"        self.assertIn(\u0027fault\u0027, server)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_df7774d3","line":4572,"updated":"2019-03-29 09:47:55.000000000","message":"So this step will fail the test until the bug is fixed. \n\nTo be able to merge the reproduction test before the bugfix you need to:\n\n* comment this step out. and write in a comment that this is the expected behavior but the bug 1821824 makes it fail\n\n* add something like\n\n  server \u003d self._wait_for_state_change(self.admin_api, server, \u0027ACTIVE\u0027)\n\nto make the test pass until the bug is fixed, showing the _wrong_ behavior.","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"        # The server should go to ERROR state because there is no valid host."},{"line_number":4572,"context_line":"        server \u003d self._wait_for_state_change(self.admin_api, server, \u0027ERROR\u0027)"},{"line_number":4573,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":4574,"context_line":"        # Make sure the failure was due to NoValidHost by checking the fault."},{"line_number":4575,"context_line":"        self.assertIn(\u0027fault\u0027, server)"},{"line_number":4576,"context_line":"        self.assertIn(\u0027No valid host\u0027, server[\u0027fault\u0027][\u0027message\u0027])"},{"line_number":4577,"context_line":""},{"line_number":4578,"context_line":"    def test_image_traits_based_scheduling(self):"},{"line_number":4579,"context_line":"        \"\"\"Tests that a server create request using a required trait on image"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_9f7d6cb3","line":4576,"range":{"start_line":4573,"start_character":0,"end_line":4576,"end_character":66},"updated":"2019-03-29 09:47:55.000000000","message":"Comment this out in this patch, and uncomment it in the patch where the bug is fixed.","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"1c2abdd13ebe2915171eeaa7c20dc8e4d256f0b9","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":4546,"context_line":"        # Make sure the failure was due to NoValidHost by checking the fault."},{"line_number":4547,"context_line":"        self.assertIn(\u0027fault\u0027, server)"},{"line_number":4548,"context_line":"        self.assertIn(\u0027No valid host\u0027, server[\u0027fault\u0027][\u0027message\u0027])"},{"line_number":4549,"context_line":""},{"line_number":4550,"context_line":"    def test_flavor_forbidden_traits_based_scheduling(self):"},{"line_number":4551,"context_line":"        \"\"\"Tests that a server create request using a forbidden trait in the"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_5c870914","line":4548,"updated":"2019-03-29 16:06:43.000000000","message":"So... I love this test case, but I think it should be present *in addition* to the one that was here before. Otherwise, we don\u0027t have any positive assertion that the trait makes the instance land in the right place.\n\nYou can tack it onto this same test method if you like, or do it separately.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"},{"author":{"_account_id":23922,"name":"Magnus Bergman","email":"mb@citynetwork.se","username":"magnusbe"},"change_message_id":"ab39eab38a3450f7b6ba237e9b3228e5fad1494e","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":4546,"context_line":"        # Make sure the failure was due to NoValidHost by checking the fault."},{"line_number":4547,"context_line":"        self.assertIn(\u0027fault\u0027, server)"},{"line_number":4548,"context_line":"        self.assertIn(\u0027No valid host\u0027, server[\u0027fault\u0027][\u0027message\u0027])"},{"line_number":4549,"context_line":""},{"line_number":4550,"context_line":"    def test_flavor_forbidden_traits_based_scheduling(self):"},{"line_number":4551,"context_line":"        \"\"\"Tests that a server create request using a forbidden trait in the"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3b455916","line":4548,"in_reply_to":"5fc1f717_3c023d71","updated":"2019-03-30 17:27:54.000000000","message":"Good points. Will modify as suggested.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"22047930e5615eb80b1148fa676607b8f4317920","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-SRV-ATTR:host\u0027])"},{"line_number":4546,"context_line":"        # Make sure the failure was due to NoValidHost by checking the fault."},{"line_number":4547,"context_line":"        self.assertIn(\u0027fault\u0027, server)"},{"line_number":4548,"context_line":"        self.assertIn(\u0027No valid host\u0027, server[\u0027fault\u0027][\u0027message\u0027])"},{"line_number":4549,"context_line":""},{"line_number":4550,"context_line":"    def test_flavor_forbidden_traits_based_scheduling(self):"},{"line_number":4551,"context_line":"        \"\"\"Tests that a server create request using a forbidden trait in the"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3c023d71","line":4548,"in_reply_to":"5fc1f717_5c870914","updated":"2019-03-29 16:13:13.000000000","message":"gibi suggested (and I agree) that the positive path should include a comment pointing out that it could succeed half the time by chance. That being the case, I think it would be best to put both paths into this one test method. I was able to do this locally by simply tacking L4534-48 onto the end of the method as it existed before.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"1c2abdd13ebe2915171eeaa7c20dc8e4d256f0b9","unresolved":false,"context_lines":[{"line_number":4566,"context_line":"        server \u003d self._create_server_with_traits("},{"line_number":4567,"context_line":"            self.flavor_with_forbidden_trait[\u0027id\u0027],"},{"line_number":4568,"context_line":"            self.image_id_without_trait"},{"line_number":4569,"context_line":"        )"},{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"        # Temporarily pass if server goes to ACTIVE STATE until bug #1821824"},{"line_number":4572,"context_line":"        # is fixed."}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_fc77d53a","line":4569,"updated":"2019-03-29 16:06:43.000000000","message":"same thing here: we need both positive and negative assertions.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"},{"author":{"_account_id":23922,"name":"Magnus Bergman","email":"mb@citynetwork.se","username":"magnusbe"},"change_message_id":"ab39eab38a3450f7b6ba237e9b3228e5fad1494e","unresolved":false,"context_lines":[{"line_number":4566,"context_line":"        server \u003d self._create_server_with_traits("},{"line_number":4567,"context_line":"            self.flavor_with_forbidden_trait[\u0027id\u0027],"},{"line_number":4568,"context_line":"            self.image_id_without_trait"},{"line_number":4569,"context_line":"        )"},{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"        # Temporarily pass if server goes to ACTIVE STATE until bug #1821824"},{"line_number":4572,"context_line":"        # is fixed."}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_bb62a9ac","line":4569,"in_reply_to":"5fc1f717_3c9b1d67","updated":"2019-03-30 17:27:54.000000000","message":"Will modify as suggested.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"980221af3d7268d782431d9710c3c3e85de44785","unresolved":false,"context_lines":[{"line_number":4566,"context_line":"        server \u003d self._create_server_with_traits("},{"line_number":4567,"context_line":"            self.flavor_with_forbidden_trait[\u0027id\u0027],"},{"line_number":4568,"context_line":"            self.image_id_without_trait"},{"line_number":4569,"context_line":"        )"},{"line_number":4570,"context_line":""},{"line_number":4571,"context_line":"        # Temporarily pass if server goes to ACTIVE STATE until bug #1821824"},{"line_number":4572,"context_line":"        # is fixed."}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3c9b1d67","line":4569,"in_reply_to":"5fc1f717_fc77d53a","updated":"2019-03-29 16:08:20.000000000","message":"...though in this case, since the bug is present, you won\u0027t be able to assert the broken behavior for the positive test case, because that *would* succeed half the time.","commit_id":"edc130bc226e0e073b9fa102e8e77d6ba0834440"}],"nova/tests/unit/scheduler/test_utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"31c8a771c50aa352837046c3e685371ddf983428","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                \u0027CUSTOM_FLAVOR_TRAIT\u0027,"},{"line_number":125,"context_line":"            }"},{"line_number":126,"context_line":"        )"},{"line_number":127,"context_line":"        self._test_resources_from_request_spec(expected_resources, flavor)"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def test_resources_from_request_spec_with_no_disk(self):"},{"line_number":130,"context_line":"        flavor \u003d objects.Flavor(vcpus\u003d1,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_7f3520f6","line":127,"updated":"2019-03-29 09:47:55.000000000","message":"This also fails the test until the bug is fixed. Either you do a similar thing that I suggested for the functional test. Or simply move this unit test to the bugfix patch as on reproduction functional test is enough for this patch.","commit_id":"7593ddcdab4022299476d2ed6d39774fca49b579"}]}