Fixes 'ensureLabelsAreCreated' by paginating API results #26
Conversation
… api with new rest api.
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@elastic/snyk-github-issue-creator", | |||
| "version": "2.1.1", | |||
| "version": "2.1.2", | |||
There was a problem hiding this comment.
@watson Is there anything else I need to do to support rolling out a new version?
There was a problem hiding this comment.
The version number should be updated independently of the changes in a PR. So just revert any changes to this file in this PR. To prepare and publish a new version, do the following after this PR is merged:
# ensure you have the latest main checked out locally
git pull upstream main
# bump the patch version in package.json and create version commit + tag
npm version patch
# ensure the patch commit is pushed upstream
git push upstream main
# ensure the version tag is pushed upstream
git push upstream --tags
# publish the new version to npm
npm publish
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "@elastic/snyk-github-issue-creator", | |||
| "version": "2.1.1", | |||
| "version": "2.1.2", | |||
There was a problem hiding this comment.
The version number should be updated independently of the changes in a PR. So just revert any changes to this file in this PR. To prepare and publish a new version, do the following after this PR is merged:
# ensure you have the latest main checked out locally
git pull upstream main
# bump the patch version in package.json and create version commit + tag
npm version patch
# ensure the patch commit is pushed upstream
git push upstream main
# ensure the version tag is pushed upstream
git push upstream --tags
# publish the new version to npm
npm publish
lib/github/labels.js
Outdated
| const response = await octokit.paginate( | ||
| await client.issues.listLabelsForRepo({ | ||
| owner: ghOwner, | ||
| repo: ghRepo, | ||
| per_page: 100 | ||
| }) | ||
| ) |
There was a problem hiding this comment.
I don't see this way of passing the response as an argument to octokit.paginate in their docs 🤔 https://octokit.github.io/rest.js/v20#pagination
But I assume you tested it?
There was a problem hiding this comment.
Yes, it works! However, I have updated the code to be more explicit in this case.
const responseData = await octokit.paginate(
await client.issues.listLabelsForRepo({
owner: ghOwner,
repo: ghRepo,
per_page: 100
}),
(response) => response.data
)
lib/snyk.js
Outdated
| const baseUrl = 'https://snyk.io/api/v1' | ||
| const baseRestUrl = 'https://api.snyk.io' |
There was a problem hiding this comment.
As far as I can see, the old const isn't used anymore? Why not simply change its value to the new host?
| const baseUrl = 'https://snyk.io/api/v1' | |
| const baseRestUrl = 'https://api.snyk.io' | |
| const baseUrl = 'https://api.snyk.io' |
There was a problem hiding this comment.
Not all v1 APIs are deprecated. It's still used in the issues and orgs functions. I will rename the const though, since we have 2 base paths now.
We have > 100 labels in the security repo, and we were not paginating the call to retrieve them. Previously, this was working because the labels we wanted to apply to an issue (typically
Kibana,snyk, andtriage_needed) were within the first 100 results, but as new labels were created, some of the labels are now in the second page of results (specifically "triage_needed"). This causedensureLabelsAreCreatedto throw when attempting to create an existing label.This PR adds pagination of the
listLabelsForRepoAPI call.Additionally, this PR replaces the deprecated snyk List all projects v1 API with the newer REST API. The v1 API EOL is December 22, 2023. See the official statement for more information.