)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"669b26c27b67526fb3a048927fe6153ff5534398","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3da77c96_7faa8b2c","updated":"2024-09-12 10:28:56.000000000","message":"Looks good. One nit inline. I\u0027d also like to see a release note and a functional test. If you can add that I should be +2","commit_id":"4b1445abb6f5be0d6af9cf48ea4ece8e3d6b8abd"},{"author":{"_account_id":37207,"name":"Yoonho Hann","display_name":"yoonhohann","email":"hnnynh125@gmail.com","username":"hnnynh"},"change_message_id":"ce69d08c2e2d443d937f00a45bdda878d5a53118","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"75c77fb4_3f449c34","in_reply_to":"3da77c96_7faa8b2c","updated":"2024-09-13 07:02:38.000000000","message":"- Add a release note and a functional test.\nI\u0027ve verified that the service name and status are correct in the functional test. Pl\bz check if I\u0027ve missed any services in the designate system.","commit_id":"4b1445abb6f5be0d6af9cf48ea4ece8e3d6b8abd"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7fc3a7c5410d9e2d4ac53bbfd46eacdde3ec1bfd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ba6c3c1f_cb0122a2","updated":"2024-09-13 09:51:44.000000000","message":"Two comments on the new functional test. This is almost there 😊","commit_id":"bbf5dbdccf67ca2ae9483b57297adccffd2a12d4"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"7d7d4682ca0ce77bd4538db196f2f417fc8e0b1f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0b75a525_52f1abcf","updated":"2024-09-17 10:03:15.000000000","message":"2 new methods has been added to the proxy but proxy tests are missed (https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/tests/unit/dns/v2/test_proxy.py)","commit_id":"494db0bd854a9fed26df4fc8c296f7f251f4d8bf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2faeacc8e2204ef6a51ba667957994e23daf2fea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d03b190b_61fd44a7","updated":"2024-09-13 16:23:55.000000000","message":"Thank you. This looks good to me now. Nice work! 👏","commit_id":"494db0bd854a9fed26df4fc8c296f7f251f4d8bf"},{"author":{"_account_id":37207,"name":"Yoonho Hann","display_name":"yoonhohann","email":"hnnynh125@gmail.com","username":"hnnynh"},"change_message_id":"9f574656aeb084126cf1e08cd24d042e507eeca4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4b01b84d_dc5c44b5","in_reply_to":"0b75a525_52f1abcf","updated":"2024-09-18 00:41:23.000000000","message":"Added a missed test. Thank you for finding something missed.","commit_id":"494db0bd854a9fed26df4fc8c296f7f251f4d8bf"}],"doc/source/user/resources/dns/v2/service_status.rst":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"669b26c27b67526fb3a048927fe6153ff5534398","unresolved":true,"context_lines":[{"line_number":6,"context_line":"The ServiceStatus Class"},{"line_number":7,"context_line":"-----------------------"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The ``DNS`` class inherits from :class:`~openstack.resource.Resource`."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":".. autoclass:: openstack.dns.v2.service_status.ServiceStatus"},{"line_number":12,"context_line":"   :members:"}],"source_content_type":"text/x-rst","patch_set":4,"id":"568e2d41_faecf7b9","line":9,"range":{"start_line":9,"start_character":6,"end_line":9,"end_character":9},"updated":"2024-09-12 10:28:56.000000000","message":"```suggestion\nThe ``ServiceStatus`` class inherits from :class:`~openstack.resource.Resource`.\n```\n\nRight?","commit_id":"4b1445abb6f5be0d6af9cf48ea4ece8e3d6b8abd"},{"author":{"_account_id":37207,"name":"Yoonho Hann","display_name":"yoonhohann","email":"hnnynh125@gmail.com","username":"hnnynh"},"change_message_id":"ce69d08c2e2d443d937f00a45bdda878d5a53118","unresolved":false,"context_lines":[{"line_number":6,"context_line":"The ServiceStatus Class"},{"line_number":7,"context_line":"-----------------------"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The ``DNS`` class inherits from :class:`~openstack.resource.Resource`."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":".. autoclass:: openstack.dns.v2.service_status.ServiceStatus"},{"line_number":12,"context_line":"   :members:"}],"source_content_type":"text/x-rst","patch_set":4,"id":"788901c3_6a70302b","line":9,"range":{"start_line":9,"start_character":6,"end_line":9,"end_character":9},"in_reply_to":"568e2d41_faecf7b9","updated":"2024-09-13 07:02:38.000000000","message":"I missed that, thanks for pointing it out!","commit_id":"4b1445abb6f5be0d6af9cf48ea4ece8e3d6b8abd"}],"openstack/tests/functional/dns/v2/test_service_status.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7fc3a7c5410d9e2d4ac53bbfd46eacdde3ec1bfd","unresolved":true,"context_lines":[{"line_number":22,"context_line":"        self.conn \u003d connection.from_config(cloud_name\u003dbase.TEST_CLOUD_NAME)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        self.services \u003d self.conn.dns.service_statuses()"},{"line_number":25,"context_line":"        if self.services is None:"},{"line_number":26,"context_line":"            self.skipTest("},{"line_number":27,"context_line":"                \"The Service in Designate System is required for this test\""},{"line_number":28,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":6,"id":"47dfa4b1_a5ad00bf","line":25,"updated":"2024-09-13 09:51:44.000000000","message":"I don\u0027t think this will ever be true? `services_statuses` (and all other \"list\" operations) returns a generator. You need to iterate through this generator to see if it\u0027s None. For example:\n\n```\ndef foo(x):\n    if x:\n        yield from x\n    else:\n        return None\n\nprint(foo(None) is None)\n```\n\nIf you run that, you\u0027ll see `False` instead of `True`. If you modify it like so:\n\n```\nprint(not list(foo(None)))\n```\n\nYou\u0027ll get something closer to what you expected.","commit_id":"bbf5dbdccf67ca2ae9483b57297adccffd2a12d4"},{"author":{"_account_id":37207,"name":"Yoonho Hann","display_name":"yoonhohann","email":"hnnynh125@gmail.com","username":"hnnynh"},"change_message_id":"042ba44c566a541b03ce5b1e76cbd51d94f720e8","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        self.conn \u003d connection.from_config(cloud_name\u003dbase.TEST_CLOUD_NAME)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"        self.services \u003d self.conn.dns.service_statuses()"},{"line_number":25,"context_line":"        if self.services is None:"},{"line_number":26,"context_line":"            self.skipTest("},{"line_number":27,"context_line":"                \"The Service in Designate System is required for this test\""},{"line_number":28,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a0af3e9_22539ba9","line":25,"in_reply_to":"47dfa4b1_a5ad00bf","updated":"2024-09-13 14:55:31.000000000","message":"I checked the generator return but missed to fix the code written just before. Fixed it thanks to you 😄","commit_id":"bbf5dbdccf67ca2ae9483b57297adccffd2a12d4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7fc3a7c5410d9e2d4ac53bbfd46eacdde3ec1bfd","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        self.assertTrue("},{"line_number":55,"context_line":"            all(status in self.service_status for status in statuses)"},{"line_number":56,"context_line":"        )"},{"line_number":57,"context_line":"        self.assertTrue(all(name in self.service_names for name in names))"}],"source_content_type":"text/x-python","patch_set":6,"id":"1c6cabc2_8c7e59f0","line":57,"updated":"2024-09-13 09:51:44.000000000","message":"This is good, but would it be possible to combine these into one test? Normally what you have done would be better, but functional tests are slow and we have tened to only have a single test class for functional tests of late to try speed things up. For example:\n\n```\ndef test_services_status(self):\n    # list service statuses\n\n    service_statuses \u003d list(self.conn.dns.service_statuses())\n    if not service_statuses:\n        self.skip(...)\n\n    # do checks for name, status\n    ...\n\n    # fetch service status\n\n    ...\n```","commit_id":"bbf5dbdccf67ca2ae9483b57297adccffd2a12d4"},{"author":{"_account_id":37207,"name":"Yoonho Hann","display_name":"yoonhohann","email":"hnnynh125@gmail.com","username":"hnnynh"},"change_message_id":"042ba44c566a541b03ce5b1e76cbd51d94f720e8","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.assertTrue("},{"line_number":55,"context_line":"            all(status in self.service_status for status in statuses)"},{"line_number":56,"context_line":"        )"},{"line_number":57,"context_line":"        self.assertTrue(all(name in self.service_names for name in names))"}],"source_content_type":"text/x-python","patch_set":6,"id":"f8809ded_11486fb8","line":57,"in_reply_to":"1c6cabc2_8c7e59f0","updated":"2024-09-13 14:55:31.000000000","message":"I realized that the difference between the intentions of the two tests was not significant. I agreed that combing the two tests would be better for performance.","commit_id":"bbf5dbdccf67ca2ae9483b57297adccffd2a12d4"}]}
