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

Paging for GetRepositoryCommitsAsync does not seem to work #2

Closed
BjartN opened this issue Mar 14, 2020 · 3 comments
Closed

Paging for GetRepositoryCommitsAsync does not seem to work #2

BjartN opened this issue Mar 14, 2020 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@BjartN
Copy link

BjartN commented Mar 14, 2020

The call await client.GetRepositoryCommitsAsync(workspace, repoSlug, 20) gives the same page 20 times. (I am using the version 1.0.0 Nuget package)

@lvermeulen
Copy link
Owner

Do you have a repository on which I can reproduce this?

@lvermeulen lvermeulen self-assigned this Mar 14, 2020
@lvermeulen lvermeulen added the bug Something isn't working label Mar 14, 2020
@BjartN
Copy link
Author

BjartN commented Mar 14, 2020

I don't at the moment. This piece of code seems to be iterating over "numPages" without ever using the value of it. I'm assuming this is the problem.

Hacking it by adding "queryParamValues["page"] = numPages;" in the while loop would perhaps fix it.

`
private async Task<IEnumerable> GetPagedResultsAsync(int? maxPages, IDictionary<string, object> queryParamValues, Func<IDictionary<string, object>, Task<PagedResults>> selector)
{
var results = new List();
bool isLastPage = false;
int numPages = 0;

		while (!isLastPage && (maxPages == null || numPages < maxPages))
		{
			var selectorResults = await selector(queryParamValues).ConfigureAwait(false);
			results.AddRange(selectorResults.Values);

			isLastPage = selectorResults.Next == null;
			numPages++;
		}

		return results;
	}

`

lvermeulen added a commit that referenced this issue Mar 18, 2020
@lvermeulen
Copy link
Owner

Found it! :-) It was actually a little bit more involved than that, have a look at GetPagedResultsAsync(). The devil is in the Math.Max details. Thanks for reporting this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants