refactor(dx): replace loadWithPagination with getAllItems#2942
Open
refactor(dx): replace loadWithPagination with getAllItems#2942
Conversation
Contributor
📝 WalkthroughWalkthroughA migration from the deprecated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
stalniy
force-pushed
the
refactor/dx-replace-loadWithPagination
branch
2 times, most recently
from
8a64763 to
0057fbc
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2942 +/- ##
==========================================
- Coverage 60.92% 60.28% -0.64%
==========================================
Files 1023 1009 -14
Lines 24479 23866 -613
Branches 6057 5940 -117
==========================================
- Hits 14913 14388 -525
+ Misses 8354 8275 -79
+ Partials 1212 1203 -9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
stalniy
force-pushed
the
refactor/dx-replace-loadWithPagination
branch
from
0057fbc to
e3bd9e2
Compare
Replace all usages of the deprecated loadWithPagination utility with getAllItems from @akashnetwork/http-sdk, which includes circular loop detection. Remove dead loadWithPagination code from deploy-web, http-sdk, and indexer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
stalniy
force-pushed
the
refactor/dx-replace-loadWithPagination
branch
from
e3bd9e2 to
7c64738
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/deploy-web/src/queries/useLeaseQuery.ts (1)
55-55: Avoid usinganytype for thedeploymentparameter.The
deploymentparameter is typed asany, which violates the coding guidelines. Consider using a more specific type likePick<DeploymentDto, "dseq" | "groups"> | undefinedto match the pattern ingetDeploymentLeases.♻️ Proposed fix
-async function getAllLeases(chainApiHttpClient: AxiosInstance, address: string, deployment?: any) { +async function getAllLeases(chainApiHttpClient: AxiosInstance, address: string, deployment?: Pick<DeploymentDto, "dseq" | "groups">) {As per coding guidelines: "Never use type
anyor cast to typeany. Always define the proper TypeScript types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.ts` at line 55, Replace the broad any type on the deployment parameter of getAllLeases with a specific type matching the pattern used in getDeploymentLeases (e.g., Pick<DeploymentDto, "dseq" | "groups"> | undefined); update the function signature async function getAllLeases(chainApiHttpClient: AxiosInstance, address: string, deployment?: Pick<DeploymentDto, "dseq" | "groups"> | undefined) and any internal usages expecting deployment to rely on those fields so the compiler can typecheck correctly and avoid any casts to any.apps/deploy-web/src/queries/useLeaseQuery.spec.tsx (2)
231-232: Consider using more specific assertion here as well.Same suggestion as above - using
expect.objectContaining()would verify the pagination params are correctly passed.♻️ Proposed improvement
- expect(chainApiHttpClient.get).toHaveBeenCalledWith(expect.stringContaining("filters.owner=test-address"), expect.anything()); + expect(chainApiHttpClient.get).toHaveBeenCalledWith( + expect.stringContaining("filters.owner=test-address"), + expect.objectContaining({ params: expect.objectContaining({ "pagination.limit": 1000, "pagination.count_total": "true" }) }) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` around lines 231 - 232, The second assertion is too loose about the second argument passed to chainApiHttpClient.get; update the test so the call is asserted with expect.objectContaining to verify pagination params are included (e.g., page, limit, or other params) rather than using expect.anything(); locate the test that calls chainApiHttpClient.get and change the second argument expectation to expect.objectContaining({...}) while keeping the existing URL assertion and the result.current.data equality check using leaseToDto(mockLeases[0], undefined as any).
156-157: Consider using more specific assertion for the options parameter.While
expect.anything()works, the test inuseGrantsQuery.spec.tsxusesexpect.objectContaining()to verify the pagination params are correctly passed. This provides better test coverage for the refactored API.♻️ Proposed improvement for more precise assertion
- expect(chainApiHttpClient.get).toHaveBeenCalledWith(expect.stringContaining(`filters.dseq=${mockDeployment.dseq}`), expect.anything()); + expect(chainApiHttpClient.get).toHaveBeenCalledWith( + expect.stringContaining(`filters.dseq=${mockDeployment.dseq}`), + expect.objectContaining({ params: expect.objectContaining({ "pagination.limit": 1000, "pagination.count_total": "true" }) }) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx` around lines 156 - 157, The test currently uses expect.anything() for the options argument to chainApiHttpClient.get; update the assertion to use expect.objectContaining to verify the expected query/pagination options are passed (e.g., replace expect.anything() with expect.objectContaining({...}) so the call to chainApiHttpClient.get includes the pagination/params you expect). Locate the assertion in useLeaseQuery.spec.tsx referencing chainApiHttpClient.get and ensure the object contains the relevant keys (pagination/limit/offset or params) while keeping the URL check (expect.stringContaining(`filters.dseq=${mockDeployment.dseq}`)) and the result.current.data / leaseToDto expectation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/http-sdk/src/deployment/deployment-http.service.ts`:
- Around line 170-174: The pagination callback passed to getAllItems (which
fetches DeploymentInfo using extractData and this.httpClient.get against baseUrl
with params including defaultLimit) lacks a request timeout; update the options
object passed to this.httpClient.get inside that callback to include timeout:
30000 (matching other calls in the file) so each paginated HTTP request will
time out after 30s and prevent hangs during full-list retrievals.
---
Nitpick comments:
In `@apps/deploy-web/src/queries/useLeaseQuery.spec.tsx`:
- Around line 231-232: The second assertion is too loose about the second
argument passed to chainApiHttpClient.get; update the test so the call is
asserted with expect.objectContaining to verify pagination params are included
(e.g., page, limit, or other params) rather than using expect.anything(); locate
the test that calls chainApiHttpClient.get and change the second argument
expectation to expect.objectContaining({...}) while keeping the existing URL
assertion and the result.current.data equality check using
leaseToDto(mockLeases[0], undefined as any).
- Around line 156-157: The test currently uses expect.anything() for the options
argument to chainApiHttpClient.get; update the assertion to use
expect.objectContaining to verify the expected query/pagination options are
passed (e.g., replace expect.anything() with expect.objectContaining({...}) so
the call to chainApiHttpClient.get includes the pagination/params you expect).
Locate the assertion in useLeaseQuery.spec.tsx referencing
chainApiHttpClient.get and ensure the object contains the relevant keys
(pagination/limit/offset or params) while keeping the URL check
(expect.stringContaining(`filters.dseq=${mockDeployment.dseq}`)) and the
result.current.data / leaseToDto expectation unchanged.
In `@apps/deploy-web/src/queries/useLeaseQuery.ts`:
- Line 55: Replace the broad any type on the deployment parameter of
getAllLeases with a specific type matching the pattern used in
getDeploymentLeases (e.g., Pick<DeploymentDto, "dseq" | "groups"> | undefined);
update the function signature async function getAllLeases(chainApiHttpClient:
AxiosInstance, address: string, deployment?: Pick<DeploymentDto, "dseq" |
"groups"> | undefined) and any internal usages expecting deployment to rely on
those fields so the compiler can typecheck correctly and avoid any casts to any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f19c9e45-2a97-405a-8c63-403a8f5c3618
⛔ Files ignored due to path filters (1)
packages/net/src/generated/netConfigData.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
apps/deploy-web/src/queries/useDeploymentQuery.tsapps/deploy-web/src/queries/useGrantsQuery.spec.tsxapps/deploy-web/src/queries/useGrantsQuery.tsapps/deploy-web/src/queries/useLeaseQuery.spec.tsxapps/deploy-web/src/queries/useLeaseQuery.tsapps/deploy-web/src/services/wallet-balances/wallet-balances.service.tsapps/deploy-web/src/utils/apiUtils.tsapps/indexer/src/shared/utils/query.tspackages/http-sdk/src/deployment/deployment-http.service.tspackages/http-sdk/src/utils/pagination.utils.ts
💤 Files with no reviewable changes (3)
- apps/indexer/src/shared/utils/query.ts
- apps/deploy-web/src/utils/apiUtils.ts
- packages/http-sdk/src/utils/pagination.utils.ts
Comment on lines
+170
to
+174
| const allDeployments = await getAllItems<DeploymentInfo>(async params => { | ||
| const response = extractData( | ||
| await this.httpClient.get<DeploymentListResponse>(baseUrl, { | ||
| params: { "pagination.limit": defaultLimit, ...params } | ||
| }) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether HttpClient/axios instance sets a global timeout
fd -i "httpClient.ts" packages --type f
fd -i "http.service.ts" packages --type f
rg -n --type=ts -C3 'timeout\s*:' packages/http-sdk/src
# Compare timeout usage in this service's pagination branches
rg -n --type=ts -C4 'getAllItems|deployments/list|timeout' packages/http-sdk/src/deployment/deployment-http.service.tsRepository: akash-network/console
Length of output: 5871
🏁 Script executed:
cat -n packages/http-sdk/src/utils/httpClient.ts | head -100Repository: akash-network/console
Length of output: 1279
🏁 Script executed:
rg -n "timeout|getAllItems" packages/http-sdk/src/utils/pagination.utils.ts -A 10 -B 2Repository: akash-network/console
Length of output: 533
🏁 Script executed:
cat -n packages/http-sdk/src/utils/pagination.utils.tsRepository: akash-network/console
Length of output: 1275
🏁 Script executed:
rg -n "constructor|httpClient\s*=" packages/http-sdk/src/deployment/deployment-http.service.ts -A 3 -B 3 | head -30Repository: akash-network/console
Length of output: 322
Add timeout to the pagination fetch loop to prevent hangs.
The callback in getAllItems (lines 170-174) performs repeated HTTP calls without an explicit timeout. Since HttpClient has no global default timeout, this path can hang indefinitely if the API becomes unresponsive, stalling full-list retrievals. Other calls in the same file (lines 149, 206) use timeout: 30000—this branch should match that pattern.
Suggested patch
const allDeployments = await getAllItems<DeploymentInfo>(async params => {
const response = extractData(
await this.httpClient.get<DeploymentListResponse>(baseUrl, {
- params: { "pagination.limit": defaultLimit, ...params }
+ params: { "pagination.limit": defaultLimit, ...params },
+ timeout: 30000
})
);
return { items: response.deployments, pagination: response.pagination };
});📝 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.
Suggested change
| const allDeployments = await getAllItems<DeploymentInfo>(async params => { | |
| const response = extractData( | |
| await this.httpClient.get<DeploymentListResponse>(baseUrl, { | |
| params: { "pagination.limit": defaultLimit, ...params } | |
| }) | |
| const allDeployments = await getAllItems<DeploymentInfo>(async params => { | |
| const response = extractData( | |
| await this.httpClient.get<DeploymentListResponse>(baseUrl, { | |
| params: { "pagination.limit": defaultLimit, ...params }, | |
| timeout: 30000 | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/http-sdk/src/deployment/deployment-http.service.ts` around lines 170
- 174, The pagination callback passed to getAllItems (which fetches
DeploymentInfo using extractData and this.httpClient.get against baseUrl with
params including defaultLimit) lacks a request timeout; update the options
object passed to this.httpClient.get inside that callback to include timeout:
30000 (matching other calls in the file) so each paginated HTTP request will
time out after 30s and prevent hangs during full-list retrievals.
baktun14
approved these changes
Apr 19, 2026
ygrishajev
approved these changes
Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
loadWithPaginationis deprecated in favor ofgetAllItemsfrom@akashnetwork/http-sdkgetAllItemsincludes circular loop detection, preventing infinite pagination loopsWhat
loadWithPaginationwithgetAllItemsfrom@akashnetwork/http-sdkin:apps/deploy-web/src/queries/useDeploymentQuery.tsapps/deploy-web/src/queries/useGrantsQuery.tsapps/deploy-web/src/queries/useLeaseQuery.tsapps/deploy-web/src/services/wallet-balances/wallet-balances.service.tspackages/http-sdk/src/deployment/deployment-http.service.tsloadWithPaginationandhasQueryParamfrom:apps/deploy-web/src/utils/apiUtils.tspackages/http-sdk/src/utils/pagination.utils.tsloadWithPaginationcode fromapps/indexer/src/shared/utils/query.tsSummary by CodeRabbit
Release Notes