fix: update code to support @octokit/request v9 and @octokit/graphql v8 #3705
Conversation
Fixes asyncapi#3690 Update code to support `@octokit/request` v9.2.2 and `@octokit/graphql` v8.2.1. * **netlify/functions/github_discussions.ts** - Remove unused import of `GraphQlQueryResponseData`. - Update `graphql` function call to use the new syntax. - Update authorization header to use the new token format. * **netlify/functions/save-discussion-background/helpers.ts** - Update authorization header to use the new token format.
Recxsmacx
requested review from
derberg,
akshatnema,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
|
Warning Rate limit exceeded@Recxsmacx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces changes in two areas. In the GitHub Actions workflow for Lighthouse CI, a new always-run step for uploading Lighthouse report artifacts has been added, complete with a specified retention period and error-handling configuration. In two Netlify function files, the GraphQL request structure was refined, with adjustments including the removal of an import and an update to the authorization header format—from using the Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Lighthouse as Lighthouse Audit
participant Upload as Upload Artifacts Step
Note over CI: Workflow Triggered
CI->>Lighthouse: Run Lighthouse Audit
Lighthouse-->>CI: Generate Lighthouse Report
CI->>Upload: Execute Upload Artifacts (always runs)
Upload-->>CI: Upload "lighthouse-report" (5-day retention)
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Hello, @coderabbitai[bot]! 👋🏼 |
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Tip For best results, initiate chat on the files or code changes. 🧠 Learnings used |
|
@derberg @devilkiller-ag @akshatnema @sambhavgupta0705 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3705 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 732 732
=========================================
Hits 732 732 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/lighthouse-ci.yml(1 hunks)netlify/functions/github_discussions.ts(1 hunks)netlify/functions/save-discussion-background/helpers.ts(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/lighthouse-ci.yml
99-99: the runner of "actions/upload-artifact@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 180000ms (1)
- GitHub Check: Test NodeJS PR - macos-13
🔇 Additional comments (2)
netlify/functions/github_discussions.ts (2)
27-40: LGTM! GraphQL query structure improved.The restructured GraphQL query with a dedicated object parameter improves code organization and aligns with @octokit/graphql v8 requirements.
37-37: LGTM! Authorization header updated consistently.The change from
tokentoBearerprefix maintains consistency with the helper file and aligns with OAuth 2.0 standards.
| @@ -18,7 +18,7 @@ export function toTitleCase(title: string): string { | |||
| export async function fetchGraphql(query: string): Promise<any> { | |||
| const parameters = { | |||
| headers: { | |||
| authorization: `token ${process.env.GITHUB_TOKEN}`, | |||
| authorization: `Bearer ${process.env.GITHUB_TOKEN}`, | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM! Authorization header updated to OAuth 2.0 standard format.
The change from token to Bearer prefix aligns with the OAuth 2.0 standard and the requirements of @octokit/graphql v8.
🏁 Script executed:
#!/bin/bash
# Description: Verify that all GitHub API calls use Bearer token format
# Test: Search for any remaining token-based auth. Expect: No occurrences.
rg -i 'authorization.*token\s+\$'Length of output: 545
FYI: Inconsistencies detected – further updates required for consistent OAuth 2.0 token usage.
LGTM for the changes in netlify/functions/save-discussion-background/helpers.ts where the Authorization header was updated to use the Bearer prefix in line with OAuth 2.0 and @octokit/graphql v8 requirements. However, our verification script found several other locations still using the old token prefix for the Authorization header. Please review and update the following files for consistency, unless these occurrences are intentionally exempted:
netlify/edge-functions/serve-definitions.ts: ContainsoriginalRequest.headers.set('Authorization', \token ${GITHUB_TOKEN}`);`tests/tools/extract-tools-github.test.js: Usesauthorization: \token ${process.env.GITHUB_TOKEN}``scripts/dashboard/build-dashboard.js: Contains multiple instances ofauthorization: \token ${process.env.GITHUB_TOKEN}``scripts/tools/extract-tools-github.js: Usesauthorization: \token ${process.env.GITHUB_TOKEN}``
Please address these inconsistencies to ensure a uniform authentication method across the codebase.
.github/workflows/lighthouse-ci.yml
Outdated
| - name: Upload artifacts | ||
| if: always() | ||
| uses: actions/upload-artifact@v2 | ||
| with: | ||
| name: lighthouse-report | ||
| path: ./path/to/lighthouse/report | ||
| retention-days: 5 | ||
| continue-on-error: true |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Update action version and verify artifact path.
The artifact upload step is a good addition, but there are a few issues to address:
- The action version is outdated. Update to the latest version.
- The artifact path seems generic and might not be correct.
Apply these changes:
- uses: actions/upload-artifact@v2
+ uses: actions/upload-artifact@v4
- path: ./path/to/lighthouse/report
+ path: ./lhci_reportsNote: Please verify that the path matches the actual location where Lighthouse CI generates its reports.
🏁 Script executed:
#!/bin/bash
# Description: Check if Lighthouse report path exists in the configuration
# Test: Search for Lighthouse report path in config. Expect: Path definition.
rg -l 'lhci.*report|lighthouse.*report' .github/Length of output: 86
Update the artifact upload configuration.
The artifact upload step is a great addition, but please address two concerns:
- Action version: The currently used version (
actions/upload-artifact@v2) is outdated. Please update it to the latest version (actions/upload-artifact@v4). - Artifact path: The configured path (
./path/to/lighthouse/report) appears overly generic. Ensure it accurately reflects the directory where Lighthouse CI generates its reports. For instance, if reports are generated in./lhci_reports, adjust the path accordingly.
Apply these changes:
- uses: actions/upload-artifact@v2
+ uses: actions/upload-artifact@v4
- path: ./path/to/lighthouse/report
+ path: ./lhci_reportsPlease verify that the updated artifact path aligns with the actual report output directory in your configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Upload artifacts | |
| if: always() | |
| uses: actions/upload-artifact@v2 | |
| with: | |
| name: lighthouse-report | |
| path: ./path/to/lighthouse/report | |
| retention-days: 5 | |
| continue-on-error: true | |
| - name: Upload artifacts | |
| if: always() | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: lighthouse-report | |
| path: ./lhci_reports | |
| retention-days: 5 | |
| continue-on-error: true |
🧰 Tools
🪛 actionlint (1.7.4)
99-99: the runner of "actions/upload-artifact@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Recxsmacx
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/lighthouse-ci.yml (1)
97-105: 🛠️ Refactor suggestion
⚠️ Potential issueFix YAML Indentation and Update Artifact Upload Action Version
The "Upload artifacts" step (lines 97–105) is currently suffering from inconsistent indentation in its
with:block. Specifically, the keysnameandpath(lines 101–102) are indented more thanretention-daysandcontinue-on-error(lines 103–104), which leads to a syntax error. Moreover, the step is still usingactions/upload-artifact@v2, which is outdated. It is advisable to update toactions/upload-artifact@v4and adjust the indentation so that all keys underwith:are uniformly indented.A suggested diff is as follows:
- - name: Upload artifacts - if: always() - uses: actions/upload-artifact@v2 - with: - name: lighthouse-report - path: path/to/artifact - retention-days: 5 - continue-on-error: true + - name: Upload artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: lighthouse-report + path: path/to/artifact + retention-days: 5 + continue-on-error: trueThis change will fix the YAML syntax and ensure that the workflow uses the latest supported version of the artifact upload action.
🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 101-101: wrong indentation: expected 10 but found 12
(indentation)
[error] 103-103: syntax error: expected , but found ''
(syntax)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/lighthouse-ci.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/lighthouse-ci.yml
96-96: could not parse as YAML: yaml: line 96: did not find expected key
(syntax-check)
🪛 YAMLlint (1.35.1)
.github/workflows/lighthouse-ci.yml
[warning] 101-101: wrong indentation: expected 10 but found 12
(indentation)
[error] 103-103: syntax error: expected , but found ''
(syntax)
⏰ Context from checks skipped due to timeout of 180000ms (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Test NodeJS PR - macos-13
| @@ -93,3 +93,12 @@ jobs: | |||
| number: ${{ github.event.pull_request.number }} | |||
| header: lighthouse | |||
| message: ${{ steps.lighthouse_score_report.outputs.comment }} | |||
|
|
|||
There was a problem hiding this comment.
Remove Extraneous Marker Causing YAML Parse Error
Line 96 appears to contain an extraneous marker (a lone ~ or similar) which is triggering a YAML parsing error ("did not find expected key"). Please remove this line to resolve the syntax issue.
🧰 Tools
🪛 actionlint (1.7.4)
96-96: could not parse as YAML: yaml: line 96: did not find expected key
(syntax-check)
|
@anshgoyalevil bro please check this one!! |
Fixes #3690
[Update code to support
@octokit/requestv9.2.2 and@octokit/graphqlv8.2.1.](fix: update code to support @octokit/request v9 and @octokit/graphql v8)netlify/functions/github_discussions.ts
GraphQlQueryResponseData.graphqlfunction call to use the new syntax.netlify/functions/save-discussion-background/helpers.ts
Summary by CodeRabbit
Summary by CodeRabbit