Skip to content
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.

Sign up for GitHub

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

Jump to bottom

request.openapi_validated does not break non-opeanpi views #172

Merged
merged 1 commit into from
Nov 28, 2022
Merged

request.openapi_validated does not break non-opeanpi views #172

merged 1 commit into from
Nov 28, 2022

Conversation

zupo
Copy link
Collaborator

@zupo zupo commented May 27, 2022 •
edited
Loading

Accessing request.openapi_validated in a route that has view_config(openapi=False) will no longer break the route.

Refs #165

@zupo zupo force-pushed the cleanup/openapi_true_attributeerror branch 4 times, most recently from f354f5b to 368061c Compare May 27, 2022 11:58
@zupo zupo mentioned this pull request May 27, 2022
Open
@grinspins
Copy link

Thank you for the quick fix @zupo, this looks good to me. In your comment on the issue you specifically mentioned that this should take care of the 1. issue I raised. From the looks of it should take care of the 2. problem as well, or am I missing something?

@zupo
Copy link
Collaborator Author

zupo commented May 27, 2022 •
edited
Loading

Thank you for the quick fix @zupo, this looks good to me. In your comment on the issue you specifically mentioned that this should take care of the 1. issue I raised. From the looks of it should take care of the 2. problem as well, or am I missing something?

Ah yes, potentially too, but I haven't fully verified that, and wrote a test case.

@zupo zupo force-pushed the cleanup/openapi_true_attributeerror branch from 368061c to 08cb63f Compare May 27, 2022 21:10
@zupo
Copy link
Collaborator Author

zupo commented May 27, 2022

Thank you for the quick fix @zupo, this looks good to me. In your comment on the issue you specifically mentioned that this should take care of the 1. issue I raised. From the looks of it should take care of the 2. problem as well, or am I missing something?

Ah yes, potentially too, but I haven't fully verified that, and wrote a test case.

Yep, 2. is fixed too, added a test to make sure it keeps working.

@zupo zupo force-pushed the cleanup/openapi_true_attributeerror branch from 08cb63f to 61956e8 Compare May 27, 2022 21:15
karantan
karantan approved these changes May 30, 2022
View reviewed changes
pyramid_openapi3/tests/test_path_parameters.py Outdated Show resolved Hide resolved
pyramid_openapi3/tests/test_validation.py Show resolved Hide resolved
pyramid_openapi3/tests/test_validation.py Show resolved Hide resolved
@zupo zupo force-pushed the cleanup/openapi_true_attributeerror branch from 61956e8 to 9aeb507 Compare May 30, 2022 20:57
@zupo
request.openapi_validated does not break non-opeanpi views
aa924e4 
Accessing `request.openapi_validated` in a route that has
`view_config(openapi=False)` will no longer break the route.

Also a fix to make response validation still work even if request validation
is turned off.

Finally, test_request_validation_disabled was a moot test: it would pass even
if validation was enabled, because the request was valid. Made sure both this
test and test_response_validation_disabled first verify the request/response
is invalid, then disable validation and test again.

Refs #165
@zupo zupo force-pushed the cleanup/openapi_true_attributeerror branch from 9aeb507 to aa924e4 Compare November 28, 2022 21:27
@blueracer-io
Copy link

blueracer-io bot commented Nov 28, 2022

Unit tests performance report: ✅

Everything looks great, carry on!

Branch Number of tests Suite duration Duration per 100 tests
main 80 1.8s1 2.2s1
cleanup/openapi_true_attributeerror2 82 (2%) 1.8s (1%) 2.2s (-2%)

Footnotes

  1. The median of the previous 1 runs. ↩ ↩2

  2. More specifically from commit aa924e4151ae96e31e38786f7a48d35bcf0021b8 in run #1. ↩

@zupo
Copy link
Collaborator Author

zupo commented Nov 28, 2022

Rebased off of latest master, let's get this one merged, finally.

@zupo zupo merged commit c281e1f into main Nov 28, 2022
@miketheman miketheman deleted the cleanup/openapi_true_attributeerror branch March 20, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karantan karantan karantan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zupo @grinspins @karantan