feat(common): add ComputeBroker variant to CloudProvider enum#439
feat(common): add ComputeBroker variant to CloudProvider enum#439epappas wants to merge 1 commit into
Conversation
Prepares CLI/SDK for the upcoming basilica-compute-broker service (basilica-backend #326). Without this variant, any GpuOffering with provider="compute-broker" returned from /secure-cloud/gpu-prices fails to deserialize on every Rust SDK consumer — and because GpuOffering.provider is a strongly-typed enum, the entire Vec fails, not just the broker rows. basilica-sdk re-exports GpuOffering directly from basilica-common (crates/basilica-sdk/src/types.rs:541), so the fix lives entirely in this crate. The wire form is "compute-broker" (with hyphen) per the architecture doc's offering-id format compute-broker-<backend>-<product_id>-<region>. The default #[serde(rename_all = "lowercase")] on CloudProvider would produce "computebroker"; we override with #[serde(rename = "compute-broker")] on the variant. FromStr accepts case-insensitive aliases: compute-broker, compute_broker, computebroker, ComputeBroker. Tests cover the enum-string roundtrip + GpuOffering deserialization with the offering-id format the broker will actually emit (compute-broker-contabo-V92-EU). Matches the precedent set when Denvr was added (20d3b98), Shadeform before that (738f6e2), and MassCompute (683ba3f). Refs: basilica-backend#326
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Summary
Prerequisite for the upcoming
basilica-compute-brokerservice (basilica-backend #326). AddsCloudProvider::ComputeBrokerso the SDK can deserializeGpuOfferingrows withprovider=\"compute-broker\"once the backend ships.Single file change (+62 lines) following the precedent set by Denvr (
20d3b983), Shadeform (738f6e27), and MassCompute (683ba3fe).Why this PR ships before any backend change
basilica-sdkre-exportsGpuOfferingdirectly frombasilica-common(crates/basilica-sdk/src/types.rs:541).GpuOffering.provideris the strongly-typedCloudProviderenum. If a backend deployment shippedprovider=\"compute-broker\"on the wire before every CLI/SDK had this variant, the entireVec<GpuOffering>response from/secure-cloud/gpu-priceswould fail to deserialize on the client side — not just the broker rows.This is the same regression mode documented in the Shadeform onboarding architecture doc (basilica-backend
docs/architecture/SECURE-CLOUD-PROVIDER-ONBOARDING-ARCHITECTURE.md§6) and the reason Phase P1 of #326 must merge and release before Phase P3 ships.Wire form
The architecture doc's offering ID format is
compute-broker-<backend>-<product_id>-<region>(e.g.compute-broker-contabo-V92-EU), so the wire form is\"compute-broker\"with a hyphen.The default
#[serde(rename_all = \"lowercase\")]onCloudProviderwould have produced\"computebroker\"; this PR overrides with#[serde(rename = \"compute-broker\")]on the variant.FromStraccepts case-insensitive aliases:compute-broker,compute_broker,computebroker,ComputeBroker.Tests
All in
crates/basilica-common/src/types.rs::cloud_provider_tests(12 tests pass; clippy clean):test_cloud_provider_as_str— extended withcompute-brokertest_cloud_provider_from_str_compute_broker— new, covers all 5 alias spellingstest_cloud_provider_serde_all_variants— extended with the new varianttest_gpu_offering_with_compute_broker_provider— new, deserializes aGpuOfferingwhoseidmatches the offering-id format the broker will emit (compute-broker-contabo-V92-EU); this is the exact regression probe for the SDK-break-on-unknown-provider classVerification
```
cargo test -p basilica-common --lib types::cloud_provider
12 passed; 0 failed; 0 ignored
cargo clippy -p basilica-common --all-targets -- -D warnings
clean
cargo build -p basilica-sdk -p basilica-cli
clean
```
What this PR does not do
basilica-cli/basilica-sdk/basilica-sdk-pythonversions — that's a follow-up release PR (P1b in feat(hyperstack): update rate limit defaults for production usage #326's plan), opened after this merges, that publishes the new variant to PyPI / crates sobasilica-backendcan then ship the broker without breaking existing clients.Merge note
Per project convention (
feedback_never_squash), this should be merged with--merge, not--squash.References
20d3b983, Shadeform738f6e27, MassCompute683ba3feTest plan
cargo test -p basilica-common --lib types::cloud_provider— 12 / 12 passcargo clippy -p basilica-common --all-targets -- -D warnings— cleancargo build -p basilica-sdk -p basilica-cli— clean (downstream consumers compile)test_gpu_offering_with_compute_broker_providerdeserializes the exact offering-id format the broker will emitSummary by CodeRabbit
New Features
Tests