mrevutskyi / flask-restless-ng

A Flask extension for creating simple ReSTful JSON APIs from SQLAlchemy models.
https://flask-restless-ng.readthedocs.io
Other
64 stars 11 forks source link

Type error constructing pagination object when using Flask-SQLAlchemy #37

Closed imax-erik closed 1 year ago

imax-erik commented 1 year ago
File "./lib/python3.10/site-packages/flask_restless/views/base.py", line 1500, in _paginated
    pagination = items.paginate(page_number, page_size,
TypeError: Query.paginate() takes 1 positional argument but 3 positional arguments (and 1 keyword-only argument) were given

Cause seems to be that the calling line is passing page_number and page_size as positional arguments, but Query.paginate() expects those parameters to be keyword only.

To my knowledge, this issue only affects relationships. I haven't encountered it elsewhere just yet, and top-level resources can be paginated without issue. If I encounter more impacted URLs, I will add them.

flask 2.2.2 flask-sqlalchemy 3.0.2 flask-restless-ng 2.3.0

https://github.com/mrevutskyi/flask-restless-ng/blob/4cd0a05e91d6cd486c23dfdfcdd8775488b20f70/flask_restless/views/base.py#L1500-L1501

https://github.com/pallets-eco/flask-sqlalchemy/blob/decc3e88cb40835b2087e14b88ca3558743419b4/src/flask_sqlalchemy/query.py#L64-L72

    def paginate(
        self,
        *,
        page: int | None = None,
        per_page: int | None = None,
        max_per_page: int | None = None,
        error_out: bool = True,
        count: bool = True,
    ) -> Pagination:

It looks like the method definition changed between flask-sqlalchemy 2.5.1 and 3.0.2:

https://github.com/pallets-eco/flask-sqlalchemy/blob/da0e0df80cc368d95dc5a118ce857cead172aded/flask_sqlalchemy/__init__.py#L473

    def paginate(self, page=None, per_page=None, error_out=True, max_per_page=None):

However, the parameter names did not change. Calling Query.paginate() using keywords appears to correct the issue:

            pagination = items.paginate(page=page_number, per_page=page_size,
                                        error_out=False)

If flask-restless-ng shouldn't support flask-sqlalchemy 3 (yet?), it may be worthwhile to constrain the flask-sqlalchemy optional dependency to major version 2 until major version 3 can be supported. Otherwise users will install both separately and not encounter an issue until they try to use the relationship collections.

mrevutskyi commented 1 year ago

Thank you for the detailed report. Yes, this is a bug, even Flask-SQLAlchemy 2.3.0 expects those as keyword arguments, not positional arguments. Fixed that and added a test.

I've released v2.3.1, please let me know if it still does not work for you.

imax-erik commented 1 year ago

I have updated to v2.3.1 and relationships seem to be working just fine. Thank you very much!