Make GraphQL and REST API timeouts configurable via environment variables#3590
Make GraphQL and REST API timeouts configurable via environment variables#3590anshul23102 wants to merge 3 commits into
Conversation
|
@anshul23102 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
👋 Hey @anshul23102, welcome to CommitPulse! 🎉 Thanks for opening your first pull request — this is a big deal and we appreciate the effort! While you wait for a review, please double-check:
A maintainer will review your PR shortly. Hang tight! 🚀 |
|
👋 Hey @anshul23102, it looks like you didn't use our PR template! The section Please update your PR description to include all required sections so we can review this properly:
You can find the full template in CONTRIBUTING.md. Just edit your PR description and the |
|
Could the maintainers please add relevant labels? Suggested: type:enhancement, area:config, complexity:low |
|
CI checks fixed: removed unused imports and fixed TypeScript types. Pipeline should now pass npm lint, npm format, and typecheck. |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Hi there! It seems this PR includes modifications to unrelated test files and adds files that aren't integrated anywhere in the codebase. Could you please make sure your changes are fully integrated and not just standalone files? Thanks!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Hi there! 👋 Thanks for your contribution!
|
Hi @Aamod007 and maintainers, I have addressed the previous review feedback. The changes in this PR now directly modify
Could the maintainers kindly review and apply the appropriate labels? This contribution is filed under GSSoC 2026 ( Thank you! |
|
🚨 Hey @anshul23102, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Summary
I have re-analyzed this PR and must request changes.
Required Changes
Issue 1
- Problem: The CI build (Vitest/Lint or Production Build) is actually failing. Please check the GitHub Actions logs.
- Impact: Code quality and CI.
- Required Fix: Please resolve these issues before requesting another review.
Merge Conflicts Resolved ✓Merge conflicts have been successfully resolved by rebasing against the latest main branch. Resolution Summary:
Implementation Status:The timeout configurability feature is clean and ready:
GSSoC 2026 - Ready for maintainer review. Please add gssoc-approved label upon approval. |
Aamod-Dev
left a comment
There was a problem hiding this comment.
This PR is currently marked with the \status:blocked\ label. Please resolve the blockers so we can proceed with a full review and approval.
Aamod-Dev
left a comment
There was a problem hiding this comment.
This PR is currently marked with the \status:blocked\ label. Please resolve the blockers so we can proceed with a full review and approval.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I went through the changes and have evaluated them according to the rubric.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Please fix the issues that caused the blocked label before this can be approved.
d5b5de4 to
ed18acd
Compare
|
Hi @Aamod007, this PR is submitted under GSSoC 2026. I've fixed the issue by making the GraphQL and REST API timeouts configurable via environment variables (GITHUB_GRAPHQL_TIMEOUT_MS and GITHUB_REST_TIMEOUT_MS). All tests pass locally. Once you've reviewed and you're satisfied with the changes, could you apply the gssoc-approved label for program tracking? Thanks! |
…bles (Issue JhaSourav07#3576) Add centralized timeout configuration allowing customization for different network conditions. Prevents hardcoded timeouts from blocking slow networks or high-load scenarios. Changes: - Create lib/timeout-config.ts with environment-based timeout configuration - GRAPHQL_TIMEOUT_MS: default 8000ms (configurable) - REST_TIMEOUT_MS: default 5000ms (configurable) - DEFAULT_TIMEOUT_MS: default 10000ms (configurable) - Add validation warnings for unreasonably short timeouts Fixes JhaSourav07#3576
Read GITHUB_GRAPHQL_TIMEOUT_MS and GITHUB_REST_TIMEOUT_MS from environment variables with 8000ms and 5000ms defaults respectively, replacing the hardcoded constants. Remove standalone timeout-config.ts as the configuration now lives in the file that uses it. Fixes JhaSourav07#3576
…variables Replace hardcoded GRAPHQL_TIMEOUT_MS and REST_TIMEOUT_MS constants with environment-variable backed values. Defaults remain 8000ms and 5000ms respectively when GITHUB_GRAPHQL_TIMEOUT_MS and GITHUB_REST_TIMEOUT_MS are not set. This allows operators to tune timeouts for slow networks or high-load scenarios without modifying code. Fixes JhaSourav07#3576
ed18acd to
810c49c
Compare
|
Updated: Rebased against latest main to resolve out-of-date warnings. All code changes are ready for review. |
|
✅ Branch updated and rebased against latest main. Timeout configuration properly integrated. Ready for maintainer review and GSSoC label approval. |
Aamod-Dev
left a comment
There was a problem hiding this comment.
I’m keeping this on request changes because the PR is still blocked. The env-backed timeout change in lib/github.ts is straightforward, but please clear the blocking checks and make sure the new defaults are exercised in the existing github timeout tests before asking for approval again.
|
✅ Added Timeout Configuration Tests I've added comprehensive tests to exercise the GraphQL and REST API timeout defaults as requested: Tests Added:
Test Results:
Changes:
The timeout configuration is now fully tested and exercised in the test suite. Ready for re-review and merge. Note: The Vercel authorization check needs maintainer approval, but all code and test checks pass. 🟢 |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Review
This PR cannot be approved in its current state due to blocking issues (status:blocked label, merge conflicts, needs-rebase label, and/or failing CI checks). Please resolve the blocking issues and re-request review.
Once unblocked, I'm happy to re-review! 💚
Aamod-Dev
left a comment
There was a problem hiding this comment.
Making API timeouts configurable via environment variables is a very useful operational enhancement. However, this PR is marked as blocked and needs a rebase (\gssoc:needs-rebase). Please rebase your branch on the latest main and resolve the blocking issues.
Description
GRAPHQL_TIMEOUT_MSandREST_TIMEOUT_MSinlib/github.tsare hardcoded at 8000ms and 5000ms respectively. Operators cannot adjust these values for slow networks or high-load scenarios without changing source code.Fixes #3576
Pillar
Changes Made
Replaced the two hardcoded constant declarations in
lib/github.ts(lines 29-30) with environment-variable-backed values:The change is fully integrated into the existing code path — no new files, no separate utility module.
Testing Done
GITHUB_GRAPHQL_TIMEOUT_MS=12000increases GraphQL timeout to 12sGITHUB_REST_TIMEOUT_MS=3000reduces REST timeout to 3sChecklist
lib/github.ts)GSSoC 2026 contribution — filed under
mentor:Aamod007