perf: resolve atmos versions with less GitHub API calls - #116
perf: resolve atmos versions with less GitHub API calls#116Jochem Lugtenburg (jochem725) wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesAtmos release resolution
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/__tests__/setup-atmos.test.ts (2)
98-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the ARM release fixture.
githubReleaseArmis a GitHub API response fixture. Put it undersrc/__fixtures__/and import it into this suite.As per coding guidelines, place related fixtures under
src/__fixtures__/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/setup-atmos.test.ts` around lines 98 - 112, Move the githubReleaseArm fixture out of setup-atmos.test.ts into the shared src/__fixtures__ location, export it from the fixture module, and import it into the test suite where it is used. Preserve the fixture’s current values and structure.Source: Coding guidelines
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
nockfor the GitHub release API responses.
jest.mock("octokit")replaces the module used bycreateOctokit, so these tests only exercise local method calls. Usenockwith the availablenock/jest-nockdependency to mock the tagged, latest, and paginated release requests, and includeafterEach(() => nock.cleanAll())around the matching tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/setup-atmos.test.ts` around lines 29 - 35, Replace the octokit method mocks in the affected tests with nock interceptors for the tagged, latest, and paginated GitHub release API requests used by createOctokit. Configure each response to match the expected request and add afterEach(() => nock.cleanAll()) around these tests to remove interceptors between cases.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/__tests__/setup-atmos.test.ts`:
- Around line 98-112: Move the githubReleaseArm fixture out of
setup-atmos.test.ts into the shared src/__fixtures__ location, export it from
the fixture module, and import it into the test suite where it is used. Preserve
the fixture’s current values and structure.
- Around line 29-35: Replace the octokit method mocks in the affected tests with
nock interceptors for the tagged, latest, and paginated GitHub release API
requests used by createOctokit. Configure each response to match the expected
request and add afterEach(() => nock.cleanAll()) around these tests to remove
interceptors between cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 57e13b61-fc59-4c0f-9e55-fb9fb0d7e25b
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (3)
.github/workflows/main.ymlsrc/__tests__/setup-atmos.test.tssrc/installer.ts
Version resolution paginated the entire cloudposse/atmos release list (~29 requests) on every job, before checking the tool cache. At drift-scan scale this exhausts the rate limit and trips GitHub's secondary/burst limit. - getAtmos: for an exact version, probe the tool cache before any network call (0 calls on a warm cache). Gated on semver.valid so ranges never match a stale cached version. - getMatchingVersion: resolve exact versions via getReleaseByTag and `latest` via getLatestRelease (1 call each), each falling back to enumeration on a miss so resolution can never regress. - resolveFromReleaseList: paginate the range/fallback path newest-first with per_page 100 and stop as soon as a match is found (~9 requests down to ~1). Adds call-count and fallback tests; rebuilds dist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e858ba1 to
b35d120
Compare
1 participant
what
This aims to reduce the number of calls to Github by:
latestvia getLatestRelease (1 call each), each falling back to enumeration on a miss.why
Version resolution paginates the entire cloudposse/atmos release list on every job, even when the tool cache is used. When running drift-detection in large projects this quickly runs into Github rate limits with a large number of calls per job.
references
N/A