Skip to content

Commit 4592311

Browse files
Roland Krummenacherclaude
andcommitted
fix(open-data): parse and write ISF ratios with the invariant culture
Ratios were parsed and written with the current culture. On a comma-decimal machine that corrupts the dataset in both directions: - de-DE parses "2.1" as 21, so a ratio is silently ten times too large. - fr-CH fails the parse outright, so the row is dropped. - Export-Csv writes the value back as "2,1", which breaks every consumer: the Optimization Engine's externaldata(... Ratio:double) and the Power BI partitions typed as number. CI runs on ubuntu-latest and is unaffected, but the README documents running the generator by hand, so a contributor in a comma-decimal locale would publish a corrupt file. Both parse sites and the write path now pin InvariantCulture, with a regression test that drives the generator under de-DE and asserts the round trip. Also corrects the SKU count in the changelog: it still said 2,574 from before the zero-ratio row was dropped, against 2,573 actually published. Reported by Copilot review on #2308 (the parse side; the write side turned up while verifying it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 77949cd commit 4592311

3 files changed

Lines changed: 37 additions & 5 deletions

File tree

docs-mslearn/toolkit/changelog.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ The following section lists features and enhancements that are currently in deve
7979
- **Added**
8080
- Added a new [Instance size flexibility](open-data.md#instance-size-flexibility) dataset that maps each ARM SKU to its instance size flexibility group and ratio, sourced from the Azure Reservations Catalogs API. It replaces the deprecated ISF ratio files hosted on `ccmstorageprod.blob.core.windows.net` ([#2090](https://github.com/microsoft/finops-toolkit/issues/2090)).
8181
- **Fixed**
82-
- Fixed the instance size flexibility dataset omitting SKUs that are no longer available to purchase. The Catalogs API only returns purchasable SKUs, so retired series that reservations still cover (Av2, D, DS, Dv2, Dv3, Ev3, F, G, H, LS, NC, NV, and others) were missing. The dataset is now additive and backfilled from the retired ratio file, growing from 211 to 318 flexibility groups and from 1,353 to 2,574 SKUs ([#2300](https://github.com/microsoft/finops-toolkit/issues/2300)).
82+
- Fixed the instance size flexibility dataset omitting SKUs that are no longer available to purchase. The Catalogs API only returns purchasable SKUs, so retired series that reservations still cover (Av2, D, DS, Dv2, Dv3, Ev3, F, G, H, LS, NC, NV, and others) were missing. The dataset is now additive and backfilled from the retired ratio file, growing from 211 to 318 flexibility groups and from 1,353 to 2,573 SKUs ([#2300](https://github.com/microsoft/finops-toolkit/issues/2300)).
8383
- Fixed the generator sweeping a hardcoded list of 26 regions, which silently omitted SKUs that launch only in regions outside the list. It now enumerates every physical region from the ARM locations API, 63 at the time of the change ([#2300](https://github.com/microsoft/finops-toolkit/issues/2300)).
8484
- Fixed the dataset publishing a SKU whose ratio is `0`, which the Optimization Engine's benefits simulation divides by. Non-positive ratios are now dropped ([#2300](https://github.com/microsoft/finops-toolkit/issues/2300)).
8585
- Fixed ratios from the retired files sitting on a different scale than the Catalogs API. The retired files normalized each group so its smallest SKU was `1`; the API returns them unnormalized, so a group's smallest SKU usually isn't `1`. Because Azure retires individual sizes rather than whole families, the API covers only part of many groups, so combining the two left single groups holding both units. The backfilled rows were converted onto the API scale, and the generator now fails rather than publishing a group in two units ([#2300](https://github.com/microsoft/finops-toolkit/issues/2300)).

src/powershell/Tests/Unit/Update-InstanceSizeFlexibility.Tests.ps1

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,32 @@ Describe 'Update-InstanceSizeFlexibility' {
167167
@(Import-Csv $outFile).ArmSkuName | Should -Be 'Standard_D2'
168168
}
169169

170+
It 'Parses and writes ratios independently of the current culture' {
171+
# On a comma-decimal culture, culture-aware parsing turns "2.1" into 21 (de-DE) or
172+
# drops the row outright (fr-CH), and Export-Csv writes "2,1" back out, which breaks
173+
# every consumer of the published file.
174+
$originalCulture = [System.Threading.Thread]::CurrentThread.CurrentCulture
175+
try
176+
{
177+
[System.Threading.Thread]::CurrentThread.CurrentCulture = [System.Globalization.CultureInfo]::new('de-DE')
178+
@([PSCustomObject]@{ InstanceSizeFlexibilityGroup = 'Av2 Series'; ArmSkuName = 'Standard_A2_v2'; Ratio = '2.1' }) `
179+
| Export-Csv $outFile -NoTypeInformation
180+
Mock Invoke-AzRestMethod {
181+
New-CatalogResponse -Items @((New-CatalogItem -Sku 'Standard_D2' -Group 'DSeries' -Ratio '4.5'))
182+
}
183+
184+
Invoke-Generator $baseParams
185+
186+
$raw = Get-Content $outFile -Raw
187+
$raw | Should -BeLike '*"Standard_A2_v2","2.1"*' # carried forward, not 21
188+
$raw | Should -BeLike '*"Standard_D2","4.5"*' # from the API, not 45
189+
}
190+
finally
191+
{
192+
[System.Threading.Thread]::CurrentThread.CurrentCulture = $originalCulture
193+
}
194+
}
195+
170196
It 'Skips items without ISF group or ratio properties' {
171197
Mock Invoke-AzRestMethod {
172198
New-CatalogResponse -Items @(

src/scripts/Update-InstanceSizeFlexibility.ps1

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,8 @@ function Get-IsfRecords
268268
# Optimization Engine's benefits simulation does exactly that -- so drop it rather than
269269
# publish a division by zero. The API returns one today (Azure Redis Cache Isolated).
270270
$parsedRatio = 0.0
271-
if ($flexGroup -and $armSkuName -and [double]::TryParse($ratio, [ref]$parsedRatio) -and $parsedRatio -gt 0)
271+
$parsed = [double]::TryParse($ratio, [Globalization.NumberStyles]::Float, [Globalization.CultureInfo]::InvariantCulture, [ref]$parsedRatio)
272+
if ($flexGroup -and $armSkuName -and $parsed -and $parsedRatio -gt 0)
272273
{
273274
$null = $records.Add([PSCustomObject]@{
274275
InstanceSizeFlexibilityGroup = [string]$flexGroup
@@ -333,7 +334,7 @@ function Import-IsfCsv
333334
foreach ($row in $rows)
334335
{
335336
$ratio = 0.0
336-
if (-not [double]::TryParse($row.Ratio, [ref]$ratio)) { continue }
337+
if (-not [double]::TryParse($row.Ratio, [Globalization.NumberStyles]::Float, [Globalization.CultureInfo]::InvariantCulture, [ref]$ratio)) { continue }
337338
if ($ratio -le 0) { continue }
338339
if (-not $groupsPerSku.ContainsKey($row.ArmSkuName)) { $groupsPerSku[$row.ArmSkuName] = @{} }
339340
$groupsPerSku[$row.ArmSkuName][$row.InstanceSizeFlexibilityGroup] = $true
@@ -342,7 +343,7 @@ function Import-IsfCsv
342343
foreach ($row in $rows)
343344
{
344345
$ratio = 0.0
345-
if (-not [double]::TryParse($row.Ratio, [ref]$ratio)) { continue }
346+
if (-not [double]::TryParse($row.Ratio, [Globalization.NumberStyles]::Float, [Globalization.CultureInfo]::InvariantCulture, [ref]$ratio)) { continue }
346347
if ($ratio -le 0) { continue }
347348
if ($groupsPerSku[$row.ArmSkuName].Count -gt 1) { continue }
348349
# Same placeholder filter the API path applies, so a retired file's placeholder rows
@@ -513,7 +514,12 @@ if ($Normalize)
513514
$allRecords = Get-NormalizedRecords -Records $allRecords
514515
}
515516

516-
$rows = $allRecords | Sort-Object InstanceSizeFlexibilityGroup, ArmSkuName
517+
# Ratio is formatted invariantly rather than left to Export-Csv, which uses the current culture:
518+
# on a comma-decimal machine it would publish "2,1" and break every consumer of the file.
519+
$rows = $allRecords |
520+
Sort-Object InstanceSizeFlexibilityGroup, ArmSkuName |
521+
Select-Object InstanceSizeFlexibilityGroup, ArmSkuName,
522+
@{ Name = 'Ratio'; Expression = { $_.Ratio.ToString([Globalization.CultureInfo]::InvariantCulture) } }
517523

518524
$rows | Export-Csv -Path $OutputPath -UseQuotes Always -NoTypeInformation -Encoding utf8
519525
Write-Host "Wrote $($rows.Count) SKUs to $OutputPath"

0 commit comments

Comments
 (0)