Floating number from config gets misread - i.e. 1.5 => 15

Skaanning commented 7 months ago

What happened?

I try to read a double from my config file using the new Config("...").RequireDouble("...") method. But the number i get ignores the decimal completely and so 1.5 becomes 15.

Example

in my config file i set

workspace:dailyQuotaGb: 1.5

But when i read from it i get 15..


var config = new Config("workspace");
var workspaceDailyQuotaGb = config.RequireDouble("dailyQuotaGb");
var workspace = new Pulumi.AzureNative.OperationalInsights.Workspace($"loganalytics-workspace",
new WorkspaceArgs
{
                WorkspaceName = $"loganalytics-workspace-{stackName}",
                ResourceGroupName = resourceGroup.Name,
                Sku = new WorkspaceSkuArgs { Name = WorkspaceSkuNameEnum.PerGB2018, },
                WorkspaceCapping = new WorkspaceCappingArgs{DailyQuotaGb = workspaceDailyQuotaGb }
});

results in this when i run it

It makes no difference if i write workspace:dailyQuotaGb: "1.5" with quotes in the config file.

Output of `pulumi about`

Backend Name pulumi.com URL https://app.pulumi.com/Skaanning User Skaanning Organizations Skaanning, dol-sensors Token type personal

Dependencies: NAME VERSION Pulumi 3.59.0 Pulumi.AzureNative 2.27.0 Pulumi.Random 4.15.1

Pulumi locates its logs in C:\Users\mic\AppData\Local\Temp by default

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

justinvp commented 7 months ago

Hi @Skaanning. Sorry for the trouble.

I tried reproducing this in a minimal program, and couldn't reproduce the issue:

mkdir dotnetconfig && cd dotnetconfig
pulumi new csharp
pulumi config set workspace:dailyQuotaGb 1.5

using System;
using System.Collections.Generic;
using Pulumi;

return await Deployment.RunAsync(() =>
{
    var config = new Config("workspace");
    var workspaceDailyQuotaGb = config.RequireDouble("dailyQuotaGb");
    Console.WriteLine(workspaceDailyQuotaGb);

    return new Dictionary<string, object?>
    {
        ["dialyQuotaGb"] = workspaceDailyQuotaGb
    };
});

Running pulumi preview yields:

$ pulumi preview
Previewing update (dev)

View in Browser (Ctrl+O): https://app.pulumi.com/...

     Type                 Name              Plan       Info
 +   pulumi:pulumi:Stack  dotnetconfig-dev  create     1 message

Diagnostics:
  pulumi:pulumi:Stack (dotnetconfig-dev):
    1.5

Outputs:
    dialyQuotaGb: 1.5

Resources:
    + 1 to create

I also tried an minimal Azure Native program, and similarly could not repro the issue.

using System;
using System.Collections.Generic;
using Pulumi;
using Pulumi.AzureNative.Resources;
using Pulumi.AzureNative.OperationalInsights;
using Pulumi.AzureNative.OperationalInsights.Inputs;

return await Pulumi.Deployment.RunAsync(() =>
{
    var resourceGroup = new ResourceGroup("resourceGroup");

    var stackName = Pulumi.Deployment.Instance.StackName;

    var config = new Config("workspace");
    var workspaceDailyQuotaGb = config.RequireDouble("dailyQuotaGb");
    Console.WriteLine($"Workspace daily quota: {workspaceDailyQuotaGb}");
    var workspace = new Pulumi.AzureNative.OperationalInsights.Workspace($"loganalytics-workspace",
    new WorkspaceArgs
    {
                    WorkspaceName = $"loganalytics-workspace-{stackName}",
                    ResourceGroupName = resourceGroup.Name,
                    Sku = new WorkspaceSkuArgs { Name = WorkspaceSkuNameEnum.PerGB2018, },
                    WorkspaceCapping = new WorkspaceCappingArgs{DailyQuotaGb = workspaceDailyQuotaGb }
    });

    return new Dictionary<string, object?>
    {
    };
});

This creates the Workspace with the 1.5 value and prints out Workspace daily quota: 1.5.

What version of the Pulumi CLI are you using?

Can you add a Console.WriteLine of the value read from config to see what it is?

    var config = new Config("workspace");
    var workspaceDailyQuotaGb = config.RequireDouble("dailyQuotaGb");
    Console.WriteLine($"Workspace daily quota: {workspaceDailyQuotaGb}");

Skaanning commented 7 months ago

Aha I think I know what's happening.

My work PC has Danish set as a default language - meaning decimal numbers are written with "," and not "." On a pc now that uses English, the RequireDouble works as expected.

I guess my expectation would be that CultureInfo.InvariantCulture is used inside the parsing methods https://github.com/pulumi/pulumi-dotnet/blob/6382a8cbfe11ff21466a0e950f8523e06a94f295/sdk/Pulumi/Config.cs#L136

justinvp commented 7 months ago

Ah, makes sense. We likely should be using InvariantCulture here. Slight concern over the behavior change, though, which could potentially break someone relying on the culture-specific behavior.

@Frassle, @Zaid-Ajaj, @mikhailshilkov, any thoughts on fixing this to use InvariantCulture? Or should we keep the existing APIs as-is and add new invariant APIs to avoid the behavior change, e.g. RequireDoubleInvariant?

Frassle commented 7 months ago

It's parsing from JSON data so it should be using invariant culture.

pulumi / pulumi-dotnet