fix(deployment): block nvidia rtx5090m for trial deployments - #3346
Conversation
📝 WalkthroughWalkthroughAdds ChangesBlocked GPU Models Environment Config
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3346 +/- ##
==========================================
- Coverage 69.63% 69.63% -0.01%
==========================================
Files 1087 1088 +1
Lines 26672 26689 +17
Branches 6415 6420 +5
==========================================
+ Hits 18574 18585 +11
Misses 7111 7111
- Partials 987 993 +6
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
bdaad73 to
b32bded
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/deploy-web/env/.env.production`:
- Around line 38-39: The environment variable
NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELS is defined in
.env.production but missing from the environment template files. Add this
variable to .env.sample and other environment template files (.env.staging,
.env.staging-testnet) using the same value that is defined in .env.production
(the comma-separated list of blocked GPU models:
nvidia/b300,nvidia/b200,nvidia/h200,nvidia/h100,nvidia/pro6000se,nvidia/pro6000we,nvidia/a100,nvidia/rtx5090,nvidia/rtx5090m,nvidia/rtx4090,nvidia/rtx3090)
to ensure developers have proper documentation of this configuration when
setting up their environments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80bde89d-2ff6-4ef6-995d-fcb9c5fd5408
📒 Files selected for processing (1)
apps/deploy-web/env/.env.production
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/deploy-web/env/.env.sample (2)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: Key ordering differs from static analysis baseline.
The static analysis tool (dotenv-linter) flags this entry as out of lexicographic order—it should appear before
NEXT_PUBLIC_TURNSTILE_ENABLED. While not a functional issue, reordering would align with the tool's baseline and improve consistency.📋 Suggested reordering
Move the
NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELSentry to appear before any keys starting with "NEXT_PUBLIC_T" to restore alphabetical order.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/deploy-web/env/.env.sample` at line 53, The environment variable NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELS is positioned out of lexicographic order according to dotenv-linter's baseline. Move this key entry to appear before any keys starting with NEXT_PUBLIC_T (such as NEXT_PUBLIC_TURNSTILE_ENABLED) to restore proper alphabetical ordering and align with the static analysis tool's expectations for the .env.sample file.Source: Linters/SAST tools
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSample entry is minimal but lacks guidance on format.
The blank value matches the pattern used for other environment-specific variables, satisfying the path instruction requirement for example-file entries. However, since the schema enforces a strict comma-separated
vendor/modelformat (per env-config.schema.ts), developers setting this variable in production may benefit from a comment or inline example showing valid entries.Consider adding a comment above or inline documenting the expected format, or including a reference example such as the schema default list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/deploy-web/env/.env.sample` at line 53, The NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELS environment variable in the .env.sample file is blank without format guidance. Add a comment above this variable entry that documents the expected format as comma-separated vendor/model pairs (e.g., "nvidia/a100,amd/mi250"). Reference the schema validation rules from env-config.schema.ts to ensure the comment accurately reflects the required format, helping developers understand what valid entries should look like when configuring this variable in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/deploy-web/env/.env.sample`:
- Line 53: The environment variable
NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELS is positioned out of
lexicographic order according to dotenv-linter's baseline. Move this key entry
to appear before any keys starting with NEXT_PUBLIC_T (such as
NEXT_PUBLIC_TURNSTILE_ENABLED) to restore proper alphabetical ordering and align
with the static analysis tool's expectations for the .env.sample file.
- Line 53: The NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELS environment
variable in the .env.sample file is blank without format guidance. Add a comment
above this variable entry that documents the expected format as comma-separated
vendor/model pairs (e.g., "nvidia/a100,amd/mi250"). Reference the schema
validation rules from env-config.schema.ts to ensure the comment accurately
reflects the required format, helping developers understand what valid entries
should look like when configuring this variable in production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6968a00d-f602-4b62-b005-9037206aaadc
📒 Files selected for processing (1)
apps/deploy-web/env/.env.sample
2 participants
Why
nvidia/rtx5090m(mobile RTX 5090) was missing from the deploy-web trial blocked-GPU list. The frontend value is build-time only, so this sets it via the production env file to match the API policy (already updated via Doppler).What
Set
NEXT_PUBLIC_MANAGED_WALLET_TRIAL_BLOCKED_GPU_MODELSinapps/deploy-web/env/.env.productionso the trial GPU blocklist (now includingnvidia/rtx5090m) is inlined atnext build. Takes effect on the next deploy-web build/redeploy. The schema default inenv-config.schema.tsis left unchanged; the env var is the source of truth, mirroring how the API leans on Doppler. Backend enforcement is unchanged and remains authoritative.Summary by CodeRabbit
Release Notes