Skip to content

chore: update @octokit/rest from v16 to v17#371

Merged
Trott merged 2 commits intonodejs:mainfrom
Trott:update-to-rest-17
Mar 2, 2023
Merged

chore: update @octokit/rest from v16 to v17#371
Trott merged 2 commits intonodejs:mainfrom
Trott:update-to-rest-17

Conversation

@Trott
Copy link
Copy Markdown
Member

@Trott Trott commented Feb 27, 2023

Replace deprecated .hasLastPage() with pagination. This enables us to bump @octokit/rest from v16 to v17.

@Trott Trott requested a review from a team as a code owner February 27, 2023 20:46
@Trott
Copy link
Copy Markdown
Member Author

Trott commented Mar 1, 2023

@nodejs/github-bot

@Trott Trott merged commit ad3903c into nodejs:main Mar 2, 2023
@Trott Trott deleted the update-to-rest-17 branch March 2, 2023 21:29
const res = await githubClient.pulls.listCommits({
async function findLatestCommitInPr (options) {
const { Octokit } = require('@octokit/rest')
const githubClient = new Octokit({})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it deliberate to not use githubClient from line 3?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was probably a blunder. Sorry about that!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this why we're hitting rate limits? The github-bot logs now have lots of "API rate limit exceeded for 23.253.100.79. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)" errors, e.g.

{"name":"bot","hostname":"infra-rackspace-debian8-x64-1","pid":15899,"req_id":"e62c49d0-bbaa-11ed-9410-77b81a7c3334","identifier":"node-test-linux-ubuntu1804-64","event":"end","level":50,"err":{"message":"API rate limit exceeded for 23.253.100.79. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","name":"HttpError","stack":"HttpError: API rate limit exceeded for 23.253.100.79. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)\n    at /home/iojs/github-bot/node_modules/@octokit/request/dist-node/index.js:86:21\n    at runMicrotasks (<anonymous>)\n    at processTicksAndRejections (internal/process/task_queues.js:93:5)\n    at async Object.next (/home/iojs/github-bot/node_modules/@octokit/plugin-paginate-rest/dist-node/index.js:112:28)\n    at async findLatestCommitInPr (/home/iojs/github-bot/lib/push-jenkins-update.js:88:23)","code":403},"msg":"Error while handling Jenkins end event","time":"2023-03-05T23:10:20.025Z","v":0}

Copy link
Copy Markdown
Member Author

@Trott Trott Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, sending a PR momentarily. I wouldn't have realized this would caused this kind of problem. Sorry about that.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opportunity to learn why Github requests better be authenticated 👍🏻

const commitsFixturePage1 = readFixture('pull-request-commits-page-1.json')
const commitsFixturePage2 = readFixture('pull-request-commits-page-2.json')
const commitsFixturePage3 = readFixture('pull-request-commits-page-3.json')
const commitsFixturePage4 = readFixture('pull-request-commits-page-4.json')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray for tests being maintained and adjusted accordingly

@phillipj
Copy link
Copy Markdown
Member

phillipj commented Mar 5, 2023

@Trott sorry for the late reply, hectic week on my end...

Looks promising from a distance 👍🏻

@phillipj
Copy link
Copy Markdown
Member

phillipj commented Mar 6, 2023 via email

@richardlau richardlau mentioned this pull request Mar 6, 2023
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.

3 participants