refactor: move gpu endpoints to modules#1551
Conversation
|
""" WalkthroughThis change refactors the GPU API routes by removing legacy route files and implementing a new modular structure. It introduces dedicated GPU controller, service, repository, schema, and type modules, and rewires the main application and internal routing to use the new GPU API endpoints. Test and seeder code is also updated for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App
participant GpuRouter
participant GpuController
participant GpuService
participant GpuRepository
Client->>App: HTTP GET /v1/gpu (or /v1/gpu-models, /v1/gpu-breakdown, /v1/gpu-prices)
App->>GpuRouter: Route request
GpuRouter->>GpuController: Call appropriate method
GpuController->>GpuService: Fetch or process GPU data
GpuService->>GpuRepository: Query database for GPU info
GpuRepository-->>GpuService: Return GPU data
GpuService-->>GpuController: Aggregated/processed data
GpuController-->>GpuRouter: Response payload
GpuRouter-->>Client: Return JSON response
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (8)
apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts (1)
10-16: Usesetupfunction instead ofbeforeEachfor test setup.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. Thesetupfunction should be at the bottom of the rootdescribeblock, create and return the object under test, and avoid shared state.Replace the
beforeEachwith asetupfunction:- beforeEach(() => { - balancesService = { - getFreshLimits: jest.fn() - } as unknown as jest.Mocked<BalancesService>; - - service = new CachedBalanceService(balancesService); - }); + + const setup = (options: { mockLimits?: { deployment: number; fee: number } } = {}) => { + const balancesService = { + getFreshLimits: jest.fn() + } as unknown as jest.Mocked<BalancesService>; + + const service = new CachedBalanceService(balancesService); + + return { service, balancesService }; + };Then update each test to use the setup function:
it("should fetch and cache balance for new address", async () => { + const { service, balancesService } = setup(); + balancesService.getFreshLimits.mockResolvedValue({ + deployment: DEPLOYMENT_LIMIT, + fee: 100 + }); const balance = await service.get(address);apps/api/test/seeders/fees-authorization.seeder.ts (1)
20-34:_.mergeis called with a single argument and unsafe property access – overrides never apply + possible runtime crash
merge({...})without a second parameter means user-suppliedinputis ignored; callers cannot override defaults.input.allowance.spend_limit.*dereferencesallowanceandspend_limitunguarded. Supplying{}or omittingallowancewill throw at runtime.- static create(input: PartialDeep<FeeAllowanceSeederInput> = {}): FeeAllowance { - return merge({ - granter: input.granter || createAkashAddress(), - grantee: input.grantee || createAkashAddress(), - allowance: { - "@type": "/akash.deployment.v1beta3.DepositDeploymentAuthorization", - spend_limit: [ - { - denom: input.allowance.spend_limit.denom || DenomSeeder.create(), - amount: input.allowance.spend_limit.amount || faker.number.int({ min: 0, max: 10000000 }).toString() - } - ], - expiration: faker.date.future().toISOString() - } - }); + static create(input: PartialDeep<FeeAllowanceSeederInput> = {}): FeeAllowance { + return merge( + { + granter: input.granter ?? createAkashAddress(), + grantee: input.grantee ?? createAkashAddress(), + allowance: { + "@type": "/akash.deployment.v1beta3.DepositDeploymentAuthorization", + spend_limit: [ + { + denom: input.allowance?.spend_limit?.denom ?? DenomSeeder.create(), + amount: + input.allowance?.spend_limit?.amount ?? + faker.number.int({ min: 0, max: 10000000 }).toString() + } + ], + expiration: input.allowance?.expiration ?? faker.date.future().toISOString() + } + }, + input + ); }This restores override capability and guards against undefined paths.
apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts (1)
13-15: Use asetupfunction instead ofbeforeEachfor test setup.According to the coding guidelines, test files should use a
setupfunction instead ofbeforeEach. The setup function must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.Replace the
beforeEachwith asetupfunction:- beforeEach(() => { - summarizer = new TopUpSummarizer(); - });And add at the bottom of the root
describeblock:+ function setup(options: { /* any test-specific options */ } = {}) { + return new TopUpSummarizer(); + }Then update each test to call
setup()to get the summarizer instance.apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts (1)
29-60: Guideline violation – replacebeforeEachwith asetuphelperThe repo’s testing guideline for
*.spec.tsfiles mandates asetupfunction (returned from the bottom of the rootdescribe) instead ofbeforeEachto avoid hidden shared state.
Refactor the initialization currently insidebeforeEachinto such a helper, returning the mocked services & sut.- beforeEach(() => { - blockHttpService = { ... } - ... - service = new DrainingDeploymentService(...) - }); + const setup = () => { + const blockHttpService = { ... } as jest.Mocked<BlockHttpService>; + ... + const service = new DrainingDeploymentService(...); + return { blockHttpService, leaseRepository, userWalletRepository, deploymentSettingRepository, service, config }; + };Then call
const { service, ... } = setup();inside each test case.apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts (1)
47-79: Use asetupfunction instead ofbeforeEachper coding guidelines.The coding guidelines specify that test files should use a
setupfunction instead ofbeforeEachfor test setup. Thesetupfunction should be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.Consider refactoring to use a setup function:
- beforeEach(() => { + const setup = (overrides: { concurrency?: number; dryRun?: boolean } = {}) => { // ... existing setup code ... - service = new TopUpManagedDeploymentsService( + const service = new TopUpManagedDeploymentsService( managedSignerService, billingConfig as unknown as BillingConfigService, drainingDeploymentService, managedMasterWallet, rpcMessageService, cachedBalanceService, blockHttpService, chainErrorService ); - }); + return { service, /* other dependencies */ }; + };apps/api/test/functional/provider-versions.spec.ts (1)
10-70: Use asetupfunction instead ofbeforeAllper coding guidelines.The coding guidelines specify that test files should use a
setupfunction instead ofbeforeAllfor test setup. Consider refactoring the test setup to follow this pattern.apps/api/test/functional/network-capacity.spec.ts (1)
16-193: Use asetupfunction instead ofbeforeAllper coding guidelines.The coding guidelines specify that test files should use a
setupfunction instead ofbeforeAllfor test setup. Consider refactoring the extensive test setup to follow this pattern, which would also help with test maintainability.apps/api/test/functional/usage.spec.ts (1)
11-270: Refactor setup function to comply with coding guidelines.The setup function doesn't follow the coding guidelines for test files:
- It should be at the bottom of the root describe block
- It should accept a single parameter with inline type definition
- The function is very large (259 lines) and could benefit from being broken down
Consider refactoring like this:
describe("GET /v1/usage/history", () => { let isDbInitialized = false; // Move all test cases here... + function setup(options: { skipDbInit?: boolean } = {}) { // Existing setup logic... + } });Move the setup function to the bottom of the describe block and add a parameter with inline type definition as required by the guidelines.
🧹 Nitpick comments (16)
apps/api/test/seeders/fees-authorization.seeder.ts (1)
6-6: Import path consistencyOther seeders now import from
@test/seedersbarrel (…/seeders/index.ts). ImportingcreateAkashAddressdirectly from the file breaks that convention and complicates refactors. Prefer:-import { createAkashAddress } from "./akash-address.seeder"; +import { createAkashAddress } from "@test/seeders";apps/api/src/app.ts (1)
54-55: Keep import list alphabetised for readabilityThe new
gpuRouterimport breaks the alphabetical order of router imports. Moving it aftergraphDataRouterkeeps the section easy to scan.apps/api/src/billing/services/usage/usage.service.spec.ts (1)
192-200: Minor: consider lifting test-data generation out ofsetupfor readabilityThe inline construction of three sizeable
BillingUsageSeeder.createobjects insidesetupmakes the helper lengthy and harder to scan.
Moving this block to a dedicatedcreateMockUsageData(startDate)utility (or at least extracting the literal array) will improve readability and reuse across tests.apps/api/test/seeders/deployment-group-resource.seeder.ts (1)
5-18: Consider more appropriate faker methods for GPU-related fields.The seeder function structure is correct, but some faker methods could be more representative of actual GPU data:
- gpuVendor: overrides.gpuVendor || faker.company.name(), - gpuModel: overrides.gpuModel || faker.animal.horse(), + gpuVendor: overrides.gpuVendor || faker.helpers.arrayElement(['nvidia', 'amd', 'intel']), + gpuModel: overrides.gpuModel || faker.helpers.arrayElement(['rtx4090', 'rtx4080', 'rx7900xt', 'rx6900xt']),This would generate more realistic GPU vendor and model data for testing purposes.
apps/api/test/functional/usage.spec.ts (1)
406-427: Setup function should accept a parameter with inline type definition.According to the coding guidelines, the setup function should accept a single parameter with inline type definition, even if it's optional.
- function setup() { + function setup(options: {} = {}) {apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts (1)
5-5: Consider using consistent naming convention for GPU acronym.The function name uses
createProviderSnapshotNodeGpuwith camelCase 'Gpu', but the model name usesProviderSnapshotNodeGPUwith uppercase. Consider renaming tocreateProviderSnapshotNodeGPUfor consistency.-export const createProviderSnapshotNodeGpu = async (overrides: Partial<CreationAttributes<ProviderSnapshotNodeGPU>> = {}): Promise<ProviderSnapshotNodeGPU> => { +export const createProviderSnapshotNodeGPU = async (overrides: Partial<CreationAttributes<ProviderSnapshotNodeGPU>> = {}): Promise<ProviderSnapshotNodeGPU> => {apps/api/test/seeders/deployment-group.seeder.ts (1)
7-11: Use consistent null coalescing operator throughout.The code mixes
||and??operators inconsistently. For seeder overrides,??is preferred as it only checks for null/undefined, allowing explicit falsy values likefalse,0, or empty strings to be set through overrides.- id: overrides.id || faker.string.uuid(), - deploymentId: overrides.deploymentId || faker.string.uuid(), - owner: overrides.owner || faker.string.alphanumeric(44), - dseq: overrides.dseq || faker.string.numeric(20), + id: overrides.id ?? faker.string.uuid(), + deploymentId: overrides.deploymentId ?? faker.string.uuid(), + owner: overrides.owner ?? faker.string.alphanumeric(44), + dseq: overrides.dseq ?? faker.string.numeric(20),apps/api/test/seeders/provider-snapshot.seeder.ts (1)
7-11: Use consistent null coalescing operator throughout.Similar to other seeders, this file mixes
||and??operators. For consistency and to allow explicit falsy overrides, use??throughout.- owner: overrides.owner || faker.finance.ethereumAddress(), + owner: overrides.owner ?? faker.finance.ethereumAddress(), isLastOfDay: overrides.isLastOfDay ?? false, isLastSuccessOfDay: overrides.isLastSuccessOfDay ?? false, isOnline: overrides.isOnline ?? true, - checkDate: overrides.checkDate || new Date(), + checkDate: overrides.checkDate ?? new Date(),apps/api/src/gpu/types/gpu.type.ts (1)
3-51: Consider adding JSDoc documentation for better developer experience.Adding JSDoc comments to each type would improve developer experience and code maintainability, especially for complex types like
GpuBidType.+/** + * Represents a GPU provider with allocation information + */ export type GpuProviderType = { + /** Provider's wallet address */ owner: string; + /** Provider's host URI */ hostUri: string; + /** Currently allocated GPU units */ allocated: number; + /** Total allocatable GPU units */ allocatable: number; }; +/** + * Represents a GPU with its specifications and provider information + */ export type GpuType = { vendor: string; model: string; // ... continue for other typesapps/api/src/gpu/routes/gpu.router.ts (3)
80-80: Fix misleading variable name.The variable name
blocksis inappropriate for GPU data.- const blocks = await container.resolve(GpuController).getGpuList({ providerAddress, providerHostUri, vendor, model, memorySize: memory_size }); + const gpuList = await container.resolve(GpuController).getGpuList({ providerAddress, providerHostUri, vendor, model, memorySize: memory_size }); - return c.json(blocks); + return c.json(gpuList);
95-95: Fix typo in API description.- description: "List of gpu models per.", + description: "List of gpu models per vendor.",
74-74: VerifyURL.canParse()compatibility.
URL.canParse()is a newer JavaScript API that may not be available in all Node.js versions. Consider using a try-catch with URL constructor for better compatibility.- } else if (URL.canParse(provider)) { + } else { + try { + new URL(provider); + providerHostUri = provider; + } catch { + return c.json({ error: "Invalid provider parameter, should be a valid akash address or host uri" }, 400); + } - providerHostUri = provider; - } else { - return c.json({ error: "Invalid provider parameter, should be a valid akash address or host uri" }, 400); }Verify the Node.js version requirements:
What Node.js version introduced URL.canParse() and what is the current LTS version?apps/api/src/gpu/services/gpu.service.ts (1)
78-80: Consider making the GPU models URL configurable.The hardcoded GitHub URL could become a maintenance issue if the source location changes. Consider moving this to environment configuration.
Add to your environment configuration:
GPU_MODELS_URL: process.env.GPU_MODELS_URL || "https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"Then update the code:
-const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"); +const response = await axios.get<ProviderConfigGpusType>(env.GPU_MODELS_URL);apps/api/src/gpu/services/gpu-price.service.ts (2)
49-88: Consider refactoring complex bid processing for better maintainability.The bid processing logic is complex with multiple nested operations. Consider extracting helper methods for better readability and testability.
Example refactor approach:
private decodeBidMessage(message: any, days: any[]): GpuBidType | null { const day = days.find(day => day.id === message.block.dayId); const decodedBid = decodeMsg("/akash.market.v1beta4.MsgCreateBid", message.data) as MsgCreateBid; if (!day || !day.aktPrice || decodedBid?.price?.denom !== "uakt") { return null; } return this.createGpuBid(message, decodedBid, day); } private extractGpuResources(resourcesOffer: any[]): any { return { cpuUnits: this.sumResourceUnits(resourcesOffer, r => r.resources?.cpu?.units?.val), memoryUnits: this.sumResourceUnits(resourcesOffer, r => r.resources?.memory?.quantity?.val), storageUnits: this.sumResourceUnits(resourcesOffer, r => r.resources?.storage?.quantity?.val), gpus: this.extractGpus(resourcesOffer) }; }
187-189: Improve error handling and logging.Replace console.error with proper logging and consider propagating errors for better observability.
Use a proper logger:
-console.error("Error calculating pricing", e); +this.logger.error("Error calculating pricing", { error: e, providersCount: providersWithBestBid.length });Consider also whether swallowing the error is appropriate, or if it should be propagated to allow proper error tracking.
apps/api/src/gpu/repositories/gpu.repository.ts (1)
170-170: Consider removing redundant ORDER BY in CTE.The
ORDER BYclause in the CTE might be redundant sinceDISTINCT ONalready uses the first column for ordering. However, if you need a specific order for tie-breaking when multiple rows have the samehostUri, keep it as is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
apps/api/src/app.ts(2 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts(4 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts(2 hunks)apps/api/src/block/repositories/akash-block/akash-block.repository.ts(2 hunks)apps/api/src/deployment/controllers/deployment/deployment.controller.ts(1 hunks)apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts(3 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts(2 hunks)apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts(2 hunks)apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts(2 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(4 hunks)apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts(3 hunks)apps/api/src/gpu/controllers/gpu.controller.ts(1 hunks)apps/api/src/gpu/http-schemas/gpu.schema.ts(1 hunks)apps/api/src/gpu/index.ts(1 hunks)apps/api/src/gpu/repositories/day.repository.ts(1 hunks)apps/api/src/gpu/repositories/gpu.repository.ts(1 hunks)apps/api/src/gpu/routes/gpu.router.ts(1 hunks)apps/api/src/gpu/routes/index.ts(1 hunks)apps/api/src/gpu/services/gpu-price.service.ts(1 hunks)apps/api/src/gpu/services/gpu.service.ts(1 hunks)apps/api/src/gpu/types/gpu.type.ts(1 hunks)apps/api/src/notifications/routes/proxy/proxy.route.spec.ts(2 hunks)apps/api/src/provider/repositories/provider/provider.repository.spec.ts(7 hunks)apps/api/src/routes/internal/index.ts(1 hunks)apps/api/src/routes/v1/gpu.ts(0 hunks)apps/api/src/routes/v1/gpuBreakdown.ts(0 hunks)apps/api/src/routes/v1/gpuModels.ts(0 hunks)apps/api/src/routes/v1/gpuPrices.ts(0 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/services/db/gpuBreakdownService.ts(0 hunks)apps/api/test/functional/blocks.spec.ts(1 hunks)apps/api/test/functional/dashboard-data.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(7 hunks)apps/api/test/functional/leases-duration.spec.ts(6 hunks)apps/api/test/functional/network-capacity.spec.ts(8 hunks)apps/api/test/functional/provider-dashboard.spec.ts(2 hunks)apps/api/test/functional/provider-deployments.spec.ts(6 hunks)apps/api/test/functional/provider-graph-data.spec.ts(10 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(8 hunks)apps/api/test/functional/transactions.spec.ts(1 hunks)apps/api/test/functional/usage.spec.ts(11 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/seeders/akash-address.seeder.ts(1 hunks)apps/api/test/seeders/akash-block.seeder.ts(1 hunks)apps/api/test/seeders/akash-message.seeder.ts(1 hunks)apps/api/test/seeders/auto-top-up-deployment.seeder.ts(1 hunks)apps/api/test/seeders/block.seeder.ts(0 hunks)apps/api/test/seeders/chain-wallet.seeder.ts(1 hunks)apps/api/test/seeders/day.seeder.ts(1 hunks)apps/api/test/seeders/deployment-grant.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group-resource.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group.seeder.ts(1 hunks)apps/api/test/seeders/deployment-info.seeder.ts(2 hunks)apps/api/test/seeders/deployment.seeder.ts(1 hunks)apps/api/test/seeders/fees-authorization.seeder.ts(2 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/seeders/lease-api-response.seeder.ts(2 hunks)apps/api/test/seeders/lease.seeder.ts(1 hunks)apps/api/test/seeders/message.seeder.ts(0 hunks)apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot-node.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot.seeder.ts(1 hunks)apps/api/test/seeders/provider.seeder.ts(1 hunks)apps/api/test/seeders/transaction.seeder.ts(1 hunks)apps/api/test/seeders/user-wallet.seeder.ts(1 hunks)apps/api/test/seeders/validator.seeder.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (7)
- apps/api/src/routes/v1/gpuModels.ts
- apps/api/src/routes/v1/gpuBreakdown.ts
- apps/api/test/seeders/block.seeder.ts
- apps/api/test/seeders/message.seeder.ts
- apps/api/src/services/db/gpuBreakdownService.ts
- apps/api/src/routes/v1/gpu.ts
- apps/api/src/routes/v1/gpuPrices.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use a `setup` function instead of `beforeEach` for test setup. The `setup` function must be at the bottom of the root `describe` block, create and return the ...
**/*.spec.{ts,tsx}: Use asetupfunction instead ofbeforeEachfor test setup. Thesetupfunction must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.
apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.tsapps/api/src/billing/services/usage/usage.service.spec.tsapps/api/test/functional/provider-regions.spec.tsapps/api/test/functional/validators.spec.tsapps/api/src/notifications/routes/proxy/proxy.route.spec.tsapps/api/test/functional/provider-versions.spec.tsapps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.tsapps/api/test/functional/network-capacity.spec.tsapps/api/test/functional/dashboard-data.spec.tsapps/api/test/functional/transactions.spec.tsapps/api/test/functional/provider-dashboard.spec.tsapps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.tsapps/api/src/billing/repositories/usage/usage.repository.spec.tsapps/api/test/functional/providers.spec.tsapps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.tsapps/api/test/functional/provider-deployments.spec.tsapps/api/test/functional/provider-graph-data.spec.tsapps/api/test/functional/leases-duration.spec.tsapps/api/test/functional/graph-data.spec.tsapps/api/src/provider/repositories/provider/provider.repository.spec.tsapps/api/test/functional/blocks.spec.tsapps/api/test/functional/usage.spec.tsapps/api/test/functional/gpu.spec.ts
🧠 Learnings (1)
apps/api/src/deployment/controllers/deployment/deployment.controller.ts (1)
Learnt from: baktun14
PR: akash-network/console#1428
File: apps/api/src/deployment/controllers/deployment/deployment.controller.ts:0-0
Timestamp: 2025-06-03T15:06:34.211Z
Learning: The `getByOwnerAndDseq` method in `apps/api/src/deployment/controllers/deployment/deployment.controller.ts` is intentionally public without the `@Protected` decorator because it serves public blockchain data from an indexer, following the pattern of public blockchain APIs.
🪛 Biome (1.9.4)
apps/api/src/gpu/services/gpu-price.service.ts
[error] 80-80: Comparing to itself is potentially pointless.
(lint/suspicious/noSelfCompare)
🔇 Additional comments (88)
apps/api/test/seeders/lease-api-response.seeder.ts (2)
3-3: LGTM: Import updated to use new function-based seeder.The import change from class-based
AkashAddressSeederto function-basedcreateAkashAddressaligns with the seeder refactoring pattern across the codebase.
64-64: LGTM: Function calls updated consistently.The usage has been correctly updated from
AkashAddressSeeder.create()tocreateAkashAddress()for bothownerandproviderdefault values, maintaining the same functionality while using the new function-based approach.Also applies to: 68-68
apps/api/src/gpu/index.ts (1)
1-1: LGTM: Clean barrel export pattern.The barrel export pattern is correctly implemented, providing a clean entry point for the GPU module. This follows TypeScript best practices for module organization.
apps/api/tsconfig.strict.json (1)
35-35: LGTM: GPU module included in strict type checking.The addition of
"src/gpu/**/*.ts"correctly enables strict TypeScript checking for the new GPU module, ensuring type safety and consistency with the rest of the codebase.apps/api/test/seeders/chain-wallet.seeder.ts (2)
3-3: LGTM: Import updated to use new function-based seeder.The import change from class-based
AkashAddressSeederto function-basedcreateAkashAddressis consistent with the seeder refactoring pattern.
6-6: LGTM: Function call updated consistently.The usage has been correctly updated from
AkashAddressSeeder.create()tocreateAkashAddress(), maintaining the same functionality while using the new function-based approach.apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts (2)
4-4: LGTM: Import updated to use new function-based seeder.The import change from class-based
AkashAddressSeederto function-basedcreateAkashAddressis consistent with the seeder refactoring pattern.
19-19: LGTM: Function call updated consistently.The usage has been correctly updated from
AkashAddressSeeder.create()tocreateAkashAddress(), maintaining the same functionality.apps/api/src/app.ts (1)
144-145: Route registration order – place GPU routes with other feature routers
appHono.route("/", gpuRouter);is appended after pricing but before leases-duration/network routes. Consider grouping GPU routes near related market-data/pricing routes (or alphabetically) to avoid future merge conflicts and ease maintainability.apps/api/src/gpu/routes/index.ts (1)
1-1: Simple & clear re-exportGood to expose the router via the index – keeps external imports clean.
apps/api/test/seeders/deployment-grant.seeder.ts (1)
12-16: Looks good – consistent with new seeder utilities
createAkashAddress()usage is correct, and overrides can still replace defaults via the secondmergeargument.apps/api/test/seeders/auto-top-up-deployment.seeder.ts (1)
4-13: No issues spottedThe switch to
createAkashAddress()is straightforward and safe.apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts (3)
6-6: LGTM: Seeder refactor is consistent.The change from class-based to function-based seeder aligns with the broader refactor pattern.
37-38: LGTM: Function-based seeder usage is correct.The update from
AkashAddressSeeder.create()tocreateAkashAddress()is consistent with the refactor.
125-126: LGTM: Function-based seeder usage is correct.The update from
AkashAddressSeeder.create()tocreateAkashAddress()is consistent with the refactor.apps/api/test/seeders/deployment-info.seeder.ts (2)
5-5: LGTM: Import refactor is consistent.The change from class-based
AkashAddressSeederto function-basedcreateAkashAddressaligns with the broader seeder refactor.
27-27: LGTM: Function-based seeder usage is correct.The update from
AkashAddressSeeder.create()tocreateAkashAddress()maintains the same functionality with a simplified API.apps/api/test/functional/provider-regions.spec.ts (2)
7-7: LGTM: Import refactor is consistent.The change to import
createProviderfrom the consolidated seeders module aligns with the broader refactor pattern.
15-15: LGTM: Function-based seeder usage is correct.The update from
ProviderSeeder.createInDatabase()tocreateProvider()simplifies the API while maintaining the same functionality.apps/api/test/functional/validators.spec.ts (2)
7-7: LGTM: Import refactor is consistent.The change to import
createValidatorfrom the consolidated seeders module aligns with the broader refactor pattern.
14-23: LGTM: Function-based seeder usage is correct.The updates from
ValidatorSeeder.createInDatabase()tocreateValidator()simplify the API while maintaining the same functionality for creating test validators.apps/api/test/seeders/user-wallet.seeder.ts (2)
4-4: LGTM: Import refactor is consistent.The change from class-based
AkashAddressSeederto function-basedcreateAkashAddressaligns with the broader seeder refactor.
10-10: LGTM: Function-based seeder usage is correct.The update from
AkashAddressSeeder.create()tocreateAkashAddress()maintains the same functionality with a simplified API.apps/api/src/billing/services/usage/usage.service.spec.ts (1)
9-11: VerifycreateAkashAddress()is synchronousAll call-sites in this test treat
createAkashAddress()as a synchronous function that returns a string immediately.
If the new factory was implemented asasync(returning aPromise<string>), every usage here will silently pass the promise through to the service layer and expectations, likely causing subtle mismatches.Please confirm the helper’s return type and, if it is asynchronous, wrap the call in
await(and marksetupasasync) to avoid brittle tests.apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts (1)
13-14: Import change LGTMSwitching to
createAkashAddresskeeps the file in sync with the new consolidated seeders module.apps/api/test/functional/provider-dashboard.spec.ts (2)
16-32: Possible type mismatch when assertingdatetime.toISOString()
createAkashBlockis called with adatetimestring; unless the factory converts it back toDate, the later assertion
blocks[0].datetime.toISOString()(line 42) will throw (toISOStringis undefined on string).Please verify the factory’s behaviour or adjust the test:
- expect(data.current.date).toEqual(blocks[0].datetime.toISOString()); + expect(data.current.date).toEqual( + (blocks[0].datetime instanceof Date ? blocks[0].datetime : new Date(blocks[0].datetime)).toISOString() + );
7-10: Import consolidation looks goodThe switch to
createProviderandcreateAkashBlockaligns with the new seeder utilities.apps/api/src/notifications/routes/proxy/proxy.route.spec.ts (1)
9-10: Import update acknowledged
createAkashAddressintegrates the test with the refactored seeder helpers; no additional changes needed.apps/api/src/routes/v1/index.ts (1)
22-23: Removal of GPU routes confirmedThe V1 route list no longer exports the legacy GPU endpoints, consistent with the new dedicated GPU router introduced elsewhere.
apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts (2)
17-17: LGTM! Seeder refactoring improves consistency.The change from
AkashAddressSeeder.create()tocreateAkashAddress()aligns with the broader refactor to function-based seeders, improving code maintainability.
43-43: LGTM! Consistent seeder usage.The usage of
createAkashAddress()is consistent with the refactoring pattern and maintains the same functionality.Also applies to: 249-249
apps/api/test/functional/provider-versions.spec.ts (2)
5-5: LGTM! Consolidated seeder imports improve maintainability.The change to import consolidated seeder functions from a unified module is a positive refactoring that improves code organization.
14-34: LGTM! Consistent seeder function usage.The replacement of class-based seeders (
ProviderSeeder.create(),ProviderSnapshotSeeder.create()) with function-based alternatives (createProvider(),createProviderSnapshot()) is consistent with the refactoring pattern and maintains the same functionality.Also applies to: 37-57
apps/api/test/functional/transactions.spec.ts (2)
1-1: LGTM! Improved imports and seeder consolidation.The change to type-only import for
Transactionand the consolidated seeder imports improve code organization and maintainability.Also applies to: 6-6
12-21: LGTM! Improved seeder usage and concise array generation.The refactoring from
BlockSeeder.create()tocreateAkashBlock()and the use ofPromise.allwithArray.fromfor generating transactions is more concise and maintainable than the previous approach.apps/api/src/gpu/repositories/day.repository.ts (1)
1-10: LGTM! Well-structured repository implementation.The
DayRepositoryimplementation follows good practices:
- Uses singleton pattern for dependency injection
- Has a clear, focused responsibility
- Properly implements Sequelize query with
Op.gteoperator- Clean and readable code structure
The repository fits well into the GPU data layer architecture.
apps/api/test/functional/network-capacity.spec.ts (2)
7-7: LGTM! Consolidated seeder imports improve maintainability.The change to import consolidated seeder functions from a unified module improves code organization and consistency.
20-38: LGTM! Consistent seeder function usage throughout the file.The replacement of all class-based seeders (
DaySeeder.create(),ProviderSeeder.create(),ProviderSnapshotSeeder.create(),BlockSeeder.create()) with their function-based alternatives is consistent with the refactoring pattern and maintains the same functionality while improving code maintainability.Also applies to: 40-192
apps/api/test/functional/dashboard-data.spec.ts (1)
7-7: LGTM! Clean refactor from class-based to function-based seeders.The migration from class-based seeders (
BlockSeeder,DaySeeder, etc.) to function-based factory functions (createAkashBlock,createDay, etc.) is well-executed and maintains the same test data structure and functionality.Also applies to: 22-40, 42-42, 44-169, 181-194
apps/api/src/block/repositories/akash-block/akash-block.repository.ts (2)
3-3: LGTM! Correct import addition for the new method.The Op import is needed for the
Op.gteoperator used in the newgetFirstBlockAftermethod.
86-88: LGTM! Well-implemented method for date-based block queries.The method correctly uses
Op.gteto find blocks on or after the specified date and orders bydatetimeascending to return the earliest match.apps/api/test/functional/leases-duration.spec.ts (1)
7-7: LGTM! Consistent refactor to function-based seeders.The migration follows the same pattern as other test files, replacing class-based seeders with factory functions while preserving all test data and functionality.
Also applies to: 25-25, 28-44, 47-71, 74-86, 91-127
apps/api/test/seeders/akash-block.seeder.ts (1)
7-29: LGTM! Well-structured factory function with comprehensive defaults.The factory function provides good default values using faker and allows flexible overrides through the input parameter. The structure is consistent with the broader seeder refactoring effort.
apps/api/test/seeders/akash-address.seeder.ts (1)
4-12: LGTM! Correct implementation of function-based address generation.The refactor from class-based to function-based approach is well-executed. The logic correctly generates a 20-byte random address and converts it to bech32 format with the "akash" prefix.
apps/api/src/billing/repositories/usage/usage.repository.spec.ts (3)
11-11: LGTM! Clean seeder import consolidation.The consolidation of multiple seeder imports into a single unified import improves maintainability and follows the established factory function pattern.
267-312: Well-executed seeder refactoring.The consistent update of all test helper functions to use the new factory functions (e.g.,
createDay,createAkashBlock,createProvider, etc.) maintains the same test logic while improving the underlying data creation pattern.
315-321: Setup function follows coding guidelines correctly.The setup function is properly positioned at the bottom of the describe block, creates and returns the object under test, and doesn't specify a return type, adhering to the coding guidelines.
apps/api/test/functional/providers.spec.ts (3)
12-12: LGTM! Consistent seeder consolidation.The unified import of factory functions from
@test/seedersaligns with the broader seeder refactoring across the codebase.
21-59: Factory function migration executed well.All seeder calls have been consistently updated to use the new factory functions (
createProvider,createProviderSnapshot) while maintaining the same test data structure and logic.
146-198: Comprehensive seeder updates in test cases.The migration from seeder classes to factory functions is thorough and maintains test integrity while improving data creation patterns.
apps/api/test/functional/provider-graph-data.spec.ts (3)
7-7: Clean import consolidation.The unified seeder import simplifies the dependency management and follows the established factory function pattern.
22-170: Excellent seeder migration consistency.All factory function calls have been systematically updated while preserving the comprehensive test data setup. The migration maintains the same parameter structure and test logic.
246-278: Additional test cases properly updated.The seeder updates extend consistently to all test scenarios, including edge cases like the "first 15 minutes of the day" test.
apps/api/src/deployment/controllers/deployment/deployment.controller.ts (1)
127-129: Appropriate method signature enhancement.Adding
| nullto the return type properly handles cases where no deployment is found, improving the API's type safety and error handling capabilities. This aligns with the broader deployment querying enhancements mentioned in the summary.apps/api/test/seeders/akash-message.seeder.ts (1)
5-24: Well-implemented seeder factory function.The
createAkashMessagefunction follows the established pattern with:
- Proper use of faker for realistic test data generation
- Support for partial overrides via the input parameter
- Comprehensive default values for all model attributes
- Consistent return type and async/await pattern
The hex-encoded buffer data provides realistic message content for testing scenarios.
apps/api/src/provider/repositories/provider/provider.repository.spec.ts (4)
5-5: LGTM! Import consolidation improves consistency.The refactoring from class-based seeders (
AkashAddressSeeder.create(),ProviderSeeder.create()) to function-based seeders (createAkashAddress(),createProviderSeed()) streamlines the test setup and improves maintainability.
153-153: Consistent seeder function usage throughout the test.All instances of
createAkashAddress()calls are properly implemented, replacing the previous class-based seeder pattern consistently across the test cases.Also applies to: 159-159, 162-162, 173-173, 197-197, 213-213, 225-225
283-288: Correct implementation of provider seeder function.The
createProviderSeed()function call properly replaces the previousProviderSeeder.create()pattern, maintaining the same functionality while using the new function-based approach.
266-271: Test setup function follows coding guidelines correctly.The
setupfunction is properly positioned at the bottom of the rootdescribeblock and correctly returns the object under test without specifying a return type, adhering to the coding guidelines.apps/api/test/functional/provider-deployments.spec.ts (2)
9-9: LGTM! Consolidated seeder imports improve maintainability.The import consolidation from multiple separate seeder classes to unified factory functions from
@test/seedersaligns well with the broader refactoring effort and simplifies the import structure.
18-23: Consistent seeder function usage across all test data creation.All seeder function calls have been properly updated from the class-based pattern (e.g.,
BlockSeeder.createInDatabase) to the new function-based pattern (e.g.,createAkashBlock). The function signatures and parameter passing remain consistent with the original implementation.Also applies to: 25-25, 28-49, 52-61, 64-91, 94-110
apps/api/src/routes/internal/index.ts (2)
2-2: LGTM! GPU router import consolidation improves modularity.The consolidation of GPU-related router imports from individual relative paths to a unified
@src/gpuimport aligns with the broader GPU API refactoring and improves code organization.
5-5: Verify GPU router export order and completeness.The export array includes all necessary GPU routers, but ensure that the order doesn't affect functionality and that all routers are properly initialized.
#!/bin/bash # Description: Verify that all GPU routers are properly exported from @src/gpu # Expected: All three GPU routers should be found in the GPU module exports # Check GPU module exports rg -A 10 "export.*Router" apps/api/src/gpu/apps/api/test/seeders/deployment-group-resource.seeder.ts (1)
1-18: LGTM! Seeder function follows consistent pattern.The function structure, async/await usage, parameter handling, and return type are all consistent with the broader seeder refactoring pattern. The use of CreationAttributes type and default value generation with faker is appropriate.
apps/api/test/functional/blocks.spec.ts (2)
1-1: LGTM! Import optimizations align with seeder refactoring.The change to type-only import for
AkashBlockand consolidated seeder function imports improve the module structure and follow the refactoring pattern consistently.Also applies to: 6-6
12-21: Efficient concurrent block creation with proper seeder usage.The refactored setup creates the validator and 101 blocks concurrently using the new seeder functions. The
Array.frompattern withPromise.allis an efficient approach for bulk test data creation.apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts (3)
14-14: LGTM! Clean dependency injection implementation.The refactor correctly introduces dependency injection for the GpuService, making the code more modular and testable.
Also applies to: 26-28
50-50: Approve service method usage.The change from direct function call to service method call is correct and consistent with the dependency injection pattern.
81-81: Type annotation correctly reflects service return type.The type annotation properly uses the return type of the injected service method, ensuring type safety.
apps/api/test/seeders/deployment.seeder.ts (1)
5-18: LGTM! Consistent seeder refactor pattern.The refactor from class-based to function-based seeder is well-implemented:
- Combines data generation and database insertion efficiently
- Uses appropriate faker methods for each field type
- Maintains flexibility with overrides parameter
- Follows the same pattern as other seeders in the codebase
apps/api/test/functional/graph-data.spec.ts (2)
8-8: Approve centralized seeder imports.The change to import all seeder functions from a centralized module improves maintainability and consistency.
23-41: LGTM! Consistent seeder function usage.All seeder function calls have been updated consistently:
- Maintain the same parameters and data structures
- Follow the new function-based pattern
- Preserve existing test data and logic
The refactor improves code consistency across the test suite.
Also applies to: 43-108, 111-144, 153-166
apps/api/test/seeders/transaction.seeder.ts (2)
1-1: Approve import change for database operations.Correctly changed from type-only import to value import to enable calling
Transaction.create().
5-21: LGTM! Well-structured seeder function.The refactor is excellent:
- Follows consistent pattern with other seeders
- Uses appropriate faker methods for each field type (hexadecimal for hash, numeric for IDs, etc.)
- Maintains flexibility with input overrides
- Directly creates and returns the database record
apps/api/test/seeders/index.ts (1)
1-14: LGTM! Excellent centralization of seeder utilities.This barrel export pattern provides several benefits:
- Simplifies imports across test files
- Creates a single entry point for all seeder functions
- Improves maintainability by centralizing seeder exports
- Follows standard Node.js module patterns
The exports cover all the necessary seeder utilities for comprehensive test data creation.
apps/api/test/seeders/day.seeder.ts (1)
5-15: LGTM! Clean refactor from class-based to function-based seeder.The implementation correctly consolidates the previous two-step seeding process into a single function that directly creates and persists the Day entity. The faker usage is appropriate for all fields, and the typing is correct.
apps/api/test/seeders/lease.seeder.ts (1)
5-27: LGTM! Consistent refactor with appropriate data generation.The function properly handles all Lease fields with appropriate faker generators. The monetary values use correct precision (multipleOf: 0.000001), and the field ranges are reasonable for test data.
apps/api/test/seeders/validator.seeder.ts (1)
5-21: LGTM! Proper seeder refactor with correct dependencies.The function correctly imports and uses
createAkashAddressfor address fields, and all other fields use appropriate faker generators. The refactor maintains the same data generation logic while simplifying the interface.apps/api/test/functional/usage.spec.ts (1)
6-6: LGTM! Clean consolidation of seeder imports.The import statement properly consolidates all the factory functions from the new function-based seeders, making the code cleaner and more maintainable.
apps/api/src/deployment/repositories/deployment/deployment.repository.ts (2)
1-3: LGTM! Proper import additions for GPU functionality.The new imports are correctly added to support the GPU-related query functionality, including all necessary models for the complex join operations.
77-110: ```shell
#!/bin/bashSearch for association decorators in Deployment model
rg -n "@hasmany" -C2 packages/database/dbSchemas/akash/deployment.ts || echo "No @hasmany found"
rg -n "@BelongsTo" -C2 packages/database/dbSchemas/akash/deployment.ts || echo "No @BelongsTo found"Show lines 75-120 to inspect association definitions
echo
echo "---- deployment.ts lines 75-120 ----"
sed -n '75,120p' packages/database/dbSchemas/akash/deployment.ts</details> <details> <summary>apps/api/src/gpu/types/gpu.type.ts (1)</summary> `1-52`: **Comprehensive GPU type definitions look well-structured.** The type definitions effectively model the GPU domain with clear separation of concerns. The types cover providers, GPUs, bids, and pricing in a logical hierarchy. </details> <details> <summary>apps/api/test/seeders/provider-snapshot-node.seeder.ts (1)</summary> `5-21`: **Well-implemented seeder following consistent patterns.** The implementation correctly uses the `??` operator throughout and follows the established refactoring pattern. The faker data generation is appropriate for each field type. </details> <details> <summary>apps/api/src/gpu/controllers/gpu.controller.ts (1)</summary> `1-40`: **LGTM!** Clean controller implementation with proper dependency injection and clear delegation to services. </details> <details> <summary>apps/api/test/seeders/provider.seeder.ts (1)</summary> `5-41`: **LGTM!** Good refactoring from class-based to function-based seeders, consistent with the pattern across other seeder files. </details> <details> <summary>apps/api/src/gpu/http-schemas/gpu.schema.ts (1)</summary> `56-57`: I’d like to check how CPU utilization is typed in our HTTP schemas to keep GPU consistent. Let’s search for `cpuUtilization` in our schema files: ```shell #!/bin/bash # List schema files rg --files | grep "http-schemas" # Show context around cpuUtilization in TypeScript files rg -A3 -B3 "cpuUtilization" --type tsapps/api/src/gpu/services/gpu.service.ts (1)
14-76: LGTM! Well-structured GPU list aggregation.The method correctly handles optional filtering parameters, aggregates GPU data by vendor, and returns a well-organized response with proper sorting.
apps/api/src/gpu/repositories/gpu.repository.ts (1)
83-105: Excellent defensive programming in utilization calculations.The query correctly handles edge cases with:
NULLIFto prevent division by zeroLEASTto cap allocated values and utilization percentage- Proper null handling with
COALESCE
jzsfkzm
force-pushed
the
refactor/1279-gpu
branch
from
119fac3 to
8506d2a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/api/src/gpu/services/gpu-price.service.ts (4)
35-35: Extract magic number to named constant.The hardcoded value
31for days to include should be extracted to a named constant for better maintainability and documentation.+private static readonly DEFAULT_PRICING_PERIOD_DAYS = 31; + async getGpuPrices(debug: boolean) { // Get list of GPUs (model,vendor, ram, interface) and their availability const gpus = await this.gpuRepository.getGpusForPricing(); - const daysToInclude = 31; + const daysToInclude = GpuPriceService.DEFAULT_PRICING_PERIOD_DAYS;
127-127: Extract pricing period constant.Similar to the 31-day period, the 14-day period for recent bids should also be a named constant.
+private static readonly RECENT_BIDS_PERIOD_DAYS = 14; + const providerBidsLast14d = providerBids.filter(x => x.datetime > addDays(new Date(), -14)); +const providerBidsLast14d = providerBids.filter(x => x.datetime > addDays(new Date(), -GpuPriceService.RECENT_BIDS_PERIOD_DAYS));
131-131: Extract pricing bot CPU units to constant.The hardcoded CPU units value
100for pricing bot identification should be extracted to a named constant.+private static readonly PRICING_BOT_CPU_UNITS = 100; + -const bidsFromPricingBot = providerBids.filter(x => x.deployment.owner === pricingBotAddress && x.deployment.cpuUnits === 100); +const bidsFromPricingBot = providerBids.filter(x => x.deployment.owner === pricingBotAddress && x.deployment.cpuUnits === GpuPriceService.PRICING_BOT_CPU_UNITS);
188-191: Improve error handling specificity.The generic error handling in the pricing calculation could be more specific to help with debugging and monitoring.
} catch (e) { - console.error("Error calculating pricing", e); + console.error("Error calculating GPU pricing statistics:", { + error: e, + providersCount: providersWithBestBid?.length || 0, + context: 'getPricing' + }); return null; }apps/api/test/functional/gpu.spec.ts (1)
88-88: Remove debug console.log statements.Console.log statements should not be committed to the codebase as they can clutter the test output and may contain sensitive information.
const response = await app.request(`/v1/gpu?memory_size=2048`); expect(response.status).toBe(200); const data = await response.json(); -console.log(data); expect(data.gpus.total.allocatable).toBe(4);const response = await app.request(`/v1/gpu?provider=${providers[0].owner}`); expect(response.status).toBe(200); const data = await response.json(); -console.log(data); expect(data.gpus.total.allocatable).toBe(6);Also applies to: 104-104
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
apps/api/src/app.ts(2 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts(4 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts(2 hunks)apps/api/src/block/repositories/akash-block/akash-block.repository.ts(2 hunks)apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts(3 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts(2 hunks)apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts(2 hunks)apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts(2 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(4 hunks)apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts(3 hunks)apps/api/src/gpu/controllers/gpu.controller.ts(1 hunks)apps/api/src/gpu/http-schemas/gpu.schema.ts(1 hunks)apps/api/src/gpu/index.ts(1 hunks)apps/api/src/gpu/repositories/day.repository.ts(1 hunks)apps/api/src/gpu/repositories/gpu.repository.ts(1 hunks)apps/api/src/gpu/routes/gpu.router.ts(1 hunks)apps/api/src/gpu/routes/index.ts(1 hunks)apps/api/src/gpu/services/gpu-price.service.ts(1 hunks)apps/api/src/gpu/services/gpu.service.ts(1 hunks)apps/api/src/gpu/types/gpu.type.ts(1 hunks)apps/api/src/notifications/routes/proxy/proxy.route.spec.ts(2 hunks)apps/api/src/provider/repositories/provider/provider.repository.spec.ts(7 hunks)apps/api/src/routes/internal/index.ts(1 hunks)apps/api/src/routes/v1/gpu.ts(0 hunks)apps/api/src/routes/v1/gpuBreakdown.ts(0 hunks)apps/api/src/routes/v1/gpuModels.ts(0 hunks)apps/api/src/routes/v1/gpuPrices.ts(0 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/services/db/gpuBreakdownService.ts(0 hunks)apps/api/src/utils/env.ts(1 hunks)apps/api/test/functional/blocks.spec.ts(1 hunks)apps/api/test/functional/dashboard-data.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(7 hunks)apps/api/test/functional/leases-duration.spec.ts(6 hunks)apps/api/test/functional/network-capacity.spec.ts(8 hunks)apps/api/test/functional/provider-dashboard.spec.ts(2 hunks)apps/api/test/functional/provider-deployments.spec.ts(6 hunks)apps/api/test/functional/provider-graph-data.spec.ts(10 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(8 hunks)apps/api/test/functional/transactions.spec.ts(6 hunks)apps/api/test/functional/usage.spec.ts(11 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/seeders/akash-address.seeder.ts(1 hunks)apps/api/test/seeders/akash-block.seeder.ts(1 hunks)apps/api/test/seeders/akash-message.seeder.ts(1 hunks)apps/api/test/seeders/auto-top-up-deployment.seeder.ts(1 hunks)apps/api/test/seeders/block.seeder.ts(0 hunks)apps/api/test/seeders/chain-wallet.seeder.ts(1 hunks)apps/api/test/seeders/day.seeder.ts(1 hunks)apps/api/test/seeders/deployment-grant.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group-resource.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group.seeder.ts(1 hunks)apps/api/test/seeders/deployment-info.seeder.ts(2 hunks)apps/api/test/seeders/deployment.seeder.ts(1 hunks)apps/api/test/seeders/fees-authorization.seeder.ts(2 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/seeders/lease-api-response.seeder.ts(2 hunks)apps/api/test/seeders/lease.seeder.ts(1 hunks)apps/api/test/seeders/message.seeder.ts(0 hunks)apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot-node.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot.seeder.ts(1 hunks)apps/api/test/seeders/provider.seeder.ts(1 hunks)apps/api/test/seeders/transaction.seeder.ts(1 hunks)apps/api/test/seeders/user-wallet.seeder.ts(1 hunks)apps/api/test/seeders/validator.seeder.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (7)
- apps/api/src/routes/v1/gpuModels.ts
- apps/api/test/seeders/message.seeder.ts
- apps/api/src/routes/v1/gpuBreakdown.ts
- apps/api/test/seeders/block.seeder.ts
- apps/api/src/services/db/gpuBreakdownService.ts
- apps/api/src/routes/v1/gpu.ts
- apps/api/src/routes/v1/gpuPrices.ts
✅ Files skipped from review due to trivial changes (4)
- apps/api/src/gpu/routes/index.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts
- apps/api/src/gpu/types/gpu.type.ts
🚧 Files skipped from review as they are similar to previous changes (57)
- apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts
- apps/api/test/functional/provider-regions.spec.ts
- apps/api/src/app.ts
- apps/api/test/seeders/deployment-grant.seeder.ts
- apps/api/test/seeders/lease-api-response.seeder.ts
- apps/api/test/seeders/deployment-info.seeder.ts
- apps/api/test/seeders/fees-authorization.seeder.ts
- apps/api/tsconfig.strict.json
- apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
- apps/api/test/seeders/chain-wallet.seeder.ts
- apps/api/src/notifications/routes/proxy/proxy.route.spec.ts
- apps/api/test/functional/provider-dashboard.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
- apps/api/src/gpu/index.ts
- apps/api/src/block/repositories/akash-block/akash-block.repository.ts
- apps/api/src/gpu/repositories/day.repository.ts
- apps/api/src/billing/repositories/usage/usage.repository.spec.ts
- apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts
- apps/api/test/seeders/akash-message.seeder.ts
- apps/api/test/seeders/auto-top-up-deployment.seeder.ts
- apps/api/src/billing/services/usage/usage.service.spec.ts
- apps/api/test/seeders/user-wallet.seeder.ts
- apps/api/test/functional/leases-duration.spec.ts
- apps/api/test/functional/validators.spec.ts
- apps/api/src/routes/v1/index.ts
- apps/api/test/functional/provider-versions.spec.ts
- apps/api/test/functional/blocks.spec.ts
- apps/api/test/functional/transactions.spec.ts
- apps/api/src/routes/internal/index.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/src/utils/env.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/seeders/deployment-group-resource.seeder.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/seeders/provider-snapshot-node.seeder.ts
- apps/api/src/deployment/repositories/deployment/deployment.repository.ts
- apps/api/test/seeders/deployment.seeder.ts
- apps/api/test/seeders/akash-address.seeder.ts
- apps/api/test/seeders/deployment-group.seeder.ts
- apps/api/test/functional/usage.spec.ts
- apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts
- apps/api/test/seeders/lease.seeder.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/test/seeders/validator.seeder.ts
- apps/api/test/seeders/provider-snapshot.seeder.ts
- apps/api/test/seeders/day.seeder.ts
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- apps/api/test/seeders/index.ts
- apps/api/test/seeders/akash-block.seeder.ts
- apps/api/src/provider/repositories/provider/provider.repository.spec.ts
- apps/api/test/seeders/provider.seeder.ts
- apps/api/src/gpu/controllers/gpu.controller.ts
- apps/api/test/seeders/transaction.seeder.ts
- apps/api/src/gpu/routes/gpu.router.ts
- apps/api/src/gpu/services/gpu.service.ts
- apps/api/src/gpu/http-schemas/gpu.schema.ts
- apps/api/src/gpu/repositories/gpu.repository.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use a `setup` function instead of `beforeEach` for test se...
**/*.spec.{ts,tsx}: Use asetupfunction instead ofbeforeEachfor test setup. Thesetupfunction must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/api/test/functional/gpu.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-api-build
- GitHub Check: validate-api
🔇 Additional comments (3)
apps/api/test/functional/gpu.spec.ts (3)
377-712: Well-implemented setup function following coding guidelines.The setup function is properly placed at the bottom of the root describe block and correctly handles test data initialization. It follows the required pattern of accepting parameters with inline type definitions and returning the created objects without specifying a return type.
130-169: Good use of external API mocking.The test properly mocks the external GitHub API call for GPU model data, ensuring tests are isolated and don't depend on external services.
265-354: Comprehensive GPU pricing test coverage.The test thoroughly validates the GPU pricing endpoint response structure including availability totals, model-specific data, and pricing statistics. The test data setup includes all necessary entities (deployments, bids, transactions) to simulate realistic pricing scenarios.
| @Memoize({ ttlInSeconds: minutesToSeconds(15) }) | ||
| async getGpuPrices(debug: boolean) { | ||
| // Get list of GPUs (model,vendor, ram, interface) and their availability | ||
| const gpus = await this.gpuRepository.getGpusForPricing(); | ||
|
|
||
| const daysToInclude = 31; | ||
|
|
||
| // Get the height corresponding to the oldest time we want to include | ||
| const firstBlockToUse = await this.akashBlockRepository.getFirstBlockAfter(addDays(new Date(), -daysToInclude)); | ||
| if (!firstBlockToUse) { | ||
| throw new Error("No block found"); | ||
| } | ||
|
|
||
| // Fetch all deployments with GPU resources created during this period and their related MsgCreateBid messages | ||
| const deployments = await this.deploymentRepository.findAllWithGpuResources(firstBlockToUse.height); | ||
|
|
||
| // Fetch all days for the period to have historical AKT prices | ||
| const days = await this.dayRepository.getDaysAfter(addDays(new Date(), -(daysToInclude + 2))); | ||
|
|
||
| // Decode the MsgCreateBid messages and calculate the hourly and monthly price for each bid | ||
| const gpuBids = deployments | ||
| .flatMap(d => | ||
| d.relatedMessages.map(x => { | ||
| const day = days.find(day => day.id === x.block.dayId); | ||
| const decodedBid = decodeMsg("/akash.market.v1beta4.MsgCreateBid", x.data) as MsgCreateBid; | ||
|
|
||
| if (!day || !day.aktPrice) return null; // Ignore bids for days where we don't have the AKT price | ||
|
|
||
| if (decodedBid?.price?.denom !== "uakt") return null; // Ignore USDC bids for simplicity | ||
|
|
||
| return { | ||
| height: x.height, | ||
| txHash: x.transaction.hash, | ||
| datetime: x.block.datetime, | ||
| provider: decodedBid.provider, | ||
| aktTokenPrice: day.aktPrice, | ||
| hourlyPrice: this.blockPriceToHourlyPrice(parseFloat(decodedBid.price.amount), day?.aktPrice), | ||
| monthlyPrice: this.blockPriceToMonthlyPrice(parseFloat(decodedBid.price.amount), day?.aktPrice), | ||
| deployment: { | ||
| owner: d.owner, | ||
| cpuUnits: decodedBid.resourcesOffer | ||
| .flatMap(r => (r.resources?.cpu?.units?.val ? parseInt(uint8arrayToString(r.resources.cpu.units.val)) : 0)) | ||
| .reduce((a, b) => a + b, 0), | ||
| memoryUnits: decodedBid.resourcesOffer | ||
| .flatMap(r => (r.resources?.memory?.quantity?.val ? parseInt(uint8arrayToString(r.resources.memory.quantity.val)) : 0)) | ||
| .reduce((a, b) => a + b, 0), | ||
| storageUnits: decodedBid.resourcesOffer | ||
| .flatMap(r => r.resources?.storage) | ||
| .map(s => (s?.quantity?.val ? parseInt(uint8arrayToString(s.quantity.val)) : 0)) | ||
| .reduce((a, b) => a + b, 0), | ||
| gpus: decodedBid.resourcesOffer | ||
| .filter(x => (x.resources?.gpu?.units?.val ? parseInt(uint8arrayToString(x.resources.gpu.units.val)) : 0) > 0) | ||
| .flatMap(r => (r.resources?.gpu?.attributes ? this.getGpusFromAttributes(r.resources.gpu.attributes) : [])) | ||
| }, | ||
| data: decodedBid | ||
| }; | ||
| }) | ||
| ) | ||
| .filter(x => x) | ||
| .filter(x => x?.deployment?.gpus?.length === 1) as GpuBidType[]; // Ignore bids for deployments with more than 1 GPU | ||
|
|
||
| const gpuModels: GpuWithPricesType[] = gpus.map(x => ({ ...x, prices: [] as GpuBidType[] })); | ||
|
|
||
| // Add bids to their corresponding GPU models | ||
| for (const bid of gpuBids) { | ||
| const gpu = bid.deployment.gpus[0]; | ||
| const matchingGpuModels = gpuModels.filter(x => x.vendor === gpu.vendor && x.model === gpu.model); | ||
|
|
||
| for (const gpuModel of matchingGpuModels) { | ||
| gpuModel.prices.push(bid); | ||
| } | ||
| } | ||
|
|
||
| // Sort GPUs by vendor, model, ram, interface | ||
| gpuModels.sort( | ||
| (a, b) => a.vendor.localeCompare(b.vendor) || a.model.localeCompare(b.model) || a.ram.localeCompare(b.ram) || a.interface.localeCompare(b.interface) | ||
| ); | ||
|
|
||
| const totalAllocatable = gpuModels.map(x => x.allocatable).reduce((a, b) => a + b, 0); | ||
| const totalAllocated = gpuModels.map(x => x.allocated).reduce((a, b) => a + b, 0); | ||
|
|
||
| return { | ||
| availability: { | ||
| total: totalAllocatable, | ||
| available: totalAllocatable - totalAllocated | ||
| }, | ||
| models: gpuModels.map(x => { | ||
| /* | ||
| For each providers get their most relevent bid based on this order of priority: | ||
| 1- Most recent bid from the pricing bot (those deployment have tiny cpu/ram/storage specs to improve gpu price accuracy) | ||
| 2- Cheapest bid with matching ram and interface | ||
| 3- Cheapest bid with matching ram | ||
| 4- Cheapest remaining bid | ||
| 5- If no bids are found, increase search range from 14 to 31 days and repeat steps 2-4 | ||
| */ | ||
| const providersWithBestBid = x.providers | ||
| .map(p => { | ||
| const providerBids = x.prices.filter(b => b.provider === p.owner); | ||
| const providerBidsLast14d = providerBids.filter(x => x.datetime > addDays(new Date(), -14)); | ||
|
|
||
| const pricingBotAddress = env.PRICING_BOT_ADDRESS; | ||
| const bidsFromPricingBot = providerBids.filter(x => x.deployment.owner === pricingBotAddress && x.deployment.cpuUnits === 100); | ||
|
|
||
| let bestBid = null; | ||
| if (bidsFromPricingBot.length > 0) { | ||
| bestBid = bidsFromPricingBot.sort((a, b) => b.height - a.height)[0]; | ||
| } else { | ||
| bestBid = this.findBestProviderBid(providerBidsLast14d, x) ?? this.findBestProviderBid(providerBids, x); | ||
| } | ||
|
|
||
| return { | ||
| provider: p, | ||
| bestBid: bestBid | ||
| }; | ||
| }) | ||
| .filter(x => x.bestBid) as ProviderWithBestBid[]; | ||
|
|
||
| return { | ||
| vendor: x.vendor, | ||
| model: x.model, | ||
| ram: x.ram, | ||
| interface: x.interface, | ||
| availability: { | ||
| total: x.allocatable, | ||
| available: x.allocatable - x.allocated | ||
| }, | ||
| providerAvailability: { | ||
| total: x.providers.length, | ||
| available: x.availableProviders.length, | ||
| providers: debug ? x.providers : undefined | ||
| }, | ||
| price: this.getPricing(providersWithBestBid), | ||
| bidCount: debug ? x.prices.length : undefined, | ||
| providersWithBestBid: debug ? providersWithBestBid : undefined | ||
| }; | ||
| }) | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider breaking down the complex getGpuPrices method.
This method is quite long (137 lines) and handles multiple responsibilities including data fetching, bid processing, provider matching, and pricing calculations. This violates the Single Responsibility Principle and makes the code harder to test and maintain.
Consider extracting the following into separate private methods:
- Bid processing and decoding logic (lines 49-90)
- Provider best bid selection logic (lines 125-145)
- GPU model processing and sorting (lines 104-106, 116-165)
@Memoize({ ttlInSeconds: minutesToSeconds(15) })
async getGpuPrices(debug: boolean) {
const gpus = await this.gpuRepository.getGpusForPricing();
const { firstBlockToUse, deployments, days } = await this.fetchRequiredData();
- // Complex bid processing logic...
+ const gpuBids = await this.processGpuBids(deployments, days);
const gpuModels = this.createGpuModelsWithPrices(gpus, gpuBids);
return {
availability: this.calculateTotalAvailability(gpuModels),
- models: gpuModels.map(x => {
- // Complex provider processing...
- })
+ models: this.processGpuModelsWithProviders(gpuModels, debug)
};
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/src/gpu/services/gpu-price.service.ts between lines 30 and 167, the
getGpuPrices method is too long and handles multiple responsibilities, making it
hard to maintain and test. Refactor by extracting the bid processing and
decoding logic (lines 49-90) into a private method that returns processed bids,
move the provider best bid selection logic (lines 125-145) into another private
method that determines the best bid per provider, and isolate the GPU model
processing and sorting (lines 104-106 and 116-165) into a separate private
method that prepares and sorts GPU models with their prices. Then update
getGpuPrices to call these new methods to keep it concise and focused.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1551 +/- ##
==========================================
+ Coverage 40.98% 41.92% +0.94%
==========================================
Files 874 876 +2
Lines 21282 21334 +52
Branches 3866 3872 +6
==========================================
+ Hits 8723 8945 +222
+ Misses 12248 11811 -437
- Partials 311 578 +267
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
jzsfkzm
force-pushed
the
refactor/1279-gpu
branch
from
4b23e19 to
5518b7b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/api/test/functional/gpu.spec.ts (1)
88-88: Remove debug console.log statementsDebug logging statements should be removed from test files.
- console.log(data);Also applies to: 104-104
apps/api/src/gpu/repositories/gpu.repository.ts (2)
84-125: Consider optimizing complex GPU utilization queryThe GPU utilization calculation is complex with nested subqueries that could impact performance on large datasets. Consider:
- Adding indexes on frequently queried columns
- Breaking down the complex calculation into smaller CTEs for better readability
- Adding inline comments to explain the allocation distribution logic
190-224: Consider moving aggregation logic to the databaseThe in-memory aggregation of GPU data could be performed more efficiently in the SQL query using GROUP BY and array aggregation functions. This would reduce memory usage and improve performance for large datasets.
Example approach using SQL aggregation:
SELECT vendor, "modelName" as model, interface, "memorySize" as ram, SUM(allocatable) as allocatable, SUM(allocated) as allocated, JSON_AGG( JSON_BUILD_OBJECT( 'owner', owner, 'hostUri', "hostUri", 'allocatable', allocatable, 'allocated', allocated ) ) FILTER (WHERE allocated < allocatable) as availableProviders FROM (your_existing_query) as gpu_data GROUP BY vendor, "modelName", interface, "memorySize"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
apps/api/src/app.ts(2 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts(4 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts(2 hunks)apps/api/src/block/repositories/akash-block/akash-block.repository.ts(2 hunks)apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts(3 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts(2 hunks)apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts(2 hunks)apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts(2 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(4 hunks)apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts(3 hunks)apps/api/src/gpu/controllers/gpu.controller.ts(1 hunks)apps/api/src/gpu/http-schemas/gpu.schema.ts(1 hunks)apps/api/src/gpu/index.ts(1 hunks)apps/api/src/gpu/repositories/day.repository.ts(1 hunks)apps/api/src/gpu/repositories/gpu.repository.ts(1 hunks)apps/api/src/gpu/routes/gpu.router.ts(1 hunks)apps/api/src/gpu/routes/index.ts(1 hunks)apps/api/src/gpu/services/gpu-price.service.ts(1 hunks)apps/api/src/gpu/services/gpu.service.ts(1 hunks)apps/api/src/gpu/types/gpu.type.ts(1 hunks)apps/api/src/notifications/routes/proxy/proxy.route.spec.ts(2 hunks)apps/api/src/provider/repositories/provider/provider.repository.spec.ts(7 hunks)apps/api/src/routes/internal/index.ts(1 hunks)apps/api/src/routes/v1/gpu.ts(0 hunks)apps/api/src/routes/v1/gpuBreakdown.ts(0 hunks)apps/api/src/routes/v1/gpuModels.ts(0 hunks)apps/api/src/routes/v1/gpuPrices.ts(0 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/services/db/gpuBreakdownService.ts(0 hunks)apps/api/src/utils/env.ts(1 hunks)apps/api/test/functional/blocks.spec.ts(2 hunks)apps/api/test/functional/dashboard-data.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(7 hunks)apps/api/test/functional/leases-duration.spec.ts(6 hunks)apps/api/test/functional/network-capacity.spec.ts(8 hunks)apps/api/test/functional/provider-dashboard.spec.ts(2 hunks)apps/api/test/functional/provider-deployments.spec.ts(6 hunks)apps/api/test/functional/provider-graph-data.spec.ts(10 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(8 hunks)apps/api/test/functional/transactions.spec.ts(6 hunks)apps/api/test/functional/usage.spec.ts(11 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/seeders/akash-address.seeder.ts(1 hunks)apps/api/test/seeders/akash-block.seeder.ts(1 hunks)apps/api/test/seeders/akash-message.seeder.ts(1 hunks)apps/api/test/seeders/auto-top-up-deployment.seeder.ts(1 hunks)apps/api/test/seeders/block.seeder.ts(0 hunks)apps/api/test/seeders/chain-wallet.seeder.ts(1 hunks)apps/api/test/seeders/day.seeder.ts(1 hunks)apps/api/test/seeders/deployment-grant.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group-resource.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group.seeder.ts(1 hunks)apps/api/test/seeders/deployment-info.seeder.ts(2 hunks)apps/api/test/seeders/deployment.seeder.ts(1 hunks)apps/api/test/seeders/fees-authorization.seeder.ts(2 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/seeders/lease-api-response.seeder.ts(2 hunks)apps/api/test/seeders/lease.seeder.ts(1 hunks)apps/api/test/seeders/message.seeder.ts(0 hunks)apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot-node.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot.seeder.ts(1 hunks)apps/api/test/seeders/provider.seeder.ts(1 hunks)apps/api/test/seeders/transaction.seeder.ts(1 hunks)apps/api/test/seeders/user-wallet.seeder.ts(1 hunks)apps/api/test/seeders/validator.seeder.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (7)
- apps/api/src/routes/v1/gpuModels.ts
- apps/api/src/routes/v1/gpuBreakdown.ts
- apps/api/test/seeders/message.seeder.ts
- apps/api/test/seeders/block.seeder.ts
- apps/api/src/services/db/gpuBreakdownService.ts
- apps/api/src/routes/v1/gpu.ts
- apps/api/src/routes/v1/gpuPrices.ts
✅ Files skipped from review due to trivial changes (6)
- apps/api/src/gpu/routes/index.ts
- apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts
- apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts
- apps/api/src/routes/v1/index.ts
- apps/api/test/seeders/index.ts
- apps/api/src/gpu/types/gpu.type.ts
🚧 Files skipped from review as they are similar to previous changes (55)
- apps/api/src/app.ts
- apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts
- apps/api/src/gpu/index.ts
- apps/api/tsconfig.strict.json
- apps/api/test/seeders/auto-top-up-deployment.seeder.ts
- apps/api/src/billing/services/usage/usage.service.spec.ts
- apps/api/test/seeders/deployment-info.seeder.ts
- apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
- apps/api/test/functional/provider-regions.spec.ts
- apps/api/test/seeders/user-wallet.seeder.ts
- apps/api/test/functional/validators.spec.ts
- apps/api/test/seeders/deployment-grant.seeder.ts
- apps/api/src/notifications/routes/proxy/proxy.route.spec.ts
- apps/api/src/gpu/repositories/day.repository.ts
- apps/api/test/seeders/fees-authorization.seeder.ts
- apps/api/test/functional/provider-versions.spec.ts
- apps/api/src/billing/repositories/usage/usage.repository.spec.ts
- apps/api/src/routes/internal/index.ts
- apps/api/test/seeders/akash-block.seeder.ts
- apps/api/src/block/repositories/akash-block/akash-block.repository.ts
- apps/api/test/seeders/chain-wallet.seeder.ts
- apps/api/src/provider/repositories/provider/provider.repository.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/test/seeders/akash-message.seeder.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/functional/transactions.spec.ts
- apps/api/test/seeders/transaction.seeder.ts
- apps/api/test/seeders/deployment.seeder.ts
- apps/api/test/seeders/day.seeder.ts
- apps/api/test/seeders/provider-snapshot.seeder.ts
- apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- apps/api/test/seeders/akash-address.seeder.ts
- apps/api/test/seeders/provider-snapshot-node.seeder.ts
- apps/api/test/seeders/deployment-group.seeder.ts
- apps/api/src/gpu/controllers/gpu.controller.ts
- apps/api/test/seeders/lease.seeder.ts
- apps/api/test/seeders/provider.seeder.ts
- apps/api/test/seeders/deployment-group-resource.seeder.ts
- apps/api/test/functional/provider-dashboard.spec.ts
- apps/api/test/seeders/lease-api-response.seeder.ts
- apps/api/src/gpu/routes/gpu.router.ts
- apps/api/src/utils/env.ts
- apps/api/src/gpu/http-schemas/gpu.schema.ts
- apps/api/test/functional/leases-duration.spec.ts
- apps/api/src/gpu/services/gpu.service.ts
- apps/api/test/functional/blocks.spec.ts
- apps/api/src/gpu/services/gpu-price.service.ts
- apps/api/src/deployment/repositories/deployment/deployment.repository.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/test/functional/usage.spec.ts
- apps/api/test/seeders/validator.seeder.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.spec.{ts,tsx}`: Use a `setup` function instead of `beforeEach` for test se...
**/*.spec.{ts,tsx}: Use asetupfunction instead ofbeforeEachfor test setup. Thesetupfunction must be at the bottom of the rootdescribeblock, create and return the object under test, accept a single parameter with inline type definition, avoid shared state, and should not specify a return type.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/setup-instead-of-before-each.mdc)
List of files the instruction was applied to:
apps/api/test/functional/gpu.spec.ts
🧠 Learnings (2)
apps/api/test/functional/gpu.spec.ts (2)
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-06-23T12:32:22.586Z
Learning: In TypeScript React test files (`**/*.spec.tsx` and `**/*.spec.ts`), prefer using a `setup` function over `beforeEach` for test initialization. The `setup` function should be placed at the bottom of the root `describe` block, accept a single parameter with an inline type definition, avoid shared state, and not specify a return type. This pattern improves test clarity and reduces side effects.
apps/api/src/gpu/repositories/gpu.repository.ts (1)
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
[discussion](akash-network#914)
jzsfkzm
force-pushed
the
refactor/1279-gpu
branch
from
521c1ff to
73a6680
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/api/src/gpu/services/gpu.service.ts (2)
41-42: Extract complex type definition for better readability.Consider extracting the inline type definition to improve code readability and maintainability.
Add a type definition before the method:
type GpuDetails = { model: string; ram: string; interface: string; allocatable: number; allocated: number; }; type GpuDetailsMap = { [vendor: string]: GpuDetails[] };Then simplify line 42:
- details: {} as { [key: string]: { model: string; ram: string; interface: string; allocatable: number; allocated: number }[] } + details: {} as GpuDetailsMap
91-96: Improve type safety by avoiding type casting.The type casting on lines 92-96 suggests that
ProviderConfigGpusTypemight not be properly typed. Consider updating the type definition to avoid runtime type assertions.If
modelValueis properly typed inProviderConfigGpusType, you can remove the casting:- const _modelValue = modelValue as { - name: string; - memory_size: string; - interface: string; - }; - const existingModel = vendor.models.find(x => x.name === _modelValue.name); + const existingModel = vendor.models.find(x => x.name === modelValue.name);apps/api/src/gpu/repositories/gpu.repository.ts (4)
39-39: Document PostgreSQL-specific syntax usage.The query uses
DISTINCT ON, which is PostgreSQL-specific syntax. Consider adding a comment to document this database dependency for future maintainability.Add a comment above the query:
+ // Note: This query uses PostgreSQL-specific DISTINCT ON syntax return await chainDb.query<{Also applies to: 48-48
92-105: Add documentation for complex GPU utilization calculations.The GPU allocation and utilization calculations are complex and would benefit from inline comments explaining the logic, especially the subquery counting and percentage calculations.
Add comments to explain the calculation logic:
-- Calculate leased GPUs: For each node, divide allocated GPUs by total GPU count -- LEAST ensures we don't exceed the total GPU count LEAST(COALESCE(CAST(ROUND(SUM( CAST(n."gpuAllocated" as float) / NULLIF((SELECT COUNT(*) FROM "providerSnapshotNodeGPU" subgpu WHERE subgpu."snapshotNodeId" = n.id), 0) )) as int), 0), COUNT(gpu.id)) as leased_gpus, -- Calculate utilization percentage: (leased / total) * 100 -- LEAST caps the result at 100% LEAST(CAST(COALESCE( SUM( CAST(n."gpuAllocated" as float) / NULLIF((SELECT COUNT(*) FROM "providerSnapshotNodeGPU" subgpu WHERE subgpu."snapshotNodeId" = n.id), 0) ) * 100.0 / NULLIF(COUNT(gpu.id), 0) , 0) as numeric(10,2)), 100.00) as "gpuUtilization"
148-227: Consider SQL-based aggregation for better performance.The method performs manual aggregation in TypeScript (lines 190-224), which could be more efficiently done in SQL. This would reduce data transfer and improve performance for large datasets.
Consider refactoring to use SQL aggregation:
WITH gpu_aggregated AS ( -- Your existing query here SELECT gpu.vendor, gpu.name AS "modelName", gpu."memorySize", gpu.interface, SUM(n."gpuAllocatable") AS total_allocatable, SUM(LEAST(n."gpuAllocated", n."gpuAllocatable")) AS total_allocated, json_agg(json_build_object( 'owner', s.owner, 'hostUri', s.hostUri, 'allocatable', n."gpuAllocatable", 'allocated', LEAST(n."gpuAllocated", n."gpuAllocatable") )) AS providers FROM snapshots s -- ... rest of joins GROUP BY gpu.vendor, gpu.name, gpu."memorySize", gpu.interface ) SELECT * FROM gpu_aggregated;This would return pre-aggregated data, reducing the need for manual processing in TypeScript.
161-181: Extract common query patterns to reduce duplication.The CTE pattern for selecting snapshots is duplicated between
getGpuListandgetGpusForPricing. Consider extracting this as a private method or SQL fragment.Create a private method to generate the common CTE:
private getSnapshotsCTE(): string { return ` WITH snapshots AS ( SELECT DISTINCT ON("hostUri") ps.id AS id, "hostUri", p."owner" FROM provider p INNER JOIN "providerSnapshot" ps ON ps.id=p."lastSuccessfulSnapshotId" WHERE p."isOnline" IS TRUE OR ps."checkDate" >= :grace_date ORDER BY p."hostUri", p."createdHeight" DESC ) `; }Then reuse in both methods to improve maintainability and reduce duplication.
Also applies to: 37-57
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (70)
apps/api/src/app.ts(2 hunks)apps/api/src/billing/repositories/usage/usage.repository.spec.ts(4 hunks)apps/api/src/billing/services/usage/usage.service.spec.ts(2 hunks)apps/api/src/block/repositories/akash-block/akash-block.repository.ts(2 hunks)apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts(3 hunks)apps/api/src/deployment/repositories/deployment/deployment.repository.ts(2 hunks)apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts(2 hunks)apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts(2 hunks)apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts(4 hunks)apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts(3 hunks)apps/api/src/gpu/controllers/gpu.controller.ts(1 hunks)apps/api/src/gpu/http-schemas/gpu.schema.ts(1 hunks)apps/api/src/gpu/index.ts(1 hunks)apps/api/src/gpu/repositories/day.repository.ts(1 hunks)apps/api/src/gpu/repositories/gpu.repository.ts(1 hunks)apps/api/src/gpu/routes/gpu.router.ts(1 hunks)apps/api/src/gpu/routes/index.ts(1 hunks)apps/api/src/gpu/services/gpu-price.service.ts(1 hunks)apps/api/src/gpu/services/gpu.service.ts(1 hunks)apps/api/src/gpu/types/gpu.type.ts(1 hunks)apps/api/src/notifications/routes/proxy/proxy.route.spec.ts(2 hunks)apps/api/src/provider/repositories/provider/provider.repository.spec.ts(7 hunks)apps/api/src/routes/internal/index.ts(1 hunks)apps/api/src/routes/v1/gpu.ts(0 hunks)apps/api/src/routes/v1/gpuBreakdown.ts(0 hunks)apps/api/src/routes/v1/gpuModels.ts(0 hunks)apps/api/src/routes/v1/gpuPrices.ts(0 hunks)apps/api/src/routes/v1/index.ts(1 hunks)apps/api/src/services/db/gpuBreakdownService.ts(0 hunks)apps/api/src/utils/env.ts(1 hunks)apps/api/test/functional/blocks.spec.ts(2 hunks)apps/api/test/functional/dashboard-data.spec.ts(8 hunks)apps/api/test/functional/gpu.spec.ts(1 hunks)apps/api/test/functional/graph-data.spec.ts(7 hunks)apps/api/test/functional/leases-duration.spec.ts(6 hunks)apps/api/test/functional/network-capacity.spec.ts(8 hunks)apps/api/test/functional/provider-dashboard.spec.ts(2 hunks)apps/api/test/functional/provider-deployments.spec.ts(6 hunks)apps/api/test/functional/provider-graph-data.spec.ts(10 hunks)apps/api/test/functional/provider-regions.spec.ts(1 hunks)apps/api/test/functional/provider-versions.spec.ts(2 hunks)apps/api/test/functional/providers.spec.ts(8 hunks)apps/api/test/functional/transactions.spec.ts(6 hunks)apps/api/test/functional/usage.spec.ts(11 hunks)apps/api/test/functional/validators.spec.ts(1 hunks)apps/api/test/seeders/akash-address.seeder.ts(1 hunks)apps/api/test/seeders/akash-block.seeder.ts(1 hunks)apps/api/test/seeders/akash-message.seeder.ts(1 hunks)apps/api/test/seeders/auto-top-up-deployment.seeder.ts(1 hunks)apps/api/test/seeders/block.seeder.ts(0 hunks)apps/api/test/seeders/chain-wallet.seeder.ts(1 hunks)apps/api/test/seeders/day.seeder.ts(1 hunks)apps/api/test/seeders/deployment-grant.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group-resource.seeder.ts(1 hunks)apps/api/test/seeders/deployment-group.seeder.ts(1 hunks)apps/api/test/seeders/deployment-info.seeder.ts(2 hunks)apps/api/test/seeders/deployment.seeder.ts(1 hunks)apps/api/test/seeders/fees-authorization.seeder.ts(2 hunks)apps/api/test/seeders/index.ts(1 hunks)apps/api/test/seeders/lease-api-response.seeder.ts(2 hunks)apps/api/test/seeders/lease.seeder.ts(1 hunks)apps/api/test/seeders/message.seeder.ts(0 hunks)apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot-node.seeder.ts(1 hunks)apps/api/test/seeders/provider-snapshot.seeder.ts(1 hunks)apps/api/test/seeders/provider.seeder.ts(1 hunks)apps/api/test/seeders/transaction.seeder.ts(1 hunks)apps/api/test/seeders/user-wallet.seeder.ts(1 hunks)apps/api/test/seeders/validator.seeder.ts(1 hunks)apps/api/tsconfig.strict.json(1 hunks)
💤 Files with no reviewable changes (7)
- apps/api/src/routes/v1/gpuBreakdown.ts
- apps/api/src/routes/v1/gpuModels.ts
- apps/api/test/seeders/message.seeder.ts
- apps/api/test/seeders/block.seeder.ts
- apps/api/src/routes/v1/gpu.ts
- apps/api/src/services/db/gpuBreakdownService.ts
- apps/api/src/routes/v1/gpuPrices.ts
✅ Files skipped from review due to trivial changes (3)
- apps/api/src/gpu/routes/index.ts
- apps/api/src/deployment/services/draining-deployment/draining-deployment.service.spec.ts
- apps/api/test/seeders/index.ts
🚧 Files skipped from review as they are similar to previous changes (58)
- apps/api/src/deployment/services/cached-balance/cached-balance.service.spec.ts
- apps/api/src/gpu/index.ts
- apps/api/tsconfig.strict.json
- apps/api/test/seeders/chain-wallet.seeder.ts
- apps/api/test/seeders/deployment-grant.seeder.ts
- apps/api/test/seeders/deployment-info.seeder.ts
- apps/api/test/seeders/fees-authorization.seeder.ts
- apps/api/src/deployment/lib/top-up-summarizer/top-up-summarizer.spec.ts
- apps/api/test/seeders/lease-api-response.seeder.ts
- apps/api/test/functional/provider-regions.spec.ts
- apps/api/src/app.ts
- apps/api/test/seeders/auto-top-up-deployment.seeder.ts
- apps/api/src/billing/services/usage/usage.service.spec.ts
- apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
- apps/api/src/routes/v1/index.ts
- apps/api/test/functional/provider-versions.spec.ts
- apps/api/test/functional/blocks.spec.ts
- apps/api/src/notifications/routes/proxy/proxy.route.spec.ts
- apps/api/src/block/repositories/akash-block/akash-block.repository.ts
- apps/api/test/functional/validators.spec.ts
- apps/api/test/functional/provider-dashboard.spec.ts
- apps/api/src/gpu/repositories/day.repository.ts
- apps/api/test/seeders/user-wallet.seeder.ts
- apps/api/test/functional/network-capacity.spec.ts
- apps/api/test/functional/dashboard-data.spec.ts
- apps/api/test/functional/provider-graph-data.spec.ts
- apps/api/test/functional/leases-duration.spec.ts
- apps/api/src/utils/env.ts
- apps/api/test/seeders/akash-address.seeder.ts
- apps/api/test/seeders/transaction.seeder.ts
- apps/api/test/functional/providers.spec.ts
- apps/api/src/routes/internal/index.ts
- apps/api/test/seeders/akash-block.seeder.ts
- apps/api/test/functional/transactions.spec.ts
- apps/api/test/seeders/akash-message.seeder.ts
- apps/api/test/seeders/deployment.seeder.ts
- apps/api/src/provider/repositories/provider/provider.repository.spec.ts
- apps/api/test/functional/provider-deployments.spec.ts
- apps/api/test/seeders/provider-snapshot.seeder.ts
- apps/api/src/billing/repositories/usage/usage.repository.spec.ts
- apps/api/test/seeders/day.seeder.ts
- apps/api/test/seeders/deployment-group.seeder.ts
- apps/api/test/seeders/deployment-group-resource.seeder.ts
- apps/api/test/seeders/provider-snapshot-node-gpu.seeder.ts
- apps/api/src/deployment/services/gpu-bids-creator/gpu-bids-creator.service.ts
- apps/api/test/seeders/provider-snapshot-node.seeder.ts
- apps/api/test/functional/graph-data.spec.ts
- apps/api/test/seeders/validator.seeder.ts
- apps/api/src/deployment/repositories/deployment/deployment.repository.ts
- apps/api/test/functional/usage.spec.ts
- apps/api/test/seeders/provider.seeder.ts
- apps/api/test/seeders/lease.seeder.ts
- apps/api/src/gpu/types/gpu.type.ts
- apps/api/src/gpu/controllers/gpu.controller.ts
- apps/api/test/functional/gpu.spec.ts
- apps/api/src/gpu/routes/gpu.router.ts
- apps/api/src/gpu/http-schemas/gpu.schema.ts
- apps/api/src/gpu/services/gpu-price.service.ts
🧰 Additional context used
🧠 Learnings (1)
apps/api/src/gpu/repositories/gpu.repository.ts (1)
Learnt from: stalniy
PR: akash-network/console#1436
File: apps/api/src/provider/repositories/provider/provider.repository.ts:79-90
Timestamp: 2025-06-08T03:07:13.871Z
Learning: The getProvidersHostUriByAttributes method in apps/api/src/provider/repositories/provider/provider.repository.ts already has comprehensive test coverage in provider.repository.spec.ts, including tests for complex HAVING clause logic with COUNT(*) FILTER (WHERE ...) syntax, signature conditions (anyOf/allOf), and glob pattern matching.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-build
- GitHub Check: test-build
|
|
||
| @Memoize({ ttlInSeconds: minutesToSeconds(2) }) | ||
| async getGpuModels() { | ||
| const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move hardcoded URL to configuration.
The hardcoded GitHub URL should be moved to environment configuration for better maintainability and flexibility across different environments.
Add to your environment configuration:
GPU_MODELS_URL: string = "https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"Then update the code:
- const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json");
+ const response = await axios.get<ProviderConfigGpusType>(env.GPU_MODELS_URL);🤖 Prompt for AI Agents
In apps/api/src/gpu/services/gpu.service.ts at line 80, the GitHub URL is
hardcoded in the axios.get call. To fix this, move the URL string to an
environment configuration variable named GPU_MODELS_URL. Update the code to read
the URL from this environment variable instead of using the hardcoded string,
improving maintainability and flexibility.
| allocatable: gpuNodes.map(x => x.allocatable).reduce((acc, x) => acc + x, 0), | ||
| allocated: gpuNodes.map(x => x.allocated).reduce((acc, x) => acc + x, 0) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize array operations by combining map and reduce.
The current implementation creates an intermediate array with map() before reducing. This can be optimized into a single reduce operation for better performance.
- allocatable: gpuNodes.map(x => x.allocatable).reduce((acc, x) => acc + x, 0),
- allocated: gpuNodes.map(x => x.allocated).reduce((acc, x) => acc + x, 0)
+ allocatable: gpuNodes.reduce((acc, x) => acc + x.allocatable, 0),
+ allocated: gpuNodes.reduce((acc, x) => acc + x.allocated, 0)📝 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.
| allocatable: gpuNodes.map(x => x.allocatable).reduce((acc, x) => acc + x, 0), | |
| allocated: gpuNodes.map(x => x.allocated).reduce((acc, x) => acc + x, 0) | |
| allocatable: gpuNodes.reduce((acc, x) => acc + x.allocatable, 0), | |
| allocated: gpuNodes.reduce((acc, x) => acc + x.allocated, 0) |
🤖 Prompt for AI Agents
In apps/api/src/gpu/services/gpu.service.ts around lines 38 to 39, the code uses
map followed by reduce to sum properties, which creates unnecessary intermediate
arrays. Refactor by replacing each map-reduce pair with a single reduce that
accumulates the desired property directly, improving performance by avoiding
extra array creation.
| }; | ||
|
|
||
| for (const gpuNode of gpuNodes) { | ||
| const vendorName = gpuNode.vendor ?? "<UNKNOWN>"; |
There was a problem hiding this comment.
Inconsistent default vendor naming with repository.
The service uses "<UNKNOWN>" as the default vendor name, while the repository uses 'Unknown' (see gpu.repository.ts lines 87-88). This inconsistency could lead to confusion.
- const vendorName = gpuNode.vendor ?? "<UNKNOWN>";
+ const vendorName = gpuNode.vendor ?? "Unknown";🤖 Prompt for AI Agents
In apps/api/src/gpu/services/gpu.service.ts at line 46, the default vendor name
is set to "<UNKNOWN>", which is inconsistent with the repository's use of
'Unknown'. Change the default vendor name to 'Unknown' to match the repository
and maintain consistency across the codebase.
| @Memoize({ ttlInSeconds: minutesToSeconds(2) }) | ||
| async getGpuModels() { | ||
| const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"); | ||
| const gpuModels: GpuVendor[] = []; | ||
|
|
||
| // Loop over vendors | ||
| for (const [, vendorValue] of Object.entries(response.data)) { | ||
| const vendor: GpuVendor = { | ||
| name: vendorValue.name, | ||
| models: [] | ||
| }; | ||
|
|
||
| // Loop over models | ||
| for (const [, modelValue] of Object.entries(vendorValue.devices)) { | ||
| const _modelValue = modelValue as { | ||
| name: string; | ||
| memory_size: string; | ||
| interface: string; | ||
| }; | ||
| const existingModel = vendor.models.find(x => x.name === _modelValue.name); | ||
|
|
||
| if (existingModel) { | ||
| if (!existingModel.memory.includes(_modelValue.memory_size)) { | ||
| existingModel.memory.push(_modelValue.memory_size); | ||
| } | ||
| if (!existingModel.interface.includes(this.getGpuInterface(_modelValue.interface))) { | ||
| existingModel.interface.push(this.getGpuInterface(_modelValue.interface)); | ||
| } | ||
| } else { | ||
| vendor.models.push({ | ||
| name: _modelValue.name, | ||
| memory: [_modelValue.memory_size], | ||
| interface: [this.getGpuInterface(_modelValue.interface)] | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| gpuModels.push(vendor); | ||
| } | ||
|
|
||
| return gpuModels; | ||
| } |
There was a problem hiding this comment.
Add error handling for external API call.
The method lacks error handling for the axios request, which could cause unhandled exceptions if the external API is unavailable.
Wrap the axios call in a try-catch block:
async getGpuModels() {
+ try {
const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json");
const gpuModels: GpuVendor[] = [];
// ... rest of the method
return gpuModels;
+ } catch (error) {
+ // Log error and return empty array or cached value
+ console.error("Failed to fetch GPU models:", error);
+ return [];
+ }
}📝 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.
| @Memoize({ ttlInSeconds: minutesToSeconds(2) }) | |
| async getGpuModels() { | |
| const response = await axios.get<ProviderConfigGpusType>("https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json"); | |
| const gpuModels: GpuVendor[] = []; | |
| // Loop over vendors | |
| for (const [, vendorValue] of Object.entries(response.data)) { | |
| const vendor: GpuVendor = { | |
| name: vendorValue.name, | |
| models: [] | |
| }; | |
| // Loop over models | |
| for (const [, modelValue] of Object.entries(vendorValue.devices)) { | |
| const _modelValue = modelValue as { | |
| name: string; | |
| memory_size: string; | |
| interface: string; | |
| }; | |
| const existingModel = vendor.models.find(x => x.name === _modelValue.name); | |
| if (existingModel) { | |
| if (!existingModel.memory.includes(_modelValue.memory_size)) { | |
| existingModel.memory.push(_modelValue.memory_size); | |
| } | |
| if (!existingModel.interface.includes(this.getGpuInterface(_modelValue.interface))) { | |
| existingModel.interface.push(this.getGpuInterface(_modelValue.interface)); | |
| } | |
| } else { | |
| vendor.models.push({ | |
| name: _modelValue.name, | |
| memory: [_modelValue.memory_size], | |
| interface: [this.getGpuInterface(_modelValue.interface)] | |
| }); | |
| } | |
| } | |
| gpuModels.push(vendor); | |
| } | |
| return gpuModels; | |
| } | |
| @Memoize({ ttlInSeconds: minutesToSeconds(2) }) | |
| async getGpuModels() { | |
| try { | |
| const response = await axios.get<ProviderConfigGpusType>( | |
| "https://raw.githubusercontent.com/akash-network/provider-configs/main/devices/pcie/gpus.json" | |
| ); | |
| const gpuModels: GpuVendor[] = []; | |
| // Loop over vendors | |
| for (const [, vendorValue] of Object.entries(response.data)) { | |
| const vendor: GpuVendor = { | |
| name: vendorValue.name, | |
| models: [] | |
| }; | |
| // Loop over models | |
| for (const [, modelValue] of Object.entries(vendorValue.devices)) { | |
| const _modelValue = modelValue as { | |
| name: string; | |
| memory_size: string; | |
| interface: string; | |
| }; | |
| const existingModel = vendor.models.find(x => x.name === _modelValue.name); | |
| if (existingModel) { | |
| if (!existingModel.memory.includes(_modelValue.memory_size)) { | |
| existingModel.memory.push(_modelValue.memory_size); | |
| } | |
| if (!existingModel.interface.includes(this.getGpuInterface(_modelValue.interface))) { | |
| existingModel.interface.push(this.getGpuInterface(_modelValue.interface)); | |
| } | |
| } else { | |
| vendor.models.push({ | |
| name: _modelValue.name, | |
| memory: [_modelValue.memory_size], | |
| interface: [this.getGpuInterface(_modelValue.interface)] | |
| }); | |
| } | |
| } | |
| gpuModels.push(vendor); | |
| } | |
| return gpuModels; | |
| } catch (error) { | |
| console.error("Failed to fetch GPU models:", error); | |
| return []; | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/gpu/services/gpu.service.ts around lines 78 to 119, the
getGpuModels method makes an axios request without error handling, risking
unhandled exceptions if the external API fails. Wrap the axios.get call in a
try-catch block to catch any errors, log or handle the error appropriately, and
ensure the method either returns a fallback value or rethrows the error in a
controlled manner.
* refactor: refactor seeders as discussed [discussion](#914) * test: add tests for gpu endpoints refs #1279 * refactor: move gpu endpoints to modules closes #1279 * fix: follow-up on rabbit reviews refs #1279 * test: fix typing issues refs #1279 --------- Co-authored-by: Sergii <sergiy.stotskiy@gmail.com>
3 participants
closes #1279
Summary by CodeRabbit
New Features
Refactor
Chores
Bug Fixes