)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d498d789eb58722aec3da7c78e2d2a7f55a06ea8","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch modifies the check such way it counts only consumers"},{"line_number":17,"context_line":"presented in the allocation table."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"related-bug: #1780799"},{"line_number":20,"context_line":"Change-Id: I569043dd9f1c964faf7d4fcbb901a93f9569bf3d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1f621f24_d64f4232","line":18,"updated":"2020-11-17 22:08:20.000000000","message":"Concept makes sense (this recently came up on the ML) but we should have a test that verifies this change does what\u0027s intended.","commit_id":"b36e6e63779fc6de9a02ff91119f4fa77c174b63"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"59411a691a4c135ae854dced2ee1b6a8f0a5cb9f","unresolved":false,"context_lines":[{"line_number":12,"context_line":"consumer table those are not in the allocation table. The second case"},{"line_number":13,"context_line":"it seems was not expected and it can be tricky to automate a cleanup"},{"line_number":14,"context_line":"with zero touch upgrade."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch modifies the check such way it counts only consumers"},{"line_number":17,"context_line":"presented in the allocation table."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fffc6b78_a3186281","line":15,"updated":"2020-11-19 20:06:04.000000000","message":"Note to reviewers: this patch will resolve the issue described on the ML [1] where due to bug #1780799 users could encounter a result from the \u0027placement-status upgrade check\u0027 where the number of incomplete consumers found can be negative.\n\nThis is obviously confusing for users and in order to make the status check happy, they have to manually clean up old rows that were left behind by bug #1780799. While such a cleanup is something nice/good to do anyway, having the orphaned rows does not pose any harm to the upgrade or operation of the system (generally speaking -- if you have sufficiently too many orphaned rows, you could see DB performance degradation). \n\nBecause the orphaned rows are not actively harmful for the upgrade or deployment, I think it would be better that they not be flagged as such.\n\nThis patch removes orphans from consideration when checking for incomplete consumers, so that the status check will only find legit incomplete consumers that could cause problems for the upgraded deployment.\n\n[1] http://lists.openstack.org/pipermail/openstack-discuss/2020-October/018253.html","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"}],"placement/tests/functional/db/test_consumer.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eae48c8d5dc385b14b74072899485f7579e79f4b","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        self.ctx \u003d self.context"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":154,"context_line":"    def _check_incomplete_consumers(self, ctx):"},{"line_number":155,"context_line":"        config \u003d ctx.config"},{"line_number":156,"context_line":"        incomplete_project_id \u003d config.placement.incomplete_consumer_project_id"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f621f24_414fe191","line":154,"range":{"start_line":154,"start_character":8,"end_line":154,"end_character":35},"updated":"2020-11-19 01:17:41.000000000","message":"Unrelated: I noticed this is not used anywhere. Looks like its use was removed [1] but this bit was missed.\n\n[1] https://github.com/openstack/placement/commit/01e699152b99eacf61df72cac344f7276bd9c00a","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"f603f8a07a94410eb18546189daefc4f5d42e865","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        self.ctx \u003d self.context"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @db_api.placement_context_manager.reader"},{"line_number":154,"context_line":"    def _check_incomplete_consumers(self, ctx):"},{"line_number":155,"context_line":"        config \u003d ctx.config"},{"line_number":156,"context_line":"        incomplete_project_id \u003d config.placement.incomplete_consumer_project_id"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fffc6b78_8c5bdb85","line":154,"range":{"start_line":154,"start_character":8,"end_line":154,"end_character":35},"in_reply_to":"1f621f24_414fe191","updated":"2020-11-19 07:20:00.000000000","message":"Yes, looks like that. Will cleanup an a follow-up.","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eae48c8d5dc385b14b74072899485f7579e79f4b","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        res \u003d _get_allocs_with_no_consumer_relationship(ctx)"},{"line_number":188,"context_line":"        self.assertEqual(0, len(res))"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_incomplete_consumers(self):"},{"line_number":191,"context_line":"        \"\"\"Test the online data migration that creates incomplete consumer"},{"line_number":192,"context_line":"        records along with the incomplete consumer project/user records."},{"line_number":193,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"fffc6b78_6613360f","line":190,"updated":"2020-11-19 01:17:41.000000000","message":"I pulled down this patch locally and found this test does not fail without your fix. This test does not actually run the status check code at all, only the code to create incomplete consumers.\n\nI think this is the test you will want to add the corner case to instead:\n\nhttps://github.com/openstack/placement/blob/c4fb76511f94c76e63a911925e4d0bab5fbab093/placement/tests/functional/cmd/test_status.py#L37","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"f603f8a07a94410eb18546189daefc4f5d42e865","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        res \u003d _get_allocs_with_no_consumer_relationship(ctx)"},{"line_number":188,"context_line":"        self.assertEqual(0, len(res))"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_incomplete_consumers(self):"},{"line_number":191,"context_line":"        \"\"\"Test the online data migration that creates incomplete consumer"},{"line_number":192,"context_line":"        records along with the incomplete consumer project/user records."},{"line_number":193,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"fffc6b78_ece0a799","line":190,"in_reply_to":"fffc6b78_6613360f","updated":"2020-11-19 07:20:00.000000000","message":"That\u0027s interesting. What I have is http://ix.io/2EET.\n\nThere may be confusion between:\n1) placement.tests.functional.cmd.test_status.UpgradeCheckIncompleteConsumersTestCase.test_check_incomplete_consumers\n2) placement.tests.functional.db.test_consumer.CreateIncompleteConsumersTestCase.test_create_incomplete_consumers\n\nI thought about 1)","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9d001059060fe7df0450816075caf9f71534bcd","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        res \u003d _get_allocs_with_no_consumer_relationship(ctx)"},{"line_number":188,"context_line":"        self.assertEqual(0, len(res))"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def test_create_incomplete_consumers(self):"},{"line_number":191,"context_line":"        \"\"\"Test the online data migration that creates incomplete consumer"},{"line_number":192,"context_line":"        records along with the incomplete consumer project/user records."},{"line_number":193,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"fffc6b78_27a5a8b6","line":190,"in_reply_to":"fffc6b78_ece0a799","updated":"2020-11-19 08:33:05.000000000","message":"Oh no, this was my mistake, I completely missed that test_check_incomplete_consumers makes use of the same CreateIncompleteAllocationsMixin. /o\\ I do get the same result as you when I run the _correct_ test. I am so sorry!","commit_id":"da8b06b68f37bbe169d2ba40d79c990a1da50cac"}]}