)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fae207f1d8fc3f5bec6876a1a6f26e6266dd2a6e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1684af4c_9a1b15ee","updated":"2022-02-03 03:08:11.000000000","message":"Only noticed one small thing inline.","commit_id":"005b59b7085586a968b375aa76f4bed5010e3ebf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"64d97fe4321d046dbc27c89e952b1a447b9d4cb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"dc5075b7_66a3e839","updated":"2022-02-03 21:27:32.000000000","message":"This LGTM","commit_id":"7e3602b35f1919a9693b2e55669816671d0d1c5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3c6d3f87155a4c5fbe766970641aa57ea81d1417","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e1537154_d9f3621a","updated":"2022-02-09 19:21:17.000000000","message":"i agree with melanie that this looks good to proceed with.\nher main issue regarding the set() vs {} initalisation has been adressed and\nthe legacy buggy behavior is unchanged.\n\ni will try and review some of the rest of the serise tomorrow but im going to call it a day here.","commit_id":"7e3602b35f1919a9693b2e55669816671d0d1c5f"}],"placement/handlers/resource_provider.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"28dc438c5ed8b27c5dc785efcd03aa117bb3cc6d","unresolved":true,"context_lines":[{"line_number":249,"context_line":"            elif attr \u003d\u003d \u0027required\u0027:"},{"line_number":250,"context_line":"                value \u003d util.normalize_traits_qs_param("},{"line_number":251,"context_line":"                    value, allow_forbidden\u003dallow_forbidden)"},{"line_number":252,"context_line":"            filters[attr] \u003d value"},{"line_number":253,"context_line":"    try:"},{"line_number":254,"context_line":"        resource_providers \u003d rp_obj.get_all_by_filters(context, filters)"},{"line_number":255,"context_line":"    except exception.ResourceClassNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"184e84f6_beea7426","side":"PARENT","line":252,"updated":"2022-01-24 12:44:04.000000000","message":"ok so we can remove the version check for forbiden traits since we are movign the processing fo the required key out of this loop entirely","commit_id":"e6be27332726ac0a0bc02d62fdaa6501c6e48f37"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7b5e572cc0deada97d7eb3ab738c846932e9d4","unresolved":false,"context_lines":[{"line_number":249,"context_line":"            elif attr \u003d\u003d \u0027required\u0027:"},{"line_number":250,"context_line":"                value \u003d util.normalize_traits_qs_param("},{"line_number":251,"context_line":"                    value, allow_forbidden\u003dallow_forbidden)"},{"line_number":252,"context_line":"            filters[attr] \u003d value"},{"line_number":253,"context_line":"    try:"},{"line_number":254,"context_line":"        resource_providers \u003d rp_obj.get_all_by_filters(context, filters)"},{"line_number":255,"context_line":"    except exception.ResourceClassNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cc17a8a3_f708d90d","side":"PARENT","line":252,"in_reply_to":"184e84f6_beea7426","updated":"2022-02-02 16:43:13.000000000","message":"Ack","commit_id":"e6be27332726ac0a0bc02d62fdaa6501c6e48f37"}],"placement/tests/unit/test_util.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"28dc438c5ed8b27c5dc785efcd03aa117bb3cc6d","unresolved":true,"context_lines":[{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        self.assertEqual({\u0027FOO\u0027, \u0027BAR\u0027}, traits)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"    def test_allow_forbidden_1_21(self):"},{"line_number":463,"context_line":"        req \u003d self._get_req(\u0027required\u003d!BAZ\u0027, (1, 21))"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"        ex \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":1,"id":"6c20a845_d4a31847","line":462,"updated":"2022-01-24 12:44:04.000000000","message":"nit: test_reject_forbidden_1_21\n\nsince we are asserting its invalid","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7b5e572cc0deada97d7eb3ab738c846932e9d4","unresolved":false,"context_lines":[{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        self.assertEqual({\u0027FOO\u0027, \u0027BAR\u0027}, traits)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"    def test_allow_forbidden_1_21(self):"},{"line_number":463,"context_line":"        req \u003d self._get_req(\u0027required\u003d!BAZ\u0027, (1, 21))"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"        ex \u003d self.assertRaises("}],"source_content_type":"text/x-python","patch_set":1,"id":"97361586_33f549c9","line":462,"in_reply_to":"6c20a845_d4a31847","updated":"2022-02-02 16:43:13.000000000","message":"this is not part of my change. If I have to respin this then I will fix it otherwise I can push a separate trivial fix for it.","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"28dc438c5ed8b27c5dc785efcd03aa117bb3cc6d","unresolved":true,"context_lines":[{"line_number":488,"context_line":"        traits \u003d util.normalize_traits_qs_params(req, suffix\u003d\u0027\u0027)"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        self.assertEqual({\u0027BAZ\u0027}, traits)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"class TestParseQsRequestGroups(testtools.TestCase):"},{"line_number":494,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fa6922b1_a68ee8e3","line":491,"updated":"2022-01-24 12:44:04.000000000","message":"so i understand why you have define this thest this way buyt i kind fo fell\nlike this is a bug and we should be returning a 400 bad request.\n\nusing the last value is consitent with what the code is doing now but i feel like\nit not intuitive adn there might be silent bugs as a result so i think we should be returning a 400 to signel that to the end user.\n\ncan we do that without a micorverions bump? if not then this behaior is the correct one for older microverions","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7b5e572cc0deada97d7eb3ab738c846932e9d4","unresolved":false,"context_lines":[{"line_number":488,"context_line":"        traits \u003d util.normalize_traits_qs_params(req, suffix\u003d\u0027\u0027)"},{"line_number":489,"context_line":""},{"line_number":490,"context_line":"        self.assertEqual({\u0027BAZ\u0027}, traits)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":""},{"line_number":493,"context_line":"class TestParseQsRequestGroups(testtools.TestCase):"},{"line_number":494,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"050764d5_f762d761","line":491,"in_reply_to":"fa6922b1_a68ee8e3","updated":"2022-02-02 16:43:13.000000000","message":"Filed a separate bug and fixed the repeat issue in https://review.opendev.org/c/openstack/placement/+/827115","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"}],"placement/util.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"28dc438c5ed8b27c5dc785efcd03aa117bb3cc6d","unresolved":true,"context_lines":[{"line_number":353,"context_line":"    \"\"\""},{"line_number":354,"context_line":"    want_version \u003d req.environ[placement.microversion.MICROVERSION_ENVIRON]"},{"line_number":355,"context_line":"    allow_forbidden \u003d want_version.matches((1, 22))"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"}],"source_content_type":"text/x-python","patch_set":1,"id":"61fbca67_9d4d76d6","line":356,"updated":"2022-01-24 12:44:04.000000000","message":"ok and here we still do the forbiden version check so no change in behavior.","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7b5e572cc0deada97d7eb3ab738c846932e9d4","unresolved":false,"context_lines":[{"line_number":353,"context_line":"    \"\"\""},{"line_number":354,"context_line":"    want_version \u003d req.environ[placement.microversion.MICROVERSION_ENVIRON]"},{"line_number":355,"context_line":"    allow_forbidden \u003d want_version.matches((1, 22))"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"}],"source_content_type":"text/x-python","patch_set":1,"id":"56d9b0a7_0c46112a","line":356,"in_reply_to":"61fbca67_9d4d76d6","updated":"2022-02-02 16:43:13.000000000","message":"Ack","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"28dc438c5ed8b27c5dc785efcd03aa117bb3cc6d","unresolved":true,"context_lines":[{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"},{"line_number":360,"context_line":"    # the last one will be considered"},{"line_number":361,"context_line":"    for value in req.GET.getall(\u0027required\u0027 + suffix):"},{"line_number":362,"context_line":"        traits \u003d normalize_traits_qs_param(value, allow_forbidden)"},{"line_number":363,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"47d7c04a_8290a535","line":360,"updated":"2022-01-24 12:44:04.000000000","message":"that is the exisiting behvhior but i wonder if that was intentional or a bug orginally\n\nin any case you are maintaining the behaivor for now so +1","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a7b5e572cc0deada97d7eb3ab738c846932e9d4","unresolved":false,"context_lines":[{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"},{"line_number":360,"context_line":"    # the last one will be considered"},{"line_number":361,"context_line":"    for value in req.GET.getall(\u0027required\u0027 + suffix):"},{"line_number":362,"context_line":"        traits \u003d normalize_traits_qs_param(value, allow_forbidden)"},{"line_number":363,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"61344ebf_277e2f69","line":360,"in_reply_to":"47d7c04a_8290a535","updated":"2022-02-02 16:43:13.000000000","message":"Ack","commit_id":"7d40b25ff4e4d800ad8439b42d3b82f7426183dd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fae207f1d8fc3f5bec6876a1a6f26e6266dd2a6e","unresolved":true,"context_lines":[{"line_number":354,"context_line":"    want_version \u003d req.environ[placement.microversion.MICROVERSION_ENVIRON]"},{"line_number":355,"context_line":"    allow_forbidden \u003d want_version.matches((1, 22))"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"},{"line_number":360,"context_line":"    # the last one will be considered"}],"source_content_type":"text/x-python","patch_set":4,"id":"00e7a62a_7d96e7a3","line":357,"range":{"start_line":357,"start_character":13,"end_line":357,"end_character":15},"updated":"2022-02-03 03:08:11.000000000","message":"set() ? I realize that this will probably never be called without \u0027required\u0027 in qparams but if it were, it would not return a set.","commit_id":"005b59b7085586a968b375aa76f4bed5010e3ebf"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f9a008ea610ffc08d934f1a497d9b62b5f65d834","unresolved":false,"context_lines":[{"line_number":354,"context_line":"    want_version \u003d req.environ[placement.microversion.MICROVERSION_ENVIRON]"},{"line_number":355,"context_line":"    allow_forbidden \u003d want_version.matches((1, 22))"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"},{"line_number":360,"context_line":"    # the last one will be considered"}],"source_content_type":"text/x-python","patch_set":4,"id":"9d9e11f1_e0dac3c2","line":357,"range":{"start_line":357,"start_character":13,"end_line":357,"end_character":15},"in_reply_to":"00e7a62a_7d96e7a3","updated":"2022-02-03 11:03:48.000000000","message":"Yes it should be a set(). This is a bug. Probably a non visible one as in an iterable context a dict behaves like a set of keys. Anyhow I fixed it now. Thanks for catching.","commit_id":"005b59b7085586a968b375aa76f4bed5010e3ebf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3c6d3f87155a4c5fbe766970641aa57ea81d1417","unresolved":false,"context_lines":[{"line_number":354,"context_line":"    want_version \u003d req.environ[placement.microversion.MICROVERSION_ENVIRON]"},{"line_number":355,"context_line":"    allow_forbidden \u003d want_version.matches((1, 22))"},{"line_number":356,"context_line":""},{"line_number":357,"context_line":"    traits \u003d {}"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"    # NOTE(gibi): This means if the same query param is repeated then only"},{"line_number":360,"context_line":"    # the last one will be considered"}],"source_content_type":"text/x-python","patch_set":4,"id":"75a4a731_2576c826","line":357,"range":{"start_line":357,"start_character":13,"end_line":357,"end_character":15},"in_reply_to":"9d9e11f1_e0dac3c2","updated":"2022-02-09 19:21:17.000000000","message":"good spot +1","commit_id":"005b59b7085586a968b375aa76f4bed5010e3ebf"}]}