zmievsa / cadwyn

Production-ready community-driven modern Stripe-like API versioning in FastAPI
https://docs.cadwyn.dev/
MIT License
186 stars 24 forks source link

Cadwyn breaks fastapi-pagination #183

Closed gabloe closed 1 month ago

gabloe commented 1 month ago

Describe the bug FastAPI-Pagination injects dependencies in FastAPI routes to include query parameters for paginating response data. This does not appear to work with Cadwyn.

To Reproduce Steps to reproduce the behavior:

  1. Create a Cadwyn app.
  2. Call add_pagination(app)
  3. Create a route with a paginated response model
  4. Call the route with curl or using the Swagger UI
  5. Route will fail with RuntimeError("Use params, add_pagination or pagination_ctx")

Expected behavior Routes with a paginated response model should have the injected pagination_ctx dependency.

zmievsa commented 1 month ago

Will take a look soon.

zmievsa commented 1 month ago

@gabloe Please, see 3.15.3 release. Should've fixed it. It fixed cardinality on starlette-exporter as well. I will conduct my own tests on fastapi-pagination within 12 hours

https://github.com/zmievsa/cadwyn/pull/187

gabloe commented 1 month ago

Thanks @zmievsa. Now the pagination_ctx dependency is added to the route, but the Swagger UI is not updated with the query params

zmievsa commented 1 month ago

@gabloe this results from the fact that fastapi-pagination edits the routes at the very end and we are not generating our swagger dynamically. Someone is already working on this issue though I am not sure about an ETA.

As for a temporary workaround, Cadwyn apps have an "enrich_swagger" method that you can call after a call to fastapi-pagination to solve this issue.

Though I expect to solve the core issue rather soon.

zmievsa commented 1 month ago

Here is the issue I am referencing.

164

zmievsa commented 1 month ago

Please, see 3.15.4. It should fix all the issues. I have tested it with fastapi-pagination too and it worked perfectly for me.

We should now be 1 to 1 compatible with fastapi in terms of openapi and route structure so any libraries like fastapi-pagination should work out of the box.

gabloe commented 1 month ago

Wonderful! It is working great for me :)

zmievsa commented 1 month ago

Thank you for your bug reports, @gabloe ! Very descriptive and helpful. If you have any suggestions or need any further help -- feel free to contact me.