Dapr component schema #460
Conversation
FullStackChef
requested review from
aaronpowell
and removed request for
WhitWaldo and
paule96
src/CommunityToolkit.Aspire.Hosting.Dapr/DaprDistributedApplicationLifecycleHook.cs
Outdated
Show resolved
Hide resolved
| return await contentWriter(content).ConfigureAwait(false); | ||
| } | ||
|
|
||
| private string GetAppHostRelativePath(string componentName) |
There was a problem hiding this comment.
Aside, we're creating a place to store things like this in 9.1, the aspire store.
There was a problem hiding this comment.
nice -Apphost relative? wanna point me to the code and I could update inline with that?
examples/dapr/CommunityToolkit.Aspire.Hosting.Dapr.AppHost/Program.cs
Outdated
Show resolved
Hide resolved
src/CommunityToolkit.Aspire.Hosting.Dapr/DaprComponentSecretAnnotation.cs
Show resolved
Hide resolved
src/CommunityToolkit.Aspire.Hosting.Dapr/DaprComponentSchema.cs
Outdated
Show resolved
Hide resolved
…notation.cs Co-authored-by: David Fowler <[email protected]>
…hef/CommunityToolkit-Aspire into dapr-component-schema
…gram.cs Co-authored-by: David Fowler <[email protected]>
…hef/CommunityToolkit-Aspire into dapr-component-schema
src/CommunityToolkit.Aspire.Hosting.Dapr/DaprComponentSchema.cs
Outdated
Show resolved
Hide resolved
|
Maybe if it is not to big of a no scope of this PR we should already start deviding dapr sidecar and component lifecycles to make our lifes easier later. So that we would have two LifecycleHooks and that everytime you have something like With this we will have an easier life if we need to consider the endpoints to generate the correct YAML files |
|
@paule96 my preference would be not to increase the scope of the my PR and I think the concept of separating lifecycle hooks needs further thought /design |
There was a problem hiding this comment.
Do we need this file still? I don't see the reference of it in the sample
| // If any of the components have secrets, we will add an on-demand secret store component. | ||
| if (onDemandComponents.Any(component => component.TryGetAnnotationsOfType<DaprComponentSecretAnnotation>(out var annotations) && annotations.Any())) | ||
| { | ||
| onDemandComponents.Add(new DaprComponentResource("secretstore", DaprConstants.BuildingBlocks.SecretStore)); |
There was a problem hiding this comment.
is this really a good idea? I mean a secret store can / should have configuration.
To just generate a generic one sounds a bit confusing to me.
Maybe we should better throw an exception here, that the user forgot something to define.
If we find secrets but no secret store
| string componentPath = await (component.Type switch | ||
| { | ||
| DaprConstants.BuildingBlocks.PubSub => GetPubSubAsync(component, contentWriter, cancellationToken), | ||
| DaprConstants.BuildingBlocks.StateStore => GetStateStoreAsync(component, contentWriter, cancellationToken), | ||
| _ => throw new InvalidOperationException($"Unsupported Dapr component type '{component.Type}'.") | ||
| DaprConstants.BuildingBlocks.PubSub => GetBuildingBlockComponentAsync(component, contentWriter, "pubsub.in-memory", cancellationToken), // NOTE: In memory component can only be used within a single Dapr application. | ||
| DaprConstants.BuildingBlocks.StateStore => GetBuildingBlockComponentAsync(component, contentWriter, "state.in-memory", cancellationToken), | ||
| DaprConstants.BuildingBlocks.SecretStore => GetBuildingBlockComponentAsync(component, contentWriter, "secretstores.local.env", cancellationToken), | ||
| _ => GetComponentAsync(component, contentWriter, cancellationToken) | ||
| }).ConfigureAwait(false); |
There was a problem hiding this comment.
Why their is still a need for this switch / case?
Shouldn't we handle everything with the last line? (GetComponentAsync(component, contentWriter, cancellationToken))
| .WithReference(pubSub) | ||
| .WithDaprSidecar() |
There was a problem hiding this comment.
From a developer standpoint I would more expect something like:
.WithDaprSidecar()
.WithReference(pubSub)Because you configure the sidecare to use the pubSub reference as a pubsub. Not the project.
That would also mean that we would need to break with the Fluent API their or have some WithDaprSidecar parameter that accepts a action or so to configure the sidecar.
You can look for the AzurePostgreSql resource of aspire how you can configure the development contianer. I would prefer an API like their
There was a problem hiding this comment.
To prevent breaking changes for now closing this comment.
|
/stale-extend |
Closes #459
PR Checklist
Other information