Summary
Follow-up to PR #81. PR #81 wired the lazy SKU catalogue lookup for Azure SQL Database, populating DatabaseDetails.EngineVersion from armsql.CapabilitiesClient.ListByLocation. Two other deferred fields on DatabaseDetails — AZConfig and Deployment — were intentionally left empty because they require a different SDK pattern (per-server lookups, not per-region capabilities).
Current behaviour
providers/azure/services/database/client.go::convertAzureSQLRecommendation (post-#81) populates:
Leaves empty:
AZConfig — Availability Zone configuration (zoneRedundant / zonePinned / none).
Deployment — server deployment shape (single server / managed instance / serverless).
Why deferred from #81
AZConfig is a per-server attribute (armsql.ServersClient.Get(rg, server).Properties.IsZoneRedundant-equivalent), not derivable from LocationCapabilities.
Deployment similarly comes from per-server armsql.Server.Kind / armsql.ManagedInstance resource type — different SDK calls per shape.
- A reservation recommendation does not name a specific server, so the converter cannot pick "the right" server to enrich from.
- Mirrors the cosmosdb cachedAPIType pattern ("dominant single-value across the subscription, else empty"): we could fetch all SQL servers in the subscription once and use the dominant zone-redundancy / deployment shape, but this is a different code path from the LocationCapabilities one and was out of scope for the perf change.
Proposed fix
Add a second cached lookup alongside the SKU catalogue:
cachedDominantAZConfig(ctx) — sync.Once-gated, hits armsql.ServersClient.NewListPager (subscription-wide), reduces IsZoneRedundant across all servers to a single dominant value or empty when ambiguous. Mirrors the cosmosdb cachedAPIType pattern.
cachedDominantDeployment(ctx) — same shape, derives from server Kind / managed-instance presence.
- Update
convertAzureSQLRecommendation to write both into Details when non-empty.
- Add three new tests per field:
_PopulatesAZConfig, _AmbiguousAZConfig, _AZConfigPagerErrorFallsBack (and the same trio for Deployment).
No new struct fields required — DatabaseDetails.AZConfig + Deployment already exist on pkg/common.
Test plan
- Single-server subscription with
IsZoneRedundant=true → Details.AZConfig = "zoneRedundant".
- Mixed subscription (one zoneRedundant, one zonePinned) →
AZConfig empty (ambiguous).
- Pager failure →
AZConfig empty, conversion still succeeds.
_FetchedOnce invariant pinned (one call per client lifetime regardless of converter call count).
- Same matrix for Deployment.
References
Severity
Low-Medium — UI degrades to "unknown" for two presentation-only fields. Functional but loses signal.
Effort
Small — ~80 LOC + ~6 unit tests, all in the database service package. Pattern is already established by #81.
Summary
Follow-up to PR #81. PR #81 wired the lazy SKU catalogue lookup for Azure SQL Database, populating
DatabaseDetails.EngineVersionfromarmsql.CapabilitiesClient.ListByLocation. Two other deferred fields onDatabaseDetails—AZConfigandDeployment— were intentionally left empty because they require a different SDK pattern (per-server lookups, not per-region capabilities).Current behaviour
providers/azure/services/database/client.go::convertAzureSQLRecommendation(post-#81) populates:Engine= "sqlserver"InstanceClass= SKU name from the recommendation payloadEngineVersion= from the cachedarmsql.LocationCapabilitiesmap (perf(azure): batched SKU catalogue lookup eliminates N+1 in recommendation converters #81)Leaves empty:
AZConfig— Availability Zone configuration (zoneRedundant / zonePinned / none).Deployment— server deployment shape (single server / managed instance / serverless).Why deferred from #81
AZConfigis a per-server attribute (armsql.ServersClient.Get(rg, server).Properties.IsZoneRedundant-equivalent), not derivable fromLocationCapabilities.Deploymentsimilarly comes from per-serverarmsql.Server.Kind/armsql.ManagedInstanceresource type — different SDK calls per shape.Proposed fix
Add a second cached lookup alongside the SKU catalogue:
cachedDominantAZConfig(ctx)—sync.Once-gated, hitsarmsql.ServersClient.NewListPager(subscription-wide), reducesIsZoneRedundantacross all servers to a single dominant value or empty when ambiguous. Mirrors the cosmosdbcachedAPITypepattern.cachedDominantDeployment(ctx)— same shape, derives from serverKind/ managed-instance presence.convertAzureSQLRecommendationto write both intoDetailswhen non-empty._PopulatesAZConfig,_AmbiguousAZConfig,_AZConfigPagerErrorFallsBack(and the same trio for Deployment).No new struct fields required —
DatabaseDetails.AZConfig+Deploymentalready exist onpkg/common.Test plan
IsZoneRedundant=true→Details.AZConfig="zoneRedundant".AZConfigempty (ambiguous).AZConfigempty, conversion still succeeds._FetchedOnceinvariant pinned (one call per client lifetime regardless of converter call count).References
providers/azure/services/database/client.go::convertAzureSQLRecommendationproviders/azure/services/database/client_test.goproviders/azure/services/cosmosdb/client.go::cachedAPITypeSeverity
Low-Medium — UI degrades to "unknown" for two presentation-only fields. Functional but loses signal.
Effort
Small — ~80 LOC + ~6 unit tests, all in the database service package. Pattern is already established by #81.