)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"61d5e26e5e38b74fd651b3cd64ff476c154bf5a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"dd9f3b96_3083ae49","updated":"2023-03-03 08:57:35.000000000","message":"From here on out I am going to stop to re-evaluate what is going on. On this change I have added new schemas for \u0027lock\u0027, \u0027shelve\u0027 and \u0027migrate\u0027 that force every message to at the very least follow a format such as the following:\n{\n    \"lock\": null\n}\nThis makes the following message no longer valid:\n{\n    \"lock\": {}\n}\nNow, I have seen many services","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"31a40623281bc5afe7dde5a4dc15822566e5d17a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"d1ea772f_70c9b63a","updated":"2023-03-03 01:19:46.000000000","message":"I think we need to make a call on whether we want to continue allowing {}. The danger here is that by retroactively being strict about our schema, we might end up breaking clients that were incorrectly using {}. I\u0027ve added Ghanshyam to the review, I\u0027d like his opinion.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"d72dd6fde7f27ebfdab5fa25c03de0ccc4a09793","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"25f0b1b4_39c52d48","updated":"2023-03-02 15:24:56.000000000","message":"The test failures on PS11 look legit - looks as though a bunch of test code uses {} instead of None for the body. Because we had no validation, I have no way of knowing what the correct form is, but looking at the client code, None seems to be what should be sent.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"b9a0ff6e745984d26b974f3e55d98ce530d761ac","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"f19907da_1eb341c0","in_reply_to":"25f0b1b4_39c52d48","updated":"2023-03-02 15:38:19.000000000","message":"I saw other services allow {} to be sent and many unit tests give that for granted, so in the end I accepted both. At the very least, the case described on the bug ticket no longer passes and fails with a 400 error.\n\nIf we also want to get rid of {}, I would move so to another change because it would fat this one a lot. There are many tests that need to be tweaked for it.\n\nI am waiting for results for patch 12 to come in, but everything should be up and running there.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"dc470ca030157bdc7a78eba0dc2c7d9f803711b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"ed47560c_24235159","in_reply_to":"30122ade_85bd2526","updated":"2023-03-02 15:50:04.000000000","message":"Last failing trigger was from patch 10, I just got one from the arm pipeline on patch 12 that looks more promising.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"fa6cac7a1d5b822b2861b15a34f9ccbdf32c6674","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"bf6ff2dd_8e4fa6e7","in_reply_to":"dd9f3b96_3083ae49","updated":"2023-03-03 09:00:34.000000000","message":"Please ignore, just an old message from when I found trouble with {} that I forgot to remove from my buffer.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b549d2284785c69d36ff44ef28734ca962f14184","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"30122ade_85bd2526","in_reply_to":"f19907da_1eb341c0","updated":"2023-03-02 15:42:40.000000000","message":"\u003e I am waiting for results for patch 12 to come in, but everything should be up and running there.\n\nGerrit is saying PS12 is just PS11 with a commit message update, so I\u0027m not sure how tests getting fixed is possible...","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f77e5a954256536c8b8b370374639cc16719c2f1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"086780aa_27c1a067","updated":"2023-03-11 01:00:35.000000000","message":"I agree current behaviour is not best but the change you are proposing is backward incompatible change and has to be proposed with new microvesion. I explained why we have this behavour for action APIs and why changing it will break users even they are using it wrongly (I should not say wrongly as we did not document/coded that is wring behaviour).","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"1d15156c81203d14bdaa5e708a2bf7a3f37b88da","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":14,"id":"57618380_761a829a","updated":"2023-03-08 09:58:21.000000000","message":"Question, as a best practice should we not ensure all our api have a similar pattern and start with @wsgi.Controller.api_version(\"\u003cminimum_rev\u003e\") ?","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"}],"nova/api/openstack/compute/lock_server.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fad20e0ede1d1f35f3015add1ba8d4d92b777750","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock_v2_73)"},{"line_number":34,"context_line":"    def _lock(self, req, id, body):"},{"line_number":35,"context_line":"        \"\"\"Lock a server instance.\"\"\""},{"line_number":36,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"5317c42c_dde52fe6","line":33,"updated":"2023-02-28 15:45:22.000000000","message":"This isn\u0027t really correct - if you look at our api-ref [1], prior to version 2.73, the request body could only look like:\n\n  {\n    \"lock\": null\n  }\n  \nIn version 2.73 and up, an optional \"locked_reason\" is allowed:\n\n  {\n    \"lock\": {\"locked_reason\": \"I don\u0027t want to work\"}\n  }\n  \nSo we can\u0027t validate all microversions with the same schema as 2.73, because sending a \"locked_reason\" in earlier versions should result in a 400 bad request. We need a new schema, and new @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.72\u0027).\n\n[1] https://docs.openstack.org/api-ref/compute/?expanded\u003dlock-server-lock-action-detail#lock-server-lock-action","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"be2e32281e88cd64412be429b52934db5933f72e","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock_v2_73)"},{"line_number":34,"context_line":"    def _lock(self, req, id, body):"},{"line_number":35,"context_line":"        \"\"\"Lock a server instance.\"\"\""},{"line_number":36,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"876b37fd_a966cb03","line":33,"in_reply_to":"0cc63255_eec9bcd1","updated":"2023-03-08 14:48:59.000000000","message":"I was thinking more about specifying that it could be a null or an empty object.\nBut what you are proposing is good too.","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"ce574083038cf5fb15fbb5dda8df698ca71f3a0e","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock_v2_73)"},{"line_number":34,"context_line":"    def _lock(self, req, id, body):"},{"line_number":35,"context_line":"        \"\"\"Lock a server instance.\"\"\""},{"line_number":36,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"0cc63255_eec9bcd1","line":33,"in_reply_to":"14911772_6b9d7085","updated":"2023-03-08 13:43:32.000000000","message":"It would be ok if it just said what happens if you give it something else: an error, ignored data...","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"f1a5b45159004c7f38e1cedafbb782b0ad950183","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock_v2_73)"},{"line_number":34,"context_line":"    def _lock(self, req, id, body):"},{"line_number":35,"context_line":"        \"\"\"Lock a server instance.\"\"\""},{"line_number":36,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"648af8bf_ca608ec2","line":33,"in_reply_to":"5317c42c_dde52fe6","updated":"2023-03-01 10:02:50.000000000","message":"Documentation is kind of vague on this case, the \"lock\" argument is described with:\n\"The action to lock a server. This parameter can be null. Up to microversion 2.73, this parameter should be null.\",\nwhich specifies that it should be null, but does not clarify what happens if it does not. For my case, I will consider that it must be null, but we should update the docs accordingly.","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"5f458391137a0e8b2d37ca259b5dec2d60040d2f","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock_v2_73)"},{"line_number":34,"context_line":"    def _lock(self, req, id, body):"},{"line_number":35,"context_line":"        \"\"\"Lock a server instance.\"\"\""},{"line_number":36,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"14911772_6b9d7085","line":33,"in_reply_to":"648af8bf_ca608ec2","updated":"2023-03-08 11:27:13.000000000","message":"Totally agree \"This parameter can be null\" is ambiguous.\nI wonder what it could be if not null ?","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"27eaf16076b44ed2653b2c1b1258337dfaef9657","unresolved":true,"context_lines":[{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_10, \u00272.10\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"950bf471_62070871","line":34,"updated":"2023-03-07 19:29:56.000000000","message":"Did you mean 2.1 here?","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"86d2cc6ad68296fce9aa6fa452c5eba0caf337cc","unresolved":true,"context_lines":[{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_10, \u00272.10\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"7740fc23_fda708b5","line":34,"in_reply_to":"950bf471_62070871","updated":"2023-03-08 08:42:08.000000000","message":"Fixed","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"5f458391137a0e8b2d37ca259b5dec2d60040d2f","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a6c502d_b4f7832d","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"updated":"2023-03-08 11:27:13.000000000","message":"I don\u0027t catch why you make a difference between 2.0 and 2.1.\nMy understanding is that 2.0 --\u003e 2.72 lock can be null or {}, then 2.73 it can be either null or contains an object with locked_reason.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"503f73a7d5556cb63f2dcf6152a1f18cd7df2b7a","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"7992b9f6_2e817107","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"in_reply_to":"17ef3657_29286710","updated":"2023-03-08 14:59:31.000000000","message":"I could add some manual conditions on top of the schema validation instead. Whatever you believe best, I have no preference.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"7d561a65195b0f0c61cb95a053fd15d25439aa46","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"aacda428_e337214f","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"in_reply_to":"7992b9f6_2e817107","updated":"2023-03-08 15:19:27.000000000","message":"I would say if you can fix that in the schema validation code, this is better, but you will have to take care of side effects as it is used everywhere.\n\nAlso, I recommend you wait for more feedback before changing anything, people might have other preferences, and I could be wrong.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"ce574083038cf5fb15fbb5dda8df698ca71f3a0e","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"d27d560f_bae8323b","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"in_reply_to":"9a6c502d_b4f7832d","updated":"2023-03-08 13:43:32.000000000","message":"Many tests expect the body of the request to be either: { \u0027lock\u0027: null } or { \u0027lock\u0027: {} }. Now, the bug that I am trying to fix is that: { \u0027lock\u0027: { \u0027foo\u0027: \u0027bar\u0027 }} must not be allowed on microversions below 2.73 while the others two are. To achieve this I created a schema that did just that, but it turns out that at: https://opendev.org/openstack/nova/src/commit/c4fe563bdd21e8800e0e2964b51f2970363715fe/nova/api/validation/validators.py#L291-L293, the \u0027additionalProperties\u0027 field is always set to true on microversion v2.0 for some compability issues I am now very aware of. To handle that case I created a schema for v2.0 that only allows null to be given and another v2.1 forward that also allows {}.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f77e5a954256536c8b8b370374639cc16719c2f1","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"0b4e229d_0f35326c","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"in_reply_to":"aacda428_e337214f","updated":"2023-03-11 01:00:35.000000000","message":"v2.0 has to be exactly same as 2.1 that is the reason of not doing v3 in past. For backward compatiblity, we cannot have two different schema/request for 2.0 and 2.1.\n\nOnly exception we did between those in past is to allow trailing/leading whitespace in name for V2.1 compatible mode.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"be2e32281e88cd64412be429b52934db5933f72e","unresolved":true,"context_lines":[{"line_number":30,"context_line":"    @wsgi.response(202)"},{"line_number":31,"context_line":"    @wsgi.expected_errors(404)"},{"line_number":32,"context_line":"    @wsgi.action(\u0027lock\u0027)"},{"line_number":33,"context_line":"    @validation.schema(lock_server.lock, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":34,"context_line":"    @validation.schema(lock_server.lock_v2_1, \u00272.1\u0027, \u00272.72\u0027)"},{"line_number":35,"context_line":"    @validation.schema(lock_server.lock_v2_73, \u00272.73\u0027)"},{"line_number":36,"context_line":"    def _lock(self, req, id, body):"},{"line_number":37,"context_line":"        \"\"\"Lock a server instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"17ef3657_29286710","line":34,"range":{"start_line":33,"start_character":4,"end_line":34,"end_character":60},"in_reply_to":"d27d560f_bae8323b","updated":"2023-03-08 14:48:59.000000000","message":"Thank you really much for the answer. It is clearer.\nI think it is a bit weird to have these both schema and a diff in between 2.0 and 2.1 for that reason.\n\nIn that case, would it be clearer/simpler to have a more permissive schema and block improper requests just a bit later in the code ?","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"}],"nova/api/openstack/compute/migrate_server.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fad20e0ede1d1f35f3015add1ba8d4d92b777750","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    @wsgi.response(202)"},{"line_number":40,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"},{"line_number":41,"context_line":"    @wsgi.action(\u0027migrate\u0027)"},{"line_number":42,"context_line":"    @validation.schema(migrate_server.migrate_v2_56)"},{"line_number":43,"context_line":"    def _migrate(self, req, id, body):"},{"line_number":44,"context_line":"        \"\"\"Permit admins to migrate a server to a new host.\"\"\""},{"line_number":45,"context_line":"        context \u003d req.environ[\u0027nova.context\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"b309510a_0b4b92cd","line":42,"updated":"2023-02-28 15:45:22.000000000","message":"Similar logic here:\n\nhttps://docs.openstack.org/api-ref/compute/?expanded\u003dlock-server-lock-action-detail#migrate-server-migrate-action","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"27eaf16076b44ed2653b2c1b1258337dfaef9657","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"},{"line_number":41,"context_line":"    @wsgi.action(\u0027migrate\u0027)"},{"line_number":42,"context_line":"    @validation.schema(migrate_server.migrate, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":43,"context_line":"    @validation.schema(migrate_server.migrate_v2_10, \u00272.10\u0027, \u00272.55\u0027)"},{"line_number":44,"context_line":"    @validation.schema(migrate_server.migrate_v2_56, \u00272.56\u0027)"},{"line_number":45,"context_line":"    def _migrate(self, req, id, body):"},{"line_number":46,"context_line":"        \"\"\"Permit admins to migrate a server to a new host.\"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"90bd87ca_6e6ea5c0","line":43,"updated":"2023-03-07 19:29:56.000000000","message":"And here","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"86d2cc6ad68296fce9aa6fa452c5eba0caf337cc","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"},{"line_number":41,"context_line":"    @wsgi.action(\u0027migrate\u0027)"},{"line_number":42,"context_line":"    @validation.schema(migrate_server.migrate, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":43,"context_line":"    @validation.schema(migrate_server.migrate_v2_10, \u00272.10\u0027, \u00272.55\u0027)"},{"line_number":44,"context_line":"    @validation.schema(migrate_server.migrate_v2_56, \u00272.56\u0027)"},{"line_number":45,"context_line":"    def _migrate(self, req, id, body):"},{"line_number":46,"context_line":"        \"\"\"Permit admins to migrate a server to a new host.\"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"c63ae8d6_9c2ff153","line":43,"in_reply_to":"90bd87ca_6e6ea5c0","updated":"2023-03-08 08:42:08.000000000","message":"Fixed","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f77e5a954256536c8b8b370374639cc16719c2f1","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    @wsgi.response(202)"},{"line_number":40,"context_line":"    @wsgi.expected_errors((400, 403, 404, 409))"},{"line_number":41,"context_line":"    @wsgi.action(\u0027migrate\u0027)"},{"line_number":42,"context_line":"    @validation.schema(migrate_server.migrate, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":43,"context_line":"    @validation.schema(migrate_server.migrate_v2_1, \u00272.1\u0027, \u00272.55\u0027)"},{"line_number":44,"context_line":"    @validation.schema(migrate_server.migrate_v2_56, \u00272.56\u0027)"},{"line_number":45,"context_line":"    def _migrate(self, req, id, body):"},{"line_number":46,"context_line":"        \"\"\"Permit admins to migrate a server to a new host.\"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"a61003a6_030208ea","line":43,"range":{"start_line":42,"start_character":0,"end_line":43,"end_character":66},"updated":"2023-03-11 01:00:35.000000000","message":"ditto. 2.0 and 2.1 has to be identical schema.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"}],"nova/api/openstack/compute/schemas/lock_server.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"31a40623281bc5afe7dde5a4dc15822566e5d17a","unresolved":true,"context_lines":[{"line_number":14,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":15,"context_line":"    \u0027properties\u0027: {"},{"line_number":16,"context_line":"        \u0027lock\u0027: {"},{"line_number":17,"context_line":"            \u0027type\u0027: [\u0027object\u0027, \u0027null\u0027],"},{"line_number":18,"context_line":"            \u0027properties\u0027: {},"},{"line_number":19,"context_line":"            \u0027additionalProperties\u0027: False,"},{"line_number":20,"context_line":"        },"}],"source_content_type":"text/x-python","patch_set":12,"id":"20de1ca7_27d23a25","line":17,"updated":"2023-03-03 01:19:46.000000000","message":"So I think having the \u0027object\u0027 option here makes your new test fail because it still passes schema validation, so it returns a 202 and not a 400. I\u0027m assuming you did this to allow {\u0027lock\u0027: {}} to continue to work.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"61d5e26e5e38b74fd651b3cd64ff476c154bf5a8","unresolved":true,"context_lines":[{"line_number":14,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":15,"context_line":"    \u0027properties\u0027: {"},{"line_number":16,"context_line":"        \u0027lock\u0027: {"},{"line_number":17,"context_line":"            \u0027type\u0027: [\u0027object\u0027, \u0027null\u0027],"},{"line_number":18,"context_line":"            \u0027properties\u0027: {},"},{"line_number":19,"context_line":"            \u0027additionalProperties\u0027: False,"},{"line_number":20,"context_line":"        },"}],"source_content_type":"text/x-python","patch_set":12,"id":"0f54220c_e556b623","line":17,"in_reply_to":"20de1ca7_27d23a25","updated":"2023-03-03 08:57:35.000000000","message":"That irked because the schema I provided is correct, unit tests use the same thing and they match the cases that I want. After some digging, I have seen that there is a compatibility condition with microversion v2.0 where the \u0027additionalProperties\u0027 field is ignored. That breaks my schema and allows cases in those tests to pass when they should not.\n\nFrom here I think I will need to differentiate between microversions v2.0 and v2.1. Maybe I could allow {} for v2.1 but only null for v2.0.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"5f458391137a0e8b2d37ca259b5dec2d60040d2f","unresolved":true,"context_lines":[{"line_number":17,"context_line":"            \u0027type\u0027: [\u0027null\u0027],"},{"line_number":18,"context_line":"        },"},{"line_number":19,"context_line":"    },"},{"line_number":20,"context_line":"    \u0027required\u0027: [\u0027lock\u0027]"},{"line_number":21,"context_line":"}"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"lock_v2_1 \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"641ba21a_5d023d7e","line":20,"range":{"start_line":20,"start_character":4,"end_line":20,"end_character":24},"updated":"2023-03-08 11:27:13.000000000","message":"I think it would be good to add \u0027additionalProperties\u0027: False to avoid passing more fields. Although I don\u0027t understand why you have this case.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"ce574083038cf5fb15fbb5dda8df698ca71f3a0e","unresolved":true,"context_lines":[{"line_number":17,"context_line":"            \u0027type\u0027: [\u0027null\u0027],"},{"line_number":18,"context_line":"        },"},{"line_number":19,"context_line":"    },"},{"line_number":20,"context_line":"    \u0027required\u0027: [\u0027lock\u0027]"},{"line_number":21,"context_line":"}"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"lock_v2_1 \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"a8ba7a57_ee24233a","line":20,"range":{"start_line":20,"start_character":4,"end_line":20,"end_character":24},"in_reply_to":"641ba21a_5d023d7e","updated":"2023-03-08 13:43:32.000000000","message":"For the reason above.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f77e5a954256536c8b8b370374639cc16719c2f1","unresolved":true,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"lock \u003d {"},{"line_number":14,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":15,"context_line":"    \u0027properties\u0027: {"},{"line_number":16,"context_line":"        \u0027lock\u0027: {"},{"line_number":17,"context_line":"            \u0027type\u0027: [\u0027null\u0027],"},{"line_number":18,"context_line":"        },"},{"line_number":19,"context_line":"    },"},{"line_number":20,"context_line":"    \u0027required\u0027: [\u0027lock\u0027]"},{"line_number":21,"context_line":"}"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"lock_v2_1 \u003d {"},{"line_number":24,"context_line":"    \u0027type\u0027: \u0027object\u0027,"},{"line_number":25,"context_line":"    \u0027properties\u0027: {"},{"line_number":26,"context_line":"        \u0027lock\u0027: {"},{"line_number":27,"context_line":"            \u0027type\u0027: [\u0027object\u0027, \u0027null\u0027],"},{"line_number":28,"context_line":"            \u0027properties\u0027: {},"},{"line_number":29,"context_line":"            \u0027additionalProperties\u0027: False,"},{"line_number":30,"context_line":"        },"},{"line_number":31,"context_line":"    },"},{"line_number":32,"context_line":"    \u0027required\u0027: [\u0027lock\u0027],"},{"line_number":33,"context_line":"    \u0027additionalProperties\u0027: False,"},{"line_number":34,"context_line":"}"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"lock_v2_73 \u003d {"},{"line_number":37,"context_line":"    \u0027type\u0027: \u0027object\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"26987723_61a5b576","line":34,"range":{"start_line":13,"start_character":0,"end_line":34,"end_character":1},"updated":"2023-03-11 01:00:35.000000000","message":"I agree that our action APIs are not good interface and we did not have any schema validation for 2.0/2.1. \u0027lock\u0027 in request body (\u003c2.73 microversion) is just used to selection the action to be called and its body value is ignored completely.\n\nThat is why anything is allowed: {“lock”: null}, {“lock”: {}}, {“lock”: {\u0027foo\u0027: \u0027foo\u0027}}. \n\nIf we add schema to disallow those dummy/empty value then it is backward incompatible change and cannot be done without microversion bump. Otherwise users/scripts using it like {“lock”: {}} will start failing.\n\nAnother example of such change was to make additionalProperties False and we did that ith microversion bump only (2.75).\n\nOne opportuntiy to fix these request are when we change any of these action APIs for example 2.73 in lock APIs, we do allow only {“lock”: null} or {“lock”: (\u003cexpected field of that microversion\u003e)} and disallow all other empty object or dummy values ({“lock”: {}}, {“lock”: {\u0027foo\u0027: \u0027foo\u0027})","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"}],"nova/api/openstack/compute/schemas/migrate_server.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"5f458391137a0e8b2d37ca259b5dec2d60040d2f","unresolved":true,"context_lines":[{"line_number":27,"context_line":"            \u0027type\u0027: [\u0027null\u0027],"},{"line_number":28,"context_line":"        },"},{"line_number":29,"context_line":"    },"},{"line_number":30,"context_line":"    \u0027required\u0027: [\u0027migrate\u0027],"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"migrate_v2_1 \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a1aa12e_cab60f6c","line":30,"range":{"start_line":30,"start_character":4,"end_line":30,"end_character":28},"updated":"2023-03-08 11:27:13.000000000","message":"I think it would be good to add \u0027additionalProperties\u0027: False to avoid passing more fields.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"ce574083038cf5fb15fbb5dda8df698ca71f3a0e","unresolved":true,"context_lines":[{"line_number":27,"context_line":"            \u0027type\u0027: [\u0027null\u0027],"},{"line_number":28,"context_line":"        },"},{"line_number":29,"context_line":"    },"},{"line_number":30,"context_line":"    \u0027required\u0027: [\u0027migrate\u0027],"},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"migrate_v2_1 \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"969a9c61_41633837","line":30,"range":{"start_line":30,"start_character":4,"end_line":30,"end_character":28},"in_reply_to":"9a1aa12e_cab60f6c","updated":"2023-03-08 13:43:32.000000000","message":"Same thing.","commit_id":"76d9de55e1327ac993836a9ba5ba875fd7ca1dc7"}],"nova/api/openstack/compute/shelve.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fad20e0ede1d1f35f3015add1ba8d4d92b777750","unresolved":true,"context_lines":[{"line_number":96,"context_line":"    # \u0027availability_zone\u0027 \u003d None is supported as well to unpin the"},{"line_number":97,"context_line":"    # availability zone of an instance bonded to this availability_zone"},{"line_number":98,"context_line":"    @validation.schema(shelve_schemas.unshelve_v291)"},{"line_number":99,"context_line":"    def _unshelve(self, req, id, body):"},{"line_number":100,"context_line":"        \"\"\"Restore an instance from shelved mode.\"\"\""},{"line_number":101,"context_line":"        context \u003d req.environ[\"nova.context\"]"},{"line_number":102,"context_line":"        instance \u003d common.get_instance(self.compute_api, context, id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"925ce1ba_2d8c4b69","line":99,"updated":"2023-02-28 15:45:22.000000000","message":"And here, but we\u0027ll need to add two schemas: 2.0 through 2.76, 2.77 through 2.90 (optional availability_zone allowed), and 2.91 and up (optional host allowed)\n\nhttps://docs.openstack.org/api-ref/compute/?expanded\u003dlock-server-lock-action-detail,unshelve-restore-shelved-server-unshelve-action-detail#unshelve-restore-shelved-server-unshelve-action","commit_id":"65fcbf42ec75439215a9c3ff09f4da1ecaa5b041"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"27eaf16076b44ed2653b2c1b1258337dfaef9657","unresolved":true,"context_lines":[{"line_number":84,"context_line":"    @wsgi.expected_errors((400, 404, 409))"},{"line_number":85,"context_line":"    @wsgi.action(\u0027unshelve\u0027)"},{"line_number":86,"context_line":"    @validation.schema(shelve_schemas.unshelve, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":87,"context_line":"    @validation.schema(shelve_schemas.unshelve_v210, \u00272.10\u0027, \u00272.76\u0027)"},{"line_number":88,"context_line":"    # In microversion 2.77 we support specifying \u0027availability_zone\u0027 to"},{"line_number":89,"context_line":"    # unshelve a server. But before 2.77 there is no request body"},{"line_number":90,"context_line":"    # schema validation (because of body\u003dnull)."}],"source_content_type":"text/x-python","patch_set":13,"id":"e7a23e22_27ef6191","line":87,"updated":"2023-03-07 19:29:56.000000000","message":"And here","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"},{"author":{"_account_id":34443,"name":"Jorge San Emeterio","display_name":"jsanemet","email":"jsanemet@redhat.com","username":"jsanemet"},"change_message_id":"86d2cc6ad68296fce9aa6fa452c5eba0caf337cc","unresolved":true,"context_lines":[{"line_number":84,"context_line":"    @wsgi.expected_errors((400, 404, 409))"},{"line_number":85,"context_line":"    @wsgi.action(\u0027unshelve\u0027)"},{"line_number":86,"context_line":"    @validation.schema(shelve_schemas.unshelve, \u00272.0\u0027, \u00272.0\u0027)"},{"line_number":87,"context_line":"    @validation.schema(shelve_schemas.unshelve_v210, \u00272.10\u0027, \u00272.76\u0027)"},{"line_number":88,"context_line":"    # In microversion 2.77 we support specifying \u0027availability_zone\u0027 to"},{"line_number":89,"context_line":"    # unshelve a server. But before 2.77 there is no request body"},{"line_number":90,"context_line":"    # schema validation (because of body\u003dnull)."}],"source_content_type":"text/x-python","patch_set":13,"id":"b02f5ba4_bf693ef7","line":87,"in_reply_to":"e7a23e22_27ef6191","updated":"2023-03-08 08:42:08.000000000","message":"Fixed","commit_id":"f7bac647e0a06446577ce8b707b344dd88fb2c45"}],"nova/tests/functional/api_sample_tests/test_lock_server.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"31a40623281bc5afe7dde5a4dc15822566e5d17a","unresolved":true,"context_lines":[{"line_number":36,"context_line":"    def test_post_lock_server_with_reason(self):"},{"line_number":37,"context_line":"        response \u003d self._do_post(\u0027servers/%s/action\u0027 % self.uuid,"},{"line_number":38,"context_line":"                                 name\u003d\u0027lock-server-with-reason\u0027, subs\u003d{})"},{"line_number":39,"context_line":"        self.assertEqual(400, response.status_code)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    def test_post_unlock_server(self):"},{"line_number":42,"context_line":"        # Get api samples to unlock server request."}],"source_content_type":"text/x-python","patch_set":12,"id":"460bc233_db0b725f","line":39,"updated":"2023-03-03 01:19:46.000000000","message":"This assertion fails because of my previous comment.","commit_id":"1617415ef959c090901df979c39f5dbd9e8c80b0"}]}