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

Implement Cursor #5

Closed
wants to merge 3 commits into from
Closed

Implement Cursor #5

wants to merge 3 commits into from

Conversation

pitbulk
Copy link
Contributor

@pitbulk pitbulk commented Oct 3, 2017

No description provided.

@pitbulk pitbulk changed the base branch from master to new_settings October 3, 2017 17:13
@korsosa
Copy link
Contributor

korsosa commented Nov 4, 2017

Nice!
I have one concern though, that this change would break the current usages, as everyone would need to put .objects() wherever get_users(), get_roles(), get_events() or get_groups() are used. I'm thinking that maybe if Cursor would be iterable, then this wouldn't be required and it could be backwards compatible.

Personally I use PyGithub a lot which is a python client for Github and I found their solution really good and convenient. They named it PaginatedList which is similar to this cursor, but it's iterable and fetches the next batch of data from the API only when needed: https://github.com/PyGithub/PyGithub/blob/master/github/PaginatedList.py

This way it can be used in loops just like lists without extra conversion, can have extra methods like _totalCount() or reverse() and also has lazy-loading, requesting new data from the API only when needed and not fetching all pages in the beginning.

What do you think, would this make sense and is it feasible?

@pitbulk
Copy link
Contributor Author

pitbulk commented Nov 4, 2017

Yes, I think the ruby version of the SDK is similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants