The Wayback Machine - https://web.archive.org/web/20201106140806/https://github.com/netlify/js-client/issues/39
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

Support pagination #39

Open
bcomnes opened this issue Feb 25, 2019 · 6 comments
Open

Support pagination #39

bcomnes opened this issue Feb 25, 2019 · 6 comments

Comments

@bcomnes
Copy link
Contributor

@bcomnes bcomnes commented Feb 25, 2019

We need to support pagination on calls.

@DavidWells
Copy link
Contributor

@DavidWells DavidWells commented Dec 6, 2019

Bumping this.

If you have more than 100 site associated with your account some CLI commands don't function properly

  • netlify sites:list - only pulls 100
  • netlify link and connect to site via git remote fails because repo not in the 100 sites returned.

Current workaround for netlify link is manually connecting the site-id

This behavior is also visible on https://oauth-example.netlify.com/ also using the js-client

A strategy for pagination is overdue 😃

@grahaml
Copy link

@grahaml grahaml commented Sep 25, 2020

Obligatory +1 -- I have 125 sites... I need that other 25 so badly.

@grahaml
Copy link

@grahaml grahaml commented Sep 25, 2020

Sorry for the cheeky post. Let me elaborate. I have been talking to Netlify support about this as well, but I thought it would be good to share the details directly on the issue here.

I am working on some analysis on all the sites in our account, specifically for user access management, and I have found that neither the JS client nor the CLI tool seem to support pagination. We have 125 sites live at the moment.

What I need to do is get a list of all sites under my account, which I believe the OpenAPI Endpoint listSitesForAccount should return.

When trying to use the JS client (or the CLI tool), calls that include page: n in the request return the same first 100 results (the first page).

Steps to Reproduce

Forgive the contrived example - trying to be explicit about pages for clarity

const Netlify = require('netlify');
const client = new Netlify('my_access_token');
const sitesPage1 = await client.listSitesForAccount({account_slug: 'my_account_slug'});
const sitesPage2 = await client.listSitesForAccount({account_slug: 'my_account_slug', page: 2});
const allSites = [...sitesPage1, ...sitesPage2];

Expected Results

  • sitesPage1 should contain 100 entries, the first page of sites
  • sitesPage2 should contain the remaining 25 sites that were not captured in the first page
  • Additionally I would expect the response from this call to inform me that the results are paginated, and that there are more pages to fetch

Actual Results

  • sitesPage1 does have the first 100 entries (the first page of sites)
  • sitesPage2 has the exact same 100 entries
  • There is no information about results being paginated or how to fetch the next page

Regarding the information about pagination, I realize that the docs say the Link header in the response should contain information about subsequent pages, as per https://docs.netlify.com/api/get-started/#link-header. The response headers, however, are not exposed in the JS client. Only the response body seems to be returned.


The same seems to be true with netlify-cli. I realize that this is in a different repository, but in case anyone else comes across this, I figure it doesn't hurt to include the information here as well.

This is my experience trying to use the CLI tool.

$ netlify login
# ...
$ netlify api listSitesForAccount --data '{"account_slug": "my_account_slug"}'
# ... returns the first 100 results
$ netlify api listSitesForAccount --data '{"account_slug": "my_account_slug", "page": 2}'
# ... returns the same first 100 results as above
@jverce
Copy link

@jverce jverce commented Sep 30, 2020

Should this pagination logic be exposed to users? Or could we implement a generator-based function that keeps returning items until the whole dataset is exhausted, and handle the pagination logic internally in the client?

@grahaml
Copy link

@grahaml grahaml commented Sep 30, 2020

This is a good question @jverce, and I think the answer depends on this library's purpose and intent.

If the intent is to provide an SDK that mirrors Netlify's Open API spec, then I would say that we should expose the headers or include some kind of indicator in the response body that there is another page to fetch, which would of course be a breaking change as OP indicated.

If the intent is to provide an SDK that acts as an abstraction layer over the Open API spec, then I'd argue that handling the pagination logic internally would make sense.

Having said that - I don't think the Open API spec mentions pagination at all. The only thing I could find is in the Netlify docs. So with that in mind, I suppose technically this behaves as expected (because it is as per the spec).

To anyone looking to get around this - I opted to forego this library and call the API directly, parsing the headers to determine if another call should be made. This is roughly what I came up with (naively assuming a GET request and appending page as a query param, and removing error handling for brevity):

function hasNextPage(headers) {
  // See https://github.com/netlify/micro-api-client/blob/master/src/pagination.js for inspiration
}

async function getFullPaginatedList(endpoint) {
	return function makeRequestForPage(page = 1, accumulatedResults = []) {
		let fullResults = accumulatedResults;
		const paginatedEndpoint = `${endpoint}?page=${page}`;
		const res = await fetch(paginatedEndpoint, {
			headers: {
				'Authorization': `Bearer ${ACCESS_TOKEN}`,
			}
		});
		if (res.ok) {
			const currentCallResults = await res.json();
			fullResults = fullResults.concat(currentCallResults);
			if (hasNextPage(res.headers)) {
				return makeRequestForPage(++page, fullResults);
			}
		}
		return fullResults;
	}
}

const getAllSites = getFullPaginatedList(`https://api.netlify.com/api/v1/${ACCOUNT_SLUG}/sites`);
const allSites = await getAllSites();
@jverce
Copy link

@jverce jverce commented Oct 4, 2020

Yeah, I can relate with that @grahaml, I had to do the same thing in my project since I need to support such cases.

I created a PR for the JS client to provide such functionality: #167
Given that datasets can grow indefinitely (e.g. listSiteDeploys will grow monotonically with every commit to a main branch), and also to maintain backwards compatibility, I made this functionality optional.

Next thing we can do is add a parameter that indicates how many records/items the user would want to retrieve, and play with that instead of the current all-or-100 approach.

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

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.