Distinguish Orleans hosts & clients, parameterize CluserId/ServiceId, improve playground app #3433
Conversation
…usterId & ServiceId, improve playground app
dotnet-issue-labeler
bot
added
the
area-integrations
| { | ||
| Name = name; | ||
| Builder = builder; | ||
| ServiceId = ParameterResourceBuilderExtensions.CreateGeneratedParameter(builder, $"{name}-service-id", secret: false, new GenerateParameterDefault |
There was a problem hiding this comment.
@eerhardt this API is kinda crazy 😄
There was a problem hiding this comment.
Suggestions on making it better?
|
|
||
| /// <summary> | ||
| /// Gets or sets the service identifier. | ||
| /// </summary> | ||
| public string? ServiceId { get; set; } | ||
| internal object ServiceId { get; set; } |
There was a problem hiding this comment.
Why object?
| internal object ServiceId { get; set; } | |
| internal ParameterResource ServiceId { get; set; } |
There was a problem hiding this comment.
It's either a parameter or a hard-coded string value.. I made it internal so at least this detail doesn't leak.
See WithServiceId and WithClusterId
There was a problem hiding this comment.
It's either a parameter or a hard-coded string value.
Might be good sticking that comment in here.
Co-authored-by: Eric Erhardt <[email protected]>
| /// <param name="isProxied">Specifies if the endpoint will be proxied by DCP. Defaults to true.</param> | ||
| /// <returns>The <see cref="IResourceBuilder{T}"/>.</returns> | ||
| /// <exception cref="DistributedApplicationException">Throws an exception if an endpoint with the same name already exists on the specified resource.</exception> | ||
| public static IResourceBuilder<T> WithEndpoint<T>(this IResourceBuilder<T> builder, int? port = null, int? targetPort = null, string? scheme = null, string? name = null, string? env = null, bool isProxied = true) where T : IResource | ||
| public static IResourceBuilder<T> WithEndpoint<T>(this IResourceBuilder<T> builder, int? port = null, int? targetPort = null, string? scheme = null, string? name = null, string? env = null, bool isProxied = true, bool? isExternal = null) where T : IResource |
There was a problem hiding this comment.
Just remember we can't make changes like this after we ship stable.
It might make sense to get API compat turned on after 8.0 ships.
| /// <param name="orleansServiceClient">The Orleans service client, containing clustering, etc.</param> | ||
| /// <returns>The resource builder.</returns> | ||
| /// <exception cref="InvalidOperationException">Clustering has not been configured.</exception> | ||
| public static IResourceBuilder<T> WithReference<T>( |
There was a problem hiding this comment.
When would I use WithReference(OrleansServiceClient) over using WithReference(OrleansService)?
Is it confusing to have both? What's the difference between:
builder.AddProject<Projects.OrleansServer>("silo")
.WithReference(orleans);and
builder.AddProject<Projects.OrleansClient>("frontend")
.WithReference(orleans.AsClient());?
There was a problem hiding this comment.
You use the service one for projects with an Orleans server and the client one with the Orleans clients. Their configuration follows the same format, but servers have more (storage, etc)
Co-authored-by: Eric Erhardt <[email protected]>
| // NOTE: These endpoints are specified as proxied even though we never expect any connections via the proxy, and that would be invalid anyway. | ||
| // this is a workaround for current Aspire/DCP behavior. | ||
| builder.WithEndpoint(scheme: "tcp", name: "orleans-silo", env: "Orleans__Endpoints__SiloPort", isProxied: true, isExternal: false); | ||
| builder.WithEndpoint(scheme: "tcp", name: "orleans-gateway", env: "Orleans__Endpoints__GatewayPort", isProxied: true, isExternal: false); |
There was a problem hiding this comment.
I'm a bit confused by this comment.
- Isn't "isProxied" defaulted to
true? - What exactly is this working around?
There was a problem hiding this comment.
IsProxied is defaulted to true. In this case, we don't actually want a proxy, but we need to make sure IsProxied is set to true, because otherwise DCP will barf. It's a limitation. The proxy ends up being unused by the app (they connect directly to each other), so it's benign but worth mentioning in a comment and being explicit about the current behavior.
|
/azp backport to release/8.0 |
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/aspire/actions/runs/8610492653 |
Fixes #3399
Microsoft Reviewers: Open in CodeFlow