Resolves #46 Support authoring multi-language components in .NET #275
Conversation
algompluecker
force-pushed
the
martinp/issue46-support-multi-lanugage-components
branch
from
63407a1
to
485382f
Compare
algompluecker
changed the title
algompluecker
changed the title
|
I have done some refactoring in order to prepare integration tests in another PR: PR 277 |
|
I have implemented integration tests now. Please have a look. |
…rt-multi-lanugage-components # Conflicts: # sdk/Pulumi.Tests/Provider/ProviderServerTestHost.cs
…rt-multi-lanugage-components # Conflicts: # sdk/Pulumi.Tests/Resources/CustomTimeoutsTests.cs # sdk/Pulumi/Resources/CustomTimeouts.cs
|
Hi, I have finalized this PR. Integration tests have been added and are passing. @Frassle @Zaid-Ajaj can you please do a final review? I can't wait to see this being shipped :) |
Noticed while reviewing #275. We called this helper `Marshal` but it's actually `Unmarshalling` a PropertyValue from it's wire encoding.
| @@ -10,7 +10,7 @@ internal interface IDeploymentInternal : IDeployment | |||
| string? GetConfig(string fullKey); | |||
| bool IsConfigSecret(string fullKey); | |||
|
|
|||
| Stack Stack { get; set; } | |||
| Stack? Stack { get; set; } | |||
There was a problem hiding this comment.
Really? Shouldn't we always have a stack
There was a problem hiding this comment.
Not with MLCs, I remember having to do the same when I built the PoC
There was a problem hiding this comment.
I'm pretty sure this will lead to issue where Parent isn't explicitly set. That's something I've tried to and failed so far to fix in the engine so we need SDKs to be correct here.
|
|
||
| namespace Pulumi.Experimental.Provider; | ||
|
|
||
| public class ComponentResourceProviderBase : Provider |
There was a problem hiding this comment.
I think this could just get merged into Provider. There are cases of providers that support Custom and Component resources, there's no need to make components require a different base class for that.
There was a problem hiding this comment.
These serializer changes look like
A) They could go to their own PR
B) Should have tests in sdk/Pulumi.Tests/Provider/PropertyValueTests.cs to match
| @@ -0,0 +1,7 @@ | |||
| namespace Pulumi; | |||
|
|
|||
| internal interface IDeploymentBuilder | |||
There was a problem hiding this comment.
I think its worth pulling this IDeploymentBuilder change into it's own PR and seeing if it makes sense to optionally expose it to non-service tests as well.
…rt-multi-lanugage-components # Conflicts: # sdk/Pulumi/Provider/Provider.cs
Idea taken from #275, where there were even more cases of translating CheckFailure objects. But this is a small clean change to pull in by itself.
…-support-multi-lanugage-components # Conflicts: # sdk/Pulumi/Provider/PropertyValue.cs # sdk/Pulumi/Provider/PropertyValueSerializer.cs # sdk/Pulumi/Provider/Provider.cs
|
Hi @Frassle @Zaid-Ajaj I have resolved all the points as discussed in the meetings. Please double check and resolve the remaining open discussions if you agree they are solved. |
|
I think the test failures are real issues due to the changes in RunInlineAsync. It's not handling exceptions exactly the same as before. |
|
It might be worth trying to do the change to generic in it's own smaller PR. |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
|
This PR has been shipped in release v3.68.0. |
No description provided.