Conversation
|
Great addition! Could you please implement the paging stuff for all API routes that support paging? Having paging for route A, but not for route B might be confusing and nobody exactly knows which API routes have been covered already. Thanks in advance, |
|
Sure, but I have another idea. List methods could return not a list but an iterable object just like Django ORM's QuerySet, which supports slicing. And this iterable object will hide internal github's paging implementation from the end user of the python-github2. For example, they could iterate over all commits using: And github2 will make as many requests to the GitHub as required. Or user coul only fetch first 100 commits: And github2 will do no more than 3 requests to the GitHub behind the scene? I see only one con against this approach. It is backward incompartible with the previous code and to convert old code which uses github2 to work with new library, you have to use limits and perhaps lists: If previously you wrote I suggest this implementation because without that, many developers who using github2, will have to implement same logic in their own code. For example, here is my curren implementation of the generator: By the way, such generator could be generalized to use with any listable github's object if it will support the page argument in it's list method. And this will allow us to keep backward compatibility. But this approach is a compromise :( |
|
I think the addition of an iterator hiding the details of paging is a great fit for this library. The backward incompatibility is a bit tricky, so I'm not too fond of changing the list method. Instead, we could solve this by providing an By the way: in your code sample you call |
|
We could even let the That way, you can simply call: |
|
Of cause, you can't apply [:LIMIT] to an iterator, but you can do this with any iterable object. This will look like this for end user: I'll try to write the implementation within few days, and will send you another pull request. |
|
Yeah, what I meant is that (lazy) iterables are not subscriptable (at least not up until Python 2.6), so the above code yields a syntax error. See this: However, if you wrap it in an islice wrapper, you can subscript these kinds of infinite lists: An alternative is to resolve the iterator's elements, which looses the laziness (and therefore doesn't work with infinite lists). This would effectively fetch ALL commit objects in GitHub in one big list and then slice the first 50 out of that result. |
|
Generators like this one are not subscribtable, but this does not mean, that I can't return in immediate object from |
|
Hey Alexander, how's your patch progressing? |
|
Hi. Sorry for delay. It is hard to do this in stable manner, because there is no any documentation from the GitHub and I found no way to tell GitHub the page size. By default, page size is 35 items. I need to know that this value will never changed or (better) to specify it myself. Without this, I can't to implement a reliable slicing. If you want, I could try to implement it and hardcode that page's size is 35 items, but it could be broken someday. |
|
You can safely use the static page size, technoweenie himself promised me it won't change for the v2 API. |
|
Ok, I wrote this helper, to solve this task: https://github.com/svetlyak40wt/pagerator For our case, gh.commits.iter() method should return be: But I can't test it right now, because I receive "api route not recognized" error from the GitHub, even for list method which worked previously. Do you know how to fix it? |
|
@ask any news on this ? For the first time i have now pages on my project (not only on commits, but every list) and with the current code i can't access them. And i don't know if you want the |
|
i made this : #66 |
|
Given the API v2 switchoff these bugs aren't going to be worked on / fixed. |
|
Yes of course :) |
Hi, I added an optional 'page' argument to receive not only last 35 commits, but all others too. Think, such approach could be useful for other list operations too.