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

Using maximum limits automatically #66

Closed
ghost opened this issue Nov 11, 2019 · 6 comments
Closed

Using maximum limits automatically #66

ghost opened this issue Nov 11, 2019 · 6 comments
Assignees
Labels
enhancement New feature or improvement

Comments

@ghost
Copy link

ghost commented Nov 11, 2019

.. for reducing needed api calls and speeding up thing

@ghost
Copy link
Author

ghost commented Nov 11, 2019

Current defaults seems copied what api defines as default. They are very small and the maximums are not very large either.

for example categories default limit is now 20, maximum is 50. And api currently returns total categories over 20 but less than 50.

@felix-hilden felix-hilden added the consideration Future decision to be discussed label Nov 11, 2019
@ghost
Copy link
Author

ghost commented Nov 13, 2019

also quite inconsistent:
... album().tracks.limit -> 50 ... but album_tracks() uses limit 20

@felix-hilden
Copy link
Owner

I'd hate to change it precisely because it is the API default too. I agree it would speed up calls to lengthy resources. But for any such call it's easy to include the maximum limit.

tracks = s.all_items(s.album_tracks(album_id, limit=50))

Many times all items are not needed, and 20 might be fine. I think we'll be lads to the people over at Spotify and reduce their load a bit, or respect whatever their reason was for these defaults.

@marioortizmanero
Copy link
Contributor

I'm afraid OP deleted his account, I didn't know about @ghost :(

If you need someone's else opinion, I'd leave the default limits as they are too. It's really easy to modify anyway, as you pointed out.

@felix-hilden
Copy link
Owner

Yeah, I noticed, too bad. But I'll close this then, as some sort of consensus has been achieved.

@felix-hilden
Copy link
Owner

This should be revisited. It wouldn't be too hard to provide a parameter / context manager to turn on maximum limits.

@felix-hilden felix-hilden reopened this Feb 16, 2020
@felix-hilden felix-hilden added enhancement New feature or improvement and removed consideration Future decision to be discussed labels Feb 16, 2020
@felix-hilden felix-hilden changed the title consider setting default limits to maximum Using maximum limits automatically Feb 16, 2020
@felix-hilden felix-hilden self-assigned this Feb 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

No branches or pull requests

2 participants