)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76836845da55ba73566d46cca5140b9d958c7f38","unresolved":true,"context_lines":[{"line_number":10,"context_line":"example, if we have a schema like so:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"  {type: [string, null], format: date-time}"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Then we expect either a date-time string or null, not a date-time string"},{"line_number":15,"context_line":"or (impossible) date-time null. Make this so, cleaning up and extending"},{"line_number":16,"context_line":"the tests in the process."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"41a1c8d5_7da20876","line":13,"updated":"2025-01-28 18:00:50.000000000","message":"for what its worht while i agree that format checsk shoudl be skipped for null\n\ni also agree with the comments on the issue that this general patther shoudl be extened to other nullable data types like optional ints.\n\nhttps://github.com/OAI/OpenAPI-Specification/issues/3148#issuecomment-1448722539\n\njasonschema/openapi spec  shoudl be able to to express generic vlaiatons for a data type and ignore them in for the null type where valid.\n\nanyway im ok with this refactor based on teh converstaion in the refernce issue.","commit_id":"79ca1b0637334216dd40d0a285269d8d9867769e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6c08270ebc5a30bbd86f1b010f14a4068d7b96fb","unresolved":false,"context_lines":[{"line_number":10,"context_line":"example, if we have a schema like so:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"  {type: [string, null], format: date-time}"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Then we expect either a date-time string or null, not a date-time string"},{"line_number":15,"context_line":"or (impossible) date-time null. Make this so, cleaning up and extending"},{"line_number":16,"context_line":"the tests in the process."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"bec0272e_5b183688","line":13,"in_reply_to":"41a1c8d5_7da20876","updated":"2025-03-04 17:15:21.000000000","message":"Acknowledged","commit_id":"79ca1b0637334216dd40d0a285269d8d9867769e"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b2dd51c341ddfe19a76839b86952bd05096ea880","unresolved":true,"context_lines":[{"line_number":12,"context_line":"  {type: [string, null], format: date-time}"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Then we expect either a date-time string or null, not a date-time string"},{"line_number":15,"context_line":"or (impossible) date-time null. Make this so, cleaning up and extending"},{"line_number":16,"context_line":"the tests in the process."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I755e69f78aed0cd4074204a0559f557998a34e29"},{"line_number":19,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"41a16ce4_e2c4410a","line":16,"range":{"start_line":15,"start_character":45,"end_line":16,"end_character":25},"updated":"2025-06-05 00:55:21.000000000","message":"For me both are ok and not sure what is advantage of this change but if you want to change I would like to see test for schema for passing non string/null value and failing.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"beeb57463f1e0b58474b3fc95322948266abf91f","unresolved":false,"context_lines":[{"line_number":12,"context_line":"  {type: [string, null], format: date-time}"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Then we expect either a date-time string or null, not a date-time string"},{"line_number":15,"context_line":"or (impossible) date-time null. Make this so, cleaning up and extending"},{"line_number":16,"context_line":"the tests in the process."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Change-Id: I755e69f78aed0cd4074204a0559f557998a34e29"},{"line_number":19,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"31edb5ee_ae455fa2","line":16,"range":{"start_line":15,"start_character":45,"end_line":16,"end_character":25},"in_reply_to":"41a16ce4_e2c4410a","updated":"2025-06-05 10:11:00.000000000","message":"I already have new tests added for the `null` case? If you look at `nova/tests/unit/test_api_validation.py` you\u0027ll see the following new tests:\n\n* `test_validate_cidr_null` (tests changes to `_validate_cidr_format`)\n* `test_validate_datetime_null` (tests changes to `_validate_datetime_format`)\n* `test_validate_uuid_null` (tests changes to `_validate_uuid_format`)\n* `test_validate_uri_null` (tests changes to `_validate_uri`)\n* `test_validate_ipv4_null` (tests code from jsonschema)\n* `test_validate_ipv6_null` (tests code from jsonschema)\n* `test_validate_base64_null` (tests changes to `_validate_base64_format`)\n* `test_validate_regex_null` (tests changes to `_validate_regex_format`)\n\nYou\u0027ll also see that I\u0027ve reworked a number of existing schema here, which are now tested implicitly though our unit and functional tests. You can prove this by reverting the changes to `nova/api/validation/validators.py` (but not the other changes) and looking at the resultant test failures.\n\nAs for testing non-string types, that doesn\u0027t make sense because format validation happens after type validation, meaning we wouldn\u0027t actually be testing any of our code. I\u0027ve more information left [in response to a later comment](https://review.opendev.org/c/openstack/nova/+/936368/comment/576fd536_9a89330f/) but I don\u0027t think there\u0027s anything to be done here.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"77df2488e422b8689519c026d72dc87f3e33784e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1d02b505_3d3c2485","updated":"2024-11-28 18:17:28.000000000","message":"recheck more volume woes","commit_id":"319117b3fb31fed824a418a6d2a1df890597c88a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"86312a91f7a2c4299c7c291251e8e63db63167fa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"8d16e1c1_639e373d","updated":"2025-03-05 09:54:13.000000000","message":"recheck unrelated failures","commit_id":"fb0222a6b81918d38c9a41703e92e1c97b39572c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a7f14b04c61caa0e7d7a3340af5bb1beb49a4c3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"972739ac_d977748a","updated":"2025-03-27 18:21:12.000000000","message":"recheck unrelated multi-cell failure","commit_id":"67faf21c2625ff836b4d488e0aee5b4c36d4d37b"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b2dd51c341ddfe19a76839b86952bd05096ea880","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d73de253_2f8fd634","updated":"2025-06-05 00:55:21.000000000","message":"I am not getting actual advantage of choosing one over other, if you can explain the advantage in commit msg it will be great. My preference is just leave as it is.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e96b87a7a88597c8b25c6df2452d019d3f8f7c81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"21ed3639_0921a8b2","updated":"2025-06-05 10:20:52.000000000","message":"Just to recap for everyone. format validation *only* happens once type validation passes. The reason for this patch is to allow schemas like these:\n\n```\n{\n\t\"type\": [\"integer\", \"string\"],\n\t\"pattern\": \"0-9\",\n}\n```\n\nThis says the value must be an integer *or* a string representation of an integer. Without the changes here, the value `123` (i.e. an integer) will be incorrectly rejected.\n\n```\n{\n\t\"type\": [\"boolean\", \"string\"],\n\t\"pattern\": \"^(yes|no|y|n|true|false)$\"\n}\n```\n\nThis says the value must be a boolean *or* a string representation of a boolean. Without the changes here, the value `true` (i.e. a boolean) will be incorrectly rejected.\n\n```\n{\n\t\"type\": [\"string\", \"null\"],\n\t\"format\": \"^[a-zA-Z_-]+$\",\n}\n```\n\nThis says the value must be a null or an alphanumeric string. Without the changes here, the value `null` will be incorrectly rejected.\n\nDoes this make sense? This really should be a no-brainer. I feel I\u0027m missing something to say two people have been unhappy with this so far 😅","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"131e778e6755d5861c6e7ec00bf8b87ed32bb2df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"2a31dd39_cd4dbeb5","updated":"2025-05-15 13:35:18.000000000","message":"Soft -1 to highlight my comment.\nOtherwise it looks good to me.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"43229d334b17a0d25c09e6d44be04099256d4045","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"acd55fa5_5bc6ceb2","updated":"2025-05-21 18:22:45.000000000","message":"im still ok with this but if you do respine for @rene.ribaud@gmail.com\u0027s comment that ok too.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e4ee2c0ee8d41151530955fbf73495dc62e87890","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9879e0a4_e916c437","updated":"2025-06-06 09:40:58.000000000","message":"recheck","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9c7a3b06ac8bd2b25038203222d9634d9eafc021","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"dbf87fbf_7ed1ae25","updated":"2025-06-03 14:11:46.000000000","message":"recheck","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4c54f704f670be8a70262d8b1f72956d4e771f53","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4d8c260e_b963e8ca","in_reply_to":"21ed3639_0921a8b2","updated":"2025-06-05 17:21:32.000000000","message":"I am not saying the changed way does not work, I am saying existing way also not broken and works fine. anyways my main concern was having test for non-str things are rejected from overall nova and I found those testing in controller tests so I am good on this.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6761a6d7642cecd8d5ac444c74669d6ab2321648","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"df1fc574_e8585e03","in_reply_to":"4d8c260e_b963e8ca","updated":"2025-06-05 17:52:57.000000000","message":"\u003e I am not saying the changed way does not work, I am saying existing way also not broken and works fine.\n\nIt works, but it\u0027s not fine. The existing use of `oneOf` results in schemas that are unnecessarily complex. It also makes them more expensive to process. Quoting from the [JSON Schema docs](https://json-schema.org/understanding-json-schema/reference/combining#oneOf):\n\n\u003e Careful consideration should be taken when using `oneOf` entries as the nature of it requires verification of every sub-schema which can lead to increased processing times. Prefer `anyOf` where possible.\n\nOn top of all this, not following the spec prevents us doing things that should be permitted by the spec. This is confusing for contributors (like me! 😄) There are multiple reasons to do things properly here. The only reason not to do it is inertia.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"}],"nova/api/openstack/compute/schemas/aggregates.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76836845da55ba73566d46cca5140b9d958c7f38","unresolved":false,"context_lines":[{"line_number":21,"context_line":"    \u0027oneOf\u0027: ["},{"line_number":22,"context_line":"        parameter_types.az_name_with_leading_trailing_spaces,"},{"line_number":23,"context_line":"        {\u0027type\u0027: \u0027null\u0027},"},{"line_number":24,"context_line":"    ],"},{"line_number":25,"context_line":"}"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"create \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"fb3e56b7_808cbce8","line":24,"updated":"2025-01-28 18:00:50.000000000","message":"nit: the trialign commas are not needed and i think that is what is causing autopep8 to reflow this","commit_id":"79ca1b0637334216dd40d0a285269d8d9867769e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"76836845da55ba73566d46cca5140b9d958c7f38","unresolved":true,"context_lines":[{"line_number":145,"context_line":"        \u0027availability_zone\u0027: {\u0027type\u0027: [\u0027null\u0027, \u0027string\u0027]},"},{"line_number":146,"context_line":"        \u0027created_at\u0027: {\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027date-time\u0027},"},{"line_number":147,"context_line":"        \u0027deleted\u0027: {\u0027type\u0027: \u0027boolean\u0027},"},{"line_number":148,"context_line":"        \u0027deleted_at\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027], \u0027format\u0027: \u0027date-time\u0027},"},{"line_number":149,"context_line":"        \u0027hosts\u0027: {"},{"line_number":150,"context_line":"            \u0027type\u0027: [\u0027array\u0027, \u0027null\u0027],"},{"line_number":151,"context_line":"            \u0027items\u0027: {"}],"source_content_type":"text/x-python","patch_set":6,"id":"09853af4_0b1c8fc4","line":148,"range":{"start_line":148,"start_character":30,"end_line":148,"end_character":50},"updated":"2025-01-28 18:00:50.000000000","message":"ack so this is encodeing the OR or oneOF beahivor.\nwe are providing an enumeration of types that are valid instead of a single vlaue.","commit_id":"79ca1b0637334216dd40d0a285269d8d9867769e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6c08270ebc5a30bbd86f1b010f14a4068d7b96fb","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        \u0027availability_zone\u0027: {\u0027type\u0027: [\u0027null\u0027, \u0027string\u0027]},"},{"line_number":146,"context_line":"        \u0027created_at\u0027: {\u0027type\u0027: \u0027string\u0027, \u0027format\u0027: \u0027date-time\u0027},"},{"line_number":147,"context_line":"        \u0027deleted\u0027: {\u0027type\u0027: \u0027boolean\u0027},"},{"line_number":148,"context_line":"        \u0027deleted_at\u0027: {\u0027type\u0027: [\u0027string\u0027, \u0027null\u0027], \u0027format\u0027: \u0027date-time\u0027},"},{"line_number":149,"context_line":"        \u0027hosts\u0027: {"},{"line_number":150,"context_line":"            \u0027type\u0027: [\u0027array\u0027, \u0027null\u0027],"},{"line_number":151,"context_line":"            \u0027items\u0027: {"}],"source_content_type":"text/x-python","patch_set":6,"id":"27567471_b15cd131","line":148,"range":{"start_line":148,"start_character":30,"end_line":148,"end_character":50},"in_reply_to":"09853af4_0b1c8fc4","updated":"2025-03-04 17:15:21.000000000","message":"Acknowledged","commit_id":"79ca1b0637334216dd40d0a285269d8d9867769e"}],"nova/api/validation/validators.py":[{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"131e778e6755d5861c6e7ec00bf8b87ed32bb2df","unresolved":true,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"4294327a_52c3d834","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"updated":"2025-05-15 13:35:18.000000000","message":"At a first glance I thought the validator was wrong.\nI suggest to add the following comment on all validators:\n- Format validators should only apply to values of the correct type.\n- If the type is wrong (e.g. a number instead of a string), type validation handles\n  it, and the format check should pass silently.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"beeb57463f1e0b58474b3fc95322948266abf91f","unresolved":false,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"8fe596cf_0163929f","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"174510f6_b374940d","updated":"2025-06-05 10:11:00.000000000","message":"\u003e Honestly, returning True for non-string values looks like \"all non-string values are validated successfully against this format-checker\" instead of \"format-checker is not applied to non-string values\".\n\nFor the purposes of the validator, these are the same thing. The validator result is a binary result: either the check passed or it did not. There is no reason (nor a mechanism) for us to disambiguate \"this instance validates\" and \"the validation doesn\u0027t apply to this instance\". With regards to the unasked question of whether it should apply or not, I\u0027ll note again that this is spec compliant behavior. From the [spec](https://json-schema.org/draft/2020-12/json-schema-core#name-assertions-and-instance-pri):\n\n\u003e  7.6.1. Assertions and Instance Primitive Types\n\u003e \n\u003e Most assertions only constrain values within a certain primitive type. When the type of the instance is not of the type targeted by the keyword, the instance is considered to conform to the assertion.\n\u003e\n\u003e For example, the \"maxLength\" keyword from the companion [validation vocabulary](https://json-schema.org/draft/2020-12/json-schema-core#json-schema-validation) [[json-schema-validation](https://json-schema.org/draft/2020-12/json-schema-core#json-schema-validation)]: will only restrict certain strings (that are too long) from being valid. If the instance is a number, boolean, null, array, or object, then it is valid against this assertion.\n\n\u003e at least we can jusy return instead of return True, I have not checked if return None is ok for jsonschema format checker or they expect bool only.\n\njsonschema itself expects a boolean. This is exactly what jsonschema is currently doing for the in-tree format checkers, like [email](https://github.com/python-jsonschema/jsonschema/blob/60c7b35b629fecfe6757db052e80acc52d71f30c/jsonschema/_format.py#L234-L239).","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9c7a3b06ac8bd2b25038203222d9634d9eafc021","unresolved":false,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"53ed921e_04302cee","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"1e270967_762bdc28","updated":"2025-06-03 14:11:46.000000000","message":"Acknowledged","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"df6d20f514eed91663d97c028721f630437e4e8d","unresolved":true,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"af83e44a_8ecd9d74","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"4294327a_52c3d834","updated":"2025-05-19 12:44:45.000000000","message":"This is what I\u0027m saying above with my link to the GitHub issue. Is that not clear enough? I feel I\u0027d just be repeating myself if I added more here.\n\nAlso, a comment:\n\n\u003e - If the type is wrong (e.g. a number instead of a string), type validation handles\n\u003e   it, and the format check should pass silently.\n\nCorrect: if the type is different. For example, you could have a schema like so:\n\n```\n{\n    \"type\": [\"string\", \"null\"],\n    \"format\": \"uri\"\n}\n```\n\nIf you tried validating this with e.g. `None` then `None` isn\u0027t \"wrong\" (because it\u0027s one of the two possible types). Rather, it\u0027s just different and is not something that this format checker will handle.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b2dd51c341ddfe19a76839b86952bd05096ea880","unresolved":false,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"174510f6_b374940d","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"53ed921e_04302cee","updated":"2025-06-05 00:55:21.000000000","message":"Honestly, returning True for non-string values looks like \"all non-string values are validated successfully against this format-checker\" instead of \"format-checker is not applied to non-string values\".\n\nat least we can jusy return instead of return True, I have not checked if return None is ok for jsonschema format checker or they expect bool only.\n\n    if not isinstance(instance, str):\n        return","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4c54f704f670be8a70262d8b1f72956d4e771f53","unresolved":false,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"21947850_98499df1","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"8fe596cf_0163929f","updated":"2025-06-05 17:21:32.000000000","message":"I am fine as I found testing the non-str but for overall schema. But again I am not much convienced on we must change it but also not striongly against of it if you want to do as per spec.\n\nI am ok on this now.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"43229d334b17a0d25c09e6d44be04099256d4045","unresolved":true,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fc1a1559_abe5ed7f","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"af83e44a_8ecd9d74","updated":"2025-05-21 18:22:45.000000000","message":"so by \"relevant primitive type\" stephen is say is saying \n\n\"Format validators should only apply to values of the correct type.\"\n\n\"If the type is wrong (e.g. a number instead of a string), type validation handles\nit, and the format check should pass silently.\"\n\nwhile imporant i feell like would be enough to say once in the commit mssage\nrather then repeating everywhere in the code.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":16207,"name":"ribaudr","display_name":"uggla","email":"rene.ribaud@gmail.com","username":"uggla","status":"Red Hat"},"change_message_id":"9b1ce1d560d4f2701f174a71e46037a34e30db47","unresolved":true,"context_lines":[{"line_number":39,"context_line":"def _validate_regex_format(instance):"},{"line_number":40,"context_line":"    # format checks constrain to the relevant primitive type"},{"line_number":41,"context_line":"    # https://github.com/OAI/OpenAPI-Specification/issues/3148"},{"line_number":42,"context_line":"    if not isinstance(instance, str):"},{"line_number":43,"context_line":"        return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # the empty string is a valid regex but doesn\u0027t give us anything to match"},{"line_number":46,"context_line":"    # on"},{"line_number":47,"context_line":"    if not instance:"}],"source_content_type":"text/x-python","patch_set":10,"id":"1e270967_762bdc28","line":44,"range":{"start_line":42,"start_character":0,"end_line":44,"end_character":1},"in_reply_to":"fc1a1559_abe5ed7f","updated":"2025-06-03 12:46:06.000000000","message":"Hi Stephen, sorry for the late reply.\nToday, I do understand that comment:\n\n```\n# format checks constrain to the relevant primitive type\n# https://github.com/OAI/OpenAPI-Specification/issues/3148\n```\n\nBut when I first saw it, I must admit it wasn’t very clear to me.\nAnd when I read the linked issue, I had trouble identifying what the key point was.\nI eventually figured it out, but not right away.\n\nThat’s why I suggested replacing those two lines with a comment that **I** personally find more explicit.\nThat said, I don’t want to make a big deal out of it because it’s a minor detail.\n\nI’d appreciate it if you updated it to a version that makes more sense to me, but I’m also okay with leaving it as-is now that I’ve taken a step back.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"}],"nova/tests/unit/test_api_validation.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b2dd51c341ddfe19a76839b86952bd05096ea880","unresolved":true,"context_lines":[{"line_number":934,"context_line":"        detail \u003d (\"Invalid input for field/attribute foo. Value: abc.\""},{"line_number":935,"context_line":"                  \" \u0027abc\u0027 is not a \u0027uuid\u0027\")"},{"line_number":936,"context_line":"        self.check_validation_error(self.post, body\u003d{\u0027foo\u0027: \u0027abc\u0027},"},{"line_number":937,"context_line":"                                    expected_detail\u003ddetail)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"class UriTestCase(APIValidationTestCase):"}],"source_content_type":"text/x-python","patch_set":10,"id":"576fd536_9a89330f","line":937,"range":{"start_line":937,"start_character":58,"end_line":937,"end_character":59},"updated":"2025-06-05 00:55:21.000000000","message":"As we are writting format_checker different way, I would like to have an explicit test for passing an integer or any non-string|null type.\n\nditto for the below format checker also","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"beeb57463f1e0b58474b3fc95322948266abf91f","unresolved":false,"context_lines":[{"line_number":934,"context_line":"        detail \u003d (\"Invalid input for field/attribute foo. Value: abc.\""},{"line_number":935,"context_line":"                  \" \u0027abc\u0027 is not a \u0027uuid\u0027\")"},{"line_number":936,"context_line":"        self.check_validation_error(self.post, body\u003d{\u0027foo\u0027: \u0027abc\u0027},"},{"line_number":937,"context_line":"                                    expected_detail\u003ddetail)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"class UriTestCase(APIValidationTestCase):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7484ae62_1990c987","line":937,"range":{"start_line":937,"start_character":58,"end_line":937,"end_character":59},"in_reply_to":"576fd536_9a89330f","updated":"2025-06-05 10:11:00.000000000","message":"We\u0027re testing format validation here, not type validation. Type validation occurs *before* format validation. As such, there\u0027s no value in having a check for e.g. integer values in any of these methods, because the error message will be the exact same since we never get as far as validating format.\n\nI can add tests for invalid types if you want, but I\u0027m essentially testing jsonschema itself, not anything nova-specific. That doesn\u0027t seem useful.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4c54f704f670be8a70262d8b1f72956d4e771f53","unresolved":false,"context_lines":[{"line_number":934,"context_line":"        detail \u003d (\"Invalid input for field/attribute foo. Value: abc.\""},{"line_number":935,"context_line":"                  \" \u0027abc\u0027 is not a \u0027uuid\u0027\")"},{"line_number":936,"context_line":"        self.check_validation_error(self.post, body\u003d{\u0027foo\u0027: \u0027abc\u0027},"},{"line_number":937,"context_line":"                                    expected_detail\u003ddetail)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"class UriTestCase(APIValidationTestCase):"}],"source_content_type":"text/x-python","patch_set":10,"id":"00ab984d_0f942702","line":937,"range":{"start_line":937,"start_character":58,"end_line":937,"end_character":59},"in_reply_to":"7484ae62_1990c987","updated":"2025-06-05 17:21:32.000000000","message":"you are right, we do not need to check those as part of format checker testing. And I found the overall controller tests testing those for example https://github.com/openstack/nova/blob/a1c47fc242b6002d2be60fc41176ce29d19d924e/nova/tests/unit/api/openstack/compute/test_shelve.py#L185\n\nAll good here.","commit_id":"678bc4ae6e3dd9a10f3adcbe411da7ee90cfe61c"}]}
