pulumi / pulumi-dotnet

.NET support for Pulumi
Apache License 2.0
28 stars 25 forks source link

[sdk] Delegates alias computation to engine #80

Closed Zaid-Ajaj closed 1 year ago

Zaid-Ajaj commented 1 year ago

Fixes #14

Removes AllAliases function and instead constructs aList<Pulumirpc.Alias> in PrepareAliases (linear time) which populates the RegisterResourceRequest accordingly:

if (prepareResult.SupportsAliasSpec) 
{
    request.Aliases.AddRange(prepareResult.Aliases);
}
else 
{
    var aliasUrns = prepareResult.Aliases.Select(a => a.Urn);
    request.AliasURNs.AddRange(aliasUrns);
}

Wasn't exactly sure how to test this other than constructing a lot (=1000) of aliases in AliasesMocks and observe that it doesn't just hang for a while.

All integration tests are succeeding locally except for TestProvider but that seems unrelated. Let's see what CI has to say about it.

Frassle commented 1 year ago

Run dotnet format :) Also we should split that to it's own job so the unit tests can still run and give signal even if formatting is wrong.

Zaid-Ajaj commented 1 year ago

@Frassle

Frassle commented 1 year ago

Using dotnet run format-sdk changes every file I have, removing a whitespace at the end 😓

Odd? Shouldn't this be consistent across machines?

Integration test TestProvider is failing on the destroy step.

OK leave the test off the test list for now, I'll get that fixed.

Zaid-Ajaj commented 1 year ago

Odd? Shouldn't this be consistent across machines?

@Frassle I've added a minimal .editorconfig which is now formatting the same locally and on CI

OK leave the test off the test list for now, I'll get that fixed.

Removed that test the time being

I'm pretty sure the engine is happy to take an Alias object with type or name as "".

Alright, then using empty string as default, see last commit:

var aliasSpec = new Pulumirpc.Alias.Types.Spec
{
    Name = await Resolve(resolvedAlias.Name, ""),
    Type = await Resolve(resolvedAlias.Type, ""),
    Stack = await Resolve(resolvedAlias.Stack, Instance.StackName),
    Project = await Resolve(resolvedAlias.Project, Instance.ProjectName),
};
justinvp commented 1 year ago

Change file encoding utf-8 to utf-8-bom to reduce diffs

Ugh, I'd prefer we get rid of the bom from all files and just use utf-8.

But let's do what Fraser suggested: format the codebase in a separate PR, then rebase this PR on top of that.