Skip to content

0.12.0 regression regarding usability of validation messages #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zupo opened this issue Sep 27, 2019 · 8 comments · Fixed by #168 or #170
Closed

0.12.0 regression regarding usability of validation messages #160

zupo opened this issue Sep 27, 2019 · 8 comments · Fixed by #168 or #170

Comments

@zupo
Copy link
Contributor

zupo commented Sep 27, 2019

Hey!

I'm the author of https://github.com/Pylons/pyramid_openapi3, and today I realized a new release of openapi-core is out, yay!

I immediately prepared a PR to bump pyramid_openapi3 to openapi-core 0.12.0, but tests on CI failed:

======================================================================
FAIL: test_name_too_short (app.FunctionalTests)
A name that is too short is picked up by openapi-core validation.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/repo/examples/singlefile/app.py", line 142, in test_name_too_short
    res.text,
AssertionError: 'Invalid parameter value for `name`: Value is shorter (2) than the minimum length of 3' not found in '400 Bad Request\n\nRequest validation failed.\n\n\nInvalid parameter value for `name`: Value not valid for schema\n\n'

----------------------------------------------------------------------

The test above fails in a single-file example that I ship with pyramid_openapi3. It fails because in 0.11.0, the validation error is more descriptive than in 0.12.0:

  • 0.11.0: "Invalid parameter value for `name`: Value is shorter (2) than the minimum length of 3"
  • 0.12.0: "Invalid parameter value for `name`: Value not valid for schema"

Is this an expected regression due to the move to a different validation engine that happened in 0.12.0 (or at least that is how I understood the commit messages)? Is it possible to configure the new validation to have more descriptive errors? Or am I completely missing the point here?

@p1c2u
Copy link
Collaborator

p1c2u commented Sep 30, 2019

Hi @zupo . That's correct. 0.12 comes with new OAS Validator which will allow to return more detailed errors and even json schema validation.
More descriptive errors will come with new releases of 0.12.x branch.

@zupo
Copy link
Contributor Author

zupo commented Sep 30, 2019

Fantastic! If you want/need help testing it before release, I'm happy to run pyramid_openapi3 tests against a branch of openapi-core!

zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Oct 22, 2019
More details on the regression at:
python-openapi/openapi-core#160

When 0.12.1 is released I'll update the tests to bring back
support for latest openapi-core.
zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Oct 22, 2019
More details on the regression at:
python-openapi/openapi-core#160

When 0.12.1 is released I'll update the tests to bring back
support for latest openapi-core.
zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Oct 22, 2019
More details on the regression at:
python-openapi/openapi-core#160

When 0.12.1 is released I'll update the tests to bring back
support for latest openapi-core.
@zupo
Copy link
Contributor Author

zupo commented Oct 22, 2019

Hey! Massive thanks for the fix!

I created a new PR that uses openapi-core master, here are my findings:

  1. Validation error messages are more verbose than before. I wouldn't say they are nicer, but they are certainly informative:
-            "Invalid parameter value for `name`: Value is shorter (2) "
-            "than the minimum length of 3",
+            "Invalid parameter value for `name`: "
+            "Value yo not valid for schema of type SchemaType.STRING: "
+            "[<ValidationError: \"'yo' is too short\">]\n\n",

https://github.com/Pylons/pyramid_openapi3/pull/43/files#r337661719

  1. When I try to catch the RequestValidationError to return a custom JSON instead of "validation error as plain string" I can't seem to get the information of what the actual error was. self.schema_errors is [] when I catch the error.
-   'message': "Value {'title': 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'} not "
-              'valid for schema of type SchemaType.OBJECT: []'}]
+   'field': 'title',
+   'message': 'Invalid schema property title: Value is longer (41) than the '
+              'maximum length of 40'}]

https://app.circleci.com/jobs/github/Pylons/pyramid_openapi3/178

Any idea why self.schema_errors would be empty?

@p1c2u
Copy link
Collaborator

p1c2u commented Oct 22, 2019

Hi @zupo .

  1. Most exceptions come from jsonschema validator. There is possibility to override some of them to be more descriptive. I will take a look at minItems and maxItems.
  2. Good spot. schema_errors stores generator which becomes empty after evaluation in __str__(). I need to use lazy object instead.

Thanks for help testing it.

@zupo
Copy link
Contributor Author

zupo commented Oct 24, 2019

Yup, fixed indeed, works great with the latest master! Any ETA on a new release yet? Can I help in any way?

@zupo
Copy link
Contributor Author

zupo commented Nov 19, 2019

Hey @p1c2u, can I in any way help cutting a 0.12.1 release that incorporates your fixes in #170? My PyPI handle is zupo and I've been releasing packages to PyPI since 2010. I'll do my best to avoid a brown-bag release.

@zupo
Copy link
Contributor Author

zupo commented Feb 15, 2020

Hey again @p1c2u! I love the recent changes in master and I can't wait to be able to update pyramid_openapi3 to the latest openapi-core. But I can't do it until there is the next release and so pyramid_openapi3 users are stuck with 0.11.1 which is now almost half a year old :(. I can't upgrade to 0.12.0 because of the regression reported in this ticket.

Can I in any way help with getting changes in master released? I can offer time and/or money.

@p1c2u
Copy link
Collaborator

p1c2u commented Feb 17, 2020

@zupo hey, I had to refactor couple of parts. I plan to release it shortly.

zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Feb 27, 2020
The `openapi-core` library we build on top of is upgraded to `0.13.2`
which brings a complete rewrite of the validation mechanism that is now
based on `jsonschema` library. This manifests as different validation error messages.

Additionally, I moved `openapi_validation_error` from `examples/todoapp` into the main package so it becomes a first-class citizen and people can use it without copy/pasting. You need to explicitly enable it by running `config.pyramid_openapi3_JSONify_errors()`. It's quite opinionated but
easy to override and provide your own, based on your OpenAPI schema.

Closes #59
Closes #33
Closes #43
Closes #40
Closes #34
Refs python-openapi/openapi-core#160
zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Feb 27, 2020
The `openapi-core` library we build on top of is upgraded to `0.13.2`
which brings a complete rewrite of the validation mechanism that is now
based on `jsonschema` library. This manifests as different validation error messages.

Additionally, I moved `openapi_validation_error` from `examples/todoapp` into the main package so it becomes a first-class citizen and people can use it without copy/pasting. You need to explicitly enable it by running `config.pyramid_openapi3_JSONify_errors()`. It's quite opinionated but
easy to override and provide your own, based on your OpenAPI schema.

Closes #59
Closes #33
Closes #43
Closes #40
Closes #34
Refs python-openapi/openapi-core#160
zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Feb 27, 2020
The `openapi-core` library we build on top of is upgraded to `0.13.2`
which brings a complete rewrite of the validation mechanism that is now
based on `jsonschema` library. This manifests as different validation error messages.

Additionally, I moved `openapi_validation_error` from `examples/todoapp` into the main package so it becomes a first-class citizen and people can use it without copy/pasting. You need to explicitly enable it by running `config.pyramid_openapi3_JSONify_errors()`. It's quite opinionated but
easy to override and provide your own, based on your OpenAPI schema.

Closes #59
Closes #33
Closes #43
Closes #40
Closes #34
Refs python-openapi/openapi-core#160
zupo added a commit to Pylons/pyramid_openapi3 that referenced this issue Mar 7, 2020
The `openapi-core` library we build on top of is upgraded to `0.13.2`
which brings a complete rewrite of the validation mechanism that is now
based on `jsonschema` library. This manifests as different validation error messages.

Additionally, I moved `openapi_validation_error` from `examples/todoapp` into the main package so it becomes a first-class citizen and people can use it without copy/pasting. You need to explicitly enable it by running `config.pyramid_openapi3_JSONify_errors()`. It's quite opinionated but
easy to override and provide your own, based on your OpenAPI schema.

Closes #59
Closes #33
Closes #43
Closes #40
Closes #34
Refs python-openapi/openapi-core#160
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants