Pass runConfig from apphosting.yaml during local builds - #10973
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for runConfig in App Hosting configurations, allowing settings like CPU, memory, concurrency, and instance limits to be parsed from apphosting.yaml, merged, and passed through local builds to rollout orchestration. Feedback on the changes suggests removing the index signature [key: string]: unknown; from ApiRunConfig to enforce strict typing in accordance with the style guide, and explicitly copying known fields in toApiRunConfig instead of using object rest spread to avoid sending unsupported fields to the GCP API.
| export interface ApiRunConfig { | ||
| cpu?: number; | ||
| memoryMib?: number; | ||
| concurrency?: number; | ||
| minInstances?: number; | ||
| maxInstances?: number; | ||
| [key: string]: unknown; | ||
| } |
There was a problem hiding this comment.
The index signature [key: string]: unknown; weakens type safety by allowing arbitrary properties to be added to ApiRunConfig without compiler errors. Since all valid fields are already explicitly defined, we should remove the index signature to enforce strict typing and adhere to the repository style guide.
export interface ApiRunConfig {
cpu?: number;
memoryMib?: number;
concurrency?: number;
minInstances?: number;
maxInstances?: number;
}References
- Never use
anyorunknownas an escape hatch. Define proper interfaces/types or use type guards. (link)
b6f3b50 to
f97bde9
Compare
f97bde9 to
0a6c7a5
Compare
Description
#10956
For local builds, we were not correctly passing the CPU/Memory Overrides
Scenarios Tested
Sample Commands