-
Notifications
You must be signed in to change notification settings - Fork 23
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
Flask DocumentedBlueprint: Add to spec all views defined in the blueprint. #27
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 2e6774d.
@tinproject @ergo Would you be up for reviewing this API, since you had some thoughts on #11? |
I hope I get more time on the weekend to take a proper look, this is just a quick read. I'm not convinced of the extension by subclassing approach. I'm not sure what happen with MethodViews or other extensions of flask. Also if I remember well one problem is that one view function could have more than one route and should documented for every route, but I'm not very familiar with new openapi spec. I'll try to try this PR on the weekend and write some code with it to clarify my thoughts. |
I like the idea implementation @JavierLuna but I found a little glitches:
|
I'll fix them as soon as I have a bit of free time. |
I finally get some hours and take a look at this (it's been a year since I made something with apispec so I have to refresh my mind) I've created a dumb project (at https://github.com/tinproject/apispec-webframeworks-test) and try the PR with different approach to add routes. First had to said that the code of Javier is good, it only lacks support for Blueprint I saw two drawbacks: On the project referenced above I played around url routing in flask, trying to show the map/rules/endpoints/view_functions relationships if you want to take a peek. |
self.documented_view_functions = defaultdict(list) | ||
self.spec = spec | ||
|
||
def route(self, rule, documented=True, **options): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I overrode this function to include the documented=True
kwarg, as I like declaring them explicitly.
Do you like it this way or shall I remove this?
Thanks for checking it out and for the suggestions @tinproject ! I refactored a little the implementation and added some tests reflecting your project's cases. Regarding your drawbacks: 1.- How would you implement it? The spec object is only needed when registering the application, maybe have it as 2.- I looked around and it seems that it cannot be changed in this project and a change in the Apispec project will be needed. |
if documented: | ||
self.documented_view_functions[rule].append(view_func) | ||
|
||
def register(self, app, options, first_registration=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my usercase I prefer to pass the spec in the register (or add_url_rule method) instead of the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could be a solution, yeah!
app.register_blueprint(doc_blueprint, spec=spec)
I'd still add the option of passing it through the __init__
as a kwarg and add an attach(spec)
method, to add variety of uses.
What do you think @tinproject ?
@JavierLuna The problem with the current implementation of FlaskPlugin is that it assume that one view_function match one to one to a path and that is not real. The reality is that a view_function can be matched by multiple endpoints (but an endpoint can only match one view function), and an endpoint can be matched by multiple Rule objects, that are defined by a path (and associated methods and other data). And all rules lives inside a Werkzeug Map object at Flask's I believe we can get the full spec representation of a path to return at For it we have to iterate over Flask app routing rules at This rule parse can also process and add path parameters to the spec, but this if other story. I'm seeing that right now the |
any update on this? |
Flask DocumentBlueprint
Is a blueprint which adds all views defined in it to the spec. You can ommit one view from being added to the spec setting
documented=False
in the Blueprint'sroute
function (see usage below).Usage
Has a test coverage of 100%.