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.

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

Support for MethodView #85

Closed
lafrech opened this issue Jul 19, 2016 · 5 comments

Comments

@lafrech
Copy link
Member

commented Jul 19, 2016

I'm using Flask's MethodViews for an API.

Basically, the idea is to have one class per endpoint and one class method for each HTTP method. The method name is the HTTP method, lowercase.

from flask.views import MethodView

class UserAPI(MethodView):

    def get(self):
        """User detail view.
        ---
        get:
            responses:
                200:
                    schema: UserSchema
        """
        users = User.query.all()
        ...

    def post(self):
        """User post.
        ---
        post:
            responses:
                201:
                    schema: UserSchema
        """
        user = User.from_form_data(request.form)
        ...

app.add_url_rule('/users/', view_func=UserAPI.as_view('users'))

AFAIU, there is currently no support for Flask's MethodView or Flask's View, only functions are supported. Is this correct?

I think I managed to get apispec to produce the correct OpenAPI file using the following patch:

dev...Nobatek:dev_method_views

The idea is that if the view is a MethodView, we should not use the view's docstring but loop over the docstrings of each method.

Thinking of it, maybe the only difference is that this allows the user to keep each docstring in each method rather than grouping all of them at the top of the class. This can be seen as cosmetic, but I think it would be better this way.

I'd be happy to get any feedback.

If there is interest for this feature, I may clean this up, add tests and all, but before I move any further, I'd rather be sure I'm on the right tracks.

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2016

flask-apispec uses apispec and provides support for Blueprint (which @deckar01 would like to add to apispec: #68), MethodView, along with nice decorators that allows to specify a Schema to dump the data with, automatically registering the Schema in the spec, and the same automatic registration of input arguments Schema.

I'm at a crossroad, right now. flask-apispec sounds appealing, but I'm still a bit reluctant. Not only because it says "flask-apispec isn't stable yet", but also because from what I've read, its code is far from trivial ("annotation" mechanism), and if I ever want to change or customize anything, I'm afraid I might end up losing more time than I gained using it, if I'm able to do it at all.

Anyway, I thought it was worth mentioning as there may be overlap between flask-apispec and apispec.ext.flask, and it could be worth thinking twice before duplicating the work/features.

@deckar01

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

It looks pretty good so far. (I dropped a comment on the commit 09ca9a3#commitcomment-18304112)

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2016

Let's have the discussion here rather than in "line notes".

@deckar01:

I imagine implementing this feature would require adding a new core interface like add_views that lets helpers handle collections like MethodView or Blueprint. Reusing the existing single view seems semantically incorrect.

I guess you're right.

Thanks for taking the time to investigate and sorry if my point was unclear or uninformed.

As I wrote, there is no total blocker. One can use aMethodView if the docstrings are all gathered at the top of the view. (Or maybe by adding a bit of obscure magic in the application to concatenate all docstrings from the methods in the view's docstring before it is passed to the parser...)

@deckar01

This comment has been minimized.

Copy link
Member

commented Jul 19, 2016

@lafrench Good to hear it isn't broken. You should be able to keep the docstrings on the individual class methods and register them individually. It seems like it might be more important to keep the docs with their respective methods at the expense of a little bit of duplicated registration logic.

spec.add_path(view=UserAPI.get)
spec.add_path(view=UserAPI.post)

I imagine you could add a helper utility until apispec supports registering extension specific view collections.

def register_method_view(spec, method_view):
    for method in method_view.methods:
        view = getattr(view.view_class, method.lower())
        spec.add_path(view=view)
register_method_view(spec, UserAPI)
@lafrech

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2016

Nice. This is totally satisfying to me.

I'm closing this as my patch is useless and the question is answered. The "view collection" feature can be discussed in your blueprint issue.

Thanks again, @deckar01.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.