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.

Sign up for GitHub

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

Jump to bottom

Allow to remove max request response size #250

Open
MateoLostanlen opened this issue May 1, 2023 · 2 comments · May be fixed by #277
Open

Allow to remove max request response size #250

MateoLostanlen opened this issue May 1, 2023 · 2 comments · May be fixed by #277
Assignees
@frgfm
Labels
type: feat
Milestone
0.2.0

Comments

@MateoLostanlen
Copy link
Member

#190 introduce a response size limitation of 50, I get the point of doing this but can we add an optional prams to drop this limitation if we need it ?
@frgfm
Copy link
Member

frgfm commented May 28, 2023

Hey there 👋

I thought about it and here is what went through my head:

  • the goal here is to paginate the response similarly to most popular REST APIs on the planet: say you have 1000 elements in the response with pages of 100 elements, that's 10 pages. By default, it only returns the first one, and we can easily make it so that you can request other pages.
  • however, I don't think we should allow request for the entire thing

The reason behind that is simply to avoid cluttering the RAM of the server and have it crash. Would that be OK? (to be able to select the page you want to request, and then you'll aggregate on your own?)

If you have another idea, I'm honestly all ears :)

@frgfm frgfm self-assigned this May 28, 2023
@frgfm frgfm added this to the 0.2.0 milestone May 28, 2023
@frgfm frgfm mentioned this issue May 28, 2023
Open
@MateoLostanlen
Copy link
Member Author

Hi, there,

Yes, this page system is fine for me, I just need the metadata to exploit the collected images :)

blenzi pushed a commit that referenced this issue Aug 7, 2023
feat: add limit and offset parameters to routes that fetch lists from db
95cd027 
- add limit and offset parameters with descriptions, to all routes that fetch lists from db (fix #243, #250)
- use crud.fetch_all in all the routes above, to limit the number of items returned for non-admin users (fix #276)
- forbid /installation/site-devices to access site outside of group, instead of filtering devices
- improve test_installations.py (test_get_active_devices_on_site)
- Dockerfile-dev: update FROM image name
@blenzi blenzi linked a pull request Aug 7, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@frgfm frgfm

Labels
type: feat
Projects
None yet
Milestone
0.2.0
Development

Successfully merging a pull request may close this issue.

feat: add limit and offset parameters to routes that fetch lists from db pyronear/pyro-api
2 participants
@MateoLostanlen @frgfm