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

Disable paging with &per_page=0 #16

Closed
schildbach opened this issue Jun 10, 2014 · 7 comments
Closed

Disable paging with &per_page=0 #16

schildbach opened this issue Jun 10, 2014 · 7 comments

Comments

@schildbach
Copy link

It would be nice if there was an easy way to disable paging. Currently, you have to append something like &per_page=9999999999999

The currently documented behaviour of not returning any results if you append &per_page=0 doesn't make any sense to me. When would you want to use that? Instead, it could disable paging.

@biteasy
Copy link
Owner

biteasy commented Jun 10, 2014

Disabling paging would be bad. That means that API callers would be able to request ALL data from the database which clearly isn't the best thing for performance :)

There is a limit on the per_page parameter so you can not do this basically &per_page=9999999999999 unless you discovered a bug somewhere. What we can definitely do is to increase the max per_page limit. It was pretty small at first just to see how things will go but now I am pretty confident that it can be bigger.

per_page = 0 is probably pointless to do but why expect different results if you use 0? We could add some kind of check that 0 is invalid but on the other hand this might be too much error handling for a simple thing like this that can just return empty results. It's not invalid from my point of view anyway; It's just a filter that returns empty results.

What exactly is your use case that you need paging disabled?

@schildbach
Copy link
Author

My usecase is querying all UXTOs for an address (or maybe later multiple addresses). If I start iterating all the pages it can't be any less stress on your server?!? I guess it would be better to adjust the rate limit to the size of results. E.g. 100 UXTOs per second rather than the current 4 requests per second.

In any case, 40 is very strict limit for this usecase.

@biteasy
Copy link
Owner

biteasy commented Jun 10, 2014

OK, I think if we increase the limits to 200 it will cover most cases and one API call will be sufficient to get all the data. I can't just remove the paging, even huge companies like Facebook have limits to their API results but I will definitely increase that limit; 40 is indeed too strict.

I will check a couple of things in the API first and make the change in the next couple of days. Expect an update soon!

@schildbach
Copy link
Author

200 will indeed cover the usecase pretty well for now. Can you add a magic constant for "max"? I just don't want to hard-code "200" into my app and when you decide to lower that limit to 100 the app fails completely. This is why I suggested to use "0" for this. Something like &per_page=MAX would also do.

I did not argue against API limits. I just think that if I want to get a thousand results from your server, its stressing your server more to get it in 25 requests (40 results per request) than to get it as one. So your current limits work against you.

@biteasy
Copy link
Owner

biteasy commented Jun 10, 2014

I like the MAX constant idea. It will be done.

@biteasy
Copy link
Owner

biteasy commented Jun 18, 2014

I increased the limit to 200 so you can hardcode it on your side for a couple more days and I will put it production the MAX constant parameter very soon. Let me know if the increased limit worked out for you.

@schildbach
Copy link
Author

Thanks. I will wait for the MAX constant and report back with the final tests.

@biteasy biteasy closed this as completed Jun 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants