refactor: changes LeaseHttpService to accept http client#1888
Conversation
WalkthroughRefactors HTTP SDK wiring to construct Deployment/Cosmos/Lease services with a shared injected HttpClient (CHAIN_API_HTTP_CLIENT) via a consolidated NON_AXIOS_SERVICES factory; rewrites LeaseHttpService to accept HttpClient and use extractData/httpClient calls; notifications provider added token and providers for the shared client and services. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Container
participant CHAIN_API_HTTP_CLIENT as HttpClient
participant LeaseHttpService
participant ChainAPI
Note over Container,CHAIN_API_HTTP_CLIENT: Registration phase
Container->>Container: register(CHAIN_API_HTTP_CLIENT, useFactory -> createHttpClient(...))
Container->>Container: NON_AXIOS_SERVICES.forEach(Service => register(Service, useFactory -> new Service(resolve(CHAIN_API_HTTP_CLIENT))))
Caller->>Container: resolve(LeaseHttpService)
Container->>CHAIN_API_HTTP_CLIENT: resolve token -> HttpClient instance
Container->>LeaseHttpService: construct LeaseHttpService(HttpClient)
Caller->>LeaseHttpService: list(params)
LeaseHttpService->>CHAIN_API_HTTP_CLIENT: GET /v1/lease?params
CHAIN_API_HTTP_CLIENT->>ChainAPI: HTTP GET
ChainAPI-->>CHAIN_API_HTTP_CLIENT: Response
CHAIN_API_HTTP_CLIENT-->>LeaseHttpService: Raw response
LeaseHttpService-->>Caller: extractData(response)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Files:
🧠 Learnings (1)📓 Common learnings🧬 Code graph analysis (1)packages/http-sdk/src/lease/lease-http.service.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1888 +/- ##
=======================================
Coverage 44.49% 44.49%
=======================================
Files 973 973
Lines 27349 27349
Branches 7073 7073
=======================================
+ Hits 12168 12169 +1
+ Misses 14078 14001 -77
- Partials 1103 1179 +76
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/api/src/core/providers/http-sdk.provider.ts (2)
30-32: Prefer a singleton HttpClient to avoid per-resolution instances.Without a lifecycle, each service resolution creates a fresh client, fragmenting interceptors/retry state and wasting sockets.
Apply:
-import { container } from "tsyringe"; +import { container, Lifecycle } from "tsyringe"; ... -container.register(CHAIN_API_HTTP_CLIENT, { - useFactory: () => createHttpClient({ baseURL: apiNodeUrl }) -}); +container.register(CHAIN_API_HTTP_CLIENT, { + useFactory: () => createHttpClient({ baseURL: apiNodeUrl }), + lifecycle: Lifecycle.Singleton +});
30-32: Minor typing cleanup for readability.Alias the constructor shape instead of repeating the newable type.
-const NON_AXIOS_SERVICES: Array<new (httpClient: HttpClient) => unknown> = [DeploymentHttpService, LeaseHttpService, CosmosHttpService]; +type HttpClientCtor<T = unknown> = new (httpClient: HttpClient) => T; +const NON_AXIOS_SERVICES: HttpClientCtor[] = [DeploymentHttpService, LeaseHttpService, CosmosHttpService];packages/http-sdk/src/lease/lease-http.service.ts (2)
60-62: Guard against undefined params across serializers.Axios typically drops undefined, but being explicit avoids surprises if serializers change.
- return extractData( - await this.httpClient.get<RestAkashLeaseListResponse>("/akash/market/v1beta4/leases/list", { - params: { - "filters.owner": owner, - "filters.dseq": dseq, - "filters.state": state - } - }) - ); + return extractData( + await this.httpClient.get<RestAkashLeaseListResponse>("/akash/market/v1beta4/leases/list", { + params: { + "filters.owner": owner, + ...(dseq ? { "filters.dseq": dseq } : {}), + ...(state ? { "filters.state": state } : {}) + } + }) + );
50-54: Consider exporting LeaseListParams (and re-exporting from the package index).Helps consumers import the type directly instead of relying on d.ts inference.
-type LeaseListParams = { +export type LeaseListParams = { owner: string; dseq?: string; state?: "active" | "insufficient_funds" | "closed"; };And ensure the barrel re-exports:
// packages/http-sdk/src/index.ts export * from "./lease/lease-http.service";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)packages/http-sdk/src/lease/lease-http.service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/api/src/core/providers/http-sdk.provider.tspackages/http-sdk/src/lease/lease-http.service.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/api/src/core/providers/http-sdk.provider.tspackages/http-sdk/src/lease/lease-http.service.ts
🧬 Code graph analysis (2)
apps/api/src/core/providers/http-sdk.provider.ts (4)
packages/http-sdk/src/utils/httpClient.ts (1)
HttpClient(23-23)packages/http-sdk/src/deployment/deployment-http.service.ts (1)
DeploymentHttpService(134-195)packages/http-sdk/src/lease/lease-http.service.ts (1)
LeaseHttpService(56-70)packages/http-sdk/src/cosmos/cosmos-http.service.ts (1)
CosmosHttpService(21-109)
packages/http-sdk/src/lease/lease-http.service.ts (1)
packages/http-sdk/src/index.ts (1)
HttpClient(26-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (3)
apps/api/src/core/providers/http-sdk.provider.ts (1)
27-27: Services’ constructors accept Axios config; registration is safe
All listed HTTP services—BalanceHttpService, AuthzHttpService, BlockHttpService, BidHttpService, ProviderHttpService—defineconstructor(config?: Pick<AxiosRequestConfig, …>)and callsuper(config), so theSERVICESarray registration remains valid at runtime.packages/http-sdk/src/lease/lease-http.service.ts (2)
1-2: LGTM: decoupled from HttpService and typed HttpClient import is correct.
56-57: LGTM: constructor injection matches provider changes.
stalniy
force-pushed
the
fix/lease-http
branch
2 times, most recently
from
a3eade2 to
50329ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/notifications/src/modules/alert/providers/http-sdk.provider.ts (2)
8-9: Good: dedicated, typed DI tokenSymbol-based token avoids collisions and preserves typing. If this token must be referenced across multiple packages, consider using Symbol.for to guard against accidental duplication when bundles split.
-export const CHAIN_API_HTTP_CLIENT_TOKEN: InjectionToken<HttpClient> = Symbol("CHAIN_API_HTTP_CLIENT"); +export const CHAIN_API_HTTP_CLIENT_TOKEN: InjectionToken<HttpClient> = Symbol.for("CHAIN_API_HTTP_CLIENT");
16-20: Optional DRY: generate service providers from a typed listKeeps provider list consistent as services grow without repeating boilerplate.
export const HTTP_SDK_PROVIDERS: Provider[] = [ { provide: CHAIN_API_HTTP_CLIENT_TOKEN, inject: [ConfigService], useFactory: (configService: ConfigService<AlertConfig>) => createHttpClient({ baseURL: configService.getOrThrow("alert.API_NODE_ENDPOINT") }) }, - { - provide: DeploymentHttpService, - inject: [CHAIN_API_HTTP_CLIENT_TOKEN], - useFactory: (httpClient: HttpClient) => new DeploymentHttpService(httpClient) - }, - { - provide: LeaseHttpService, - inject: [CHAIN_API_HTTP_CLIENT_TOKEN], - useFactory: (httpClient: HttpClient) => new LeaseHttpService(httpClient) - } + ...([DeploymentHttpService, LeaseHttpService] as const).map((Svc): Provider => ({ + provide: Svc, + inject: [CHAIN_API_HTTP_CLIENT_TOKEN], + useFactory: (httpClient: HttpClient) => new Svc(httpClient) + })) ];Also applies to: 23-24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/src/core/providers/http-sdk.provider.ts(1 hunks)apps/notifications/src/modules/alert/providers/http-sdk.provider.ts(1 hunks)packages/http-sdk/src/lease/lease-http.service.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/http-sdk/src/lease/lease-http.service.ts
- apps/api/src/core/providers/http-sdk.provider.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/notifications/src/modules/alert/providers/http-sdk.provider.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/notifications/src/modules/alert/providers/http-sdk.provider.ts
🧬 Code graph analysis (1)
apps/notifications/src/modules/alert/providers/http-sdk.provider.ts (4)
packages/http-sdk/src/utils/httpClient.ts (2)
HttpClient(23-23)createHttpClient(5-21)apps/notifications/src/modules/alert/config/index.ts (1)
AlertConfig(9-9)packages/http-sdk/src/deployment/deployment-http.service.ts (1)
DeploymentHttpService(134-195)packages/http-sdk/src/lease/lease-http.service.ts (1)
LeaseHttpService(56-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (1)
apps/notifications/src/modules/alert/providers/http-sdk.provider.ts (1)
1-3: Type-only and Nest DI imports look goodType-only import for HttpClient and explicit Nest types keep runtime clean and typings precise.
stalniy
force-pushed
the
fix/lease-http
branch
from
50329ff to
edc13d3
Compare
3 participants
Why
Default http client is retriable. It makes it easier to make this service a retry on network or server error. ref #1424
Summary by CodeRabbit
Refactor
Impact