Skip to content

Make validate_request pass on existing kwargs but remove those part of path #227

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

puittenbroek
Copy link

Checklist:

  • Run pytest tests and no failed.
  • Run ruff check flask_openapi3 tests examples and no failed.
  • Run mypy flask_openapi3 and no failed.
  • Run mkdocs serve and no failed.

@luolingchun The test in tests/test_validate_request.py illustrates the problem.

Our auth decorator injects some kwargs into the function which now get dropped.

My change makes any kwargs used by the path model be omitted, but other kwargs are kept.

@luolingchun
Copy link
Owner

I don't want the view function to receive a single argument, because that's not good for data validation, and all arguments can be encapsulated into the following six arguments:

path, query, form, body, header, cookie

The client_id in your code can be encapsulated in a header.

@puittenbroek
Copy link
Author

The client_id from header is an example. Our function injects multiple kwargs, related to the user but not directly from the header.

@puittenbroek
Copy link
Author

I don't want the view function to receive a single argument, because that's not good for data validation, and all arguments can be encapsulated into the following six arguments:

Sorry but I disagree. Your package is about validation the incoming data as mentioned in those 6 variables and providing very nice openapi docs for that.

But it shouldn't limit developers passing on other arguments to their functions. Decorators as I show-cased are often used, for various reason and will often (but not always) pass on their own arguments to enrich the data flow.

@luolingchun
Copy link
Owner

@puittenbroek I'm sorry for the long time to reply, I am willing to merge this PR, but the prerequisite is that you write a useful case in the documentation to illustrate this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants