marshmallow-code / flask-smorest

DB agnostic framework to build auto-documented REST APIs with Flask and marshmallow
https://flask-smorest.readthedocs.io
MIT License
651 stars 72 forks source link

Pagination documentation example is incorrect #543

Closed BrendanJM closed 6 months ago

BrendanJM commented 1 year ago

The pagination docs example for cursor pagination is incorrect as written: https://flask-smorest.readthedocs.io/en/latest/pagination.html#cursor-pager

The example used will always raise exceptions using SQLAlchemy or Mongoengine (the two examples provided):

from flask_smorest import Page

class CursorPage(Page):
    @property
    def item_count(self):
        return self.collection.count()

@blp.route("/")
class Pets(MethodView):
    @blp.response(200, PetSchema(many=True))
    @blp.paginate(CursorPage)
    def get(self):
        return Pet.get()

Both libraries have queries that implement a .get() method, but that is for single item lookup only, which is not something that makes sense with pagination:

I understand flask-smorest doesn't concern itself with the ORM, but since e.g. SQLAlchemy is so widely used with Flask, it could be nice to see a more fully-fleshed out and functioning example. In particular, because the documentation suggests "it is generally good practice to paginate the resource", it would follow showing how to do so in practice could be overall beneficial improvement to the documentation.

BrendanJM commented 1 year ago

I would love to provide a suggestion on how to improve this more concretely, but I haven't figured out how to get this working yet for a basic SQLAlchemy example. Googling around yields a lot of flask-sqlalchemy responses, but I would rather not add another flask extension if avoidable (and was hoping to find a basic enough way forward with flask-smorest).

lafrech commented 1 year ago

This part of the doc is meant to show how to create a cursor paginator.

The view func should return a cursor, so for SQLAlchemy that would be something along the lines of

@blp.route("/")
class Pets(MethodView):
    @blp.response(200, PetSchema(many=True))
    @blp.paginate(CursorPage)
    def get(self):
        return db.session.query(Pet)  # I'd also use query args to filter by chaining .filter_by(**kwargs)

I didn't think this was worth adding to the doc and I'd rather avoid it because then we could be asked to also add the code that provides db and write a fully working example that may be broken with new SQLAlchemy versions. But we may rephrase or just add a comment to stress the fact that this is just a dummy example and the view method must return a DB cursor. Creating the cursor is up to the user and their framework.

I haven't been using MongoEngine for ages and it is totally possible that the cursor paginator doesn't work anymore. FWIW, I've been using umongo above pymongo and latest versions of pymongo removed the method or attribute allowing to count so things are more complicated now, unfortunately: https://jira.mongodb.org/browse/PYTHON-1724.

lafrech commented 6 months ago

I reworked the doc paragraph to only provide a SQLAlchemy example that should work out-of-the-box.