[Automated] Update API Surface Area#17700
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17700Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17700" |
joperezr
left a comment
There was a problem hiding this comment.
API review for the 13.4 surface — questions / suggestions inline. Findings on APIs already marked [Experimental] were de-prioritized.
|
|
||
| [AspireExport] | ||
| public static ApplicationModel.IResourceBuilder<T> WithEndpoint<T>(this ApplicationModel.IResourceBuilder<T> builder, int? port = null, int? targetPort = null, string? scheme = null, string? name = null, string? env = null, bool? isProxied = null, bool? isExternal = null, System.Net.Sockets.ProtocolType? protocol = null) | ||
| where T : ApplicationModel.IResourceWithEndpoints { throw null; } |
There was a problem hiding this comment.
The default for isProxied changed from true to null. Is that intentional? A binary-compat shim is added, but source callers relying on the implicit true will silently get the new default — worth a release-note callout if the runtime semantics differ.
There was a problem hiding this comment.
Yeah, we intentionally kept binary compatibility (with the old default) if you built against an old Aspire.Hosting, while changing the defaults going forward when building against 13.4 and later. Seemed like the best trade-off.
There was a problem hiding this comment.
Not addressed in the fix PR — flagging for the owner to confirm whether the default change to null is intentional.
|
|
||
| [AspireExport] | ||
| public static ApplicationModel.IResourceBuilder<T> WithHttpEndpoint<T>(this ApplicationModel.IResourceBuilder<T> builder, int? port = null, int? targetPort = null, string? name = null, string? env = null, bool? isProxied = null) | ||
| where T : ApplicationModel.IResourceWithEndpoints { throw null; } |
There was a problem hiding this comment.
Same isProxied default change here (true → null) — intentional?
There was a problem hiding this comment.
Yeah, it's so that going forward we can differentiate between explicitly true and default. Allows us to disable proxies for persistent resources by default (assuming the integration is built against Aspire.Hosting 13.4).
There was a problem hiding this comment.
Same as above — not addressed in the fix PR; owner should confirm intent.
| } | ||
|
|
||
| public enum GatewayPathMatchType | ||
| { |
There was a problem hiding this comment.
Heads-up: GatewayPathMatchType.PathPrefix vs IngressPathType.Prefix (line ~283 below) name the same concept differently. Worth aligning unless we're deliberately mirroring the K8s Gateway / Ingress spec naming.
There was a problem hiding this comment.
Not addressed in the fix PR — naming consistency question deferred to the owner.
38f05e4 to
d837395
Compare
* API review fixes for 13.4 (PR #17700) Addresses several issues found during API surface review: 1. Rename NetworkID -> NetworkId (and networkID -> networkId) on AllocatedEndpoint, EndpointAnnotation, EndpointReference, EndpointReferenceAnnotation, NetworkEndpointSnapshot, NetworkEndpointSnapshotList, and related methods/parameters. 2. Add [Experimental("ASPIREAZURE003")] to AzureRoleAssignmentResource. 3. Change EndpointReferenceAnnotation.EndpointNames from HashSet<string> to ISet<string> (backing field stays HashSet). 4. Add 'sealed' to new public resource classes that are not subclassed in the repo: KubernetesHelmChartResource, BlazorWasmAppResource, BunAppResource, NextJsAppResource, ViteAppResource, AzureNatGatewayResource, AzureNetworkSecurityGroupResource, AzureNetworkSecurityPerimeterResource, AzurePrivateEndpointResource, AzurePublicIPAddressResource, AzureSubnetResource, AzureVirtualNetworkResource. (GoAppResource and NodeAppResource left non-sealed because they are used as generic type constraints in the same assembly.) 5. Disambiguate WithHiddenOnCompletion overloads by removing the '= 0' default from the int overload, so calls with no argument resolve to the params overload. api/*.cs and api/*.ats.txt are intentionally not updated here - the API surface PR will regenerate them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert breaking API changes; bump baseline to 13.3.5 Reverts the subset of API changes from the previous commit that would be binary-breaking against 13.3.5, and bumps PackageValidationBaselineVersion from 13.2.2 to 13.3.5 so pack validation runs against the latest shipped release. Reverts (binary-breaking against 13.3.5): - AllocatedEndpoint.NetworkID (kept ctor param 'networkId' - not breaking) - EndpointAnnotation.DefaultNetworkID (kept ctor param 'networkId') - EndpointReference.ContextNetworkID (kept ctor param 'contextNetworkId') - NetworkEndpointSnapshot.NetworkID record positional param - Removed 'sealed' from 9 shipped resource classes: - NextJsAppResource, ViteAppResource - AzureNatGatewayResource, AzureNetworkSecurityGroupResource, AzureNetworkSecurityPerimeterResource, AzurePrivateEndpointResource, AzurePublicIPAddressResource, AzureSubnetResource, AzureVirtualNetworkResource Kept (not binary-breaking): - All constructor/method parameter renames (networkID->networkId, etc.) - EndpointReferenceAnnotation.ContextNetworkId (new in 13.4) - EndpointNames type change (HashSet -> ISet) - WithHiddenOnCompletion overload disambiguation - [Experimental("ASPIREAZURE003")] on AzureRoleAssignmentResource - 'sealed' on KubernetesHelmChartResource, BlazorWasmAppResource, BunAppResource (new in 13.4) Package validation: - Bumped PackageValidationBaselineVersion 13.2.2 -> 13.3.5 - Regenerated CompatibilitySuppressions.xml in 4 projects: most legacy entries against 13.2.2 are no longer needed because those APIs already shipped in 13.3.x. The remaining suppression is PublishAsNpmScript in Aspire.Hosting.JavaScript (documented removal from PR #17382). --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d837395 to
86ec705
Compare
86ec705 to
4596801
Compare
joperezr
left a comment
There was a problem hiding this comment.
13.4.0 shipped today. This is good to go.
…Script support - Update bun-apps.mdx to reference Aspire.Hosting.JavaScript (BunAppResource) instead of the deprecated CommunityToolkit.Aspire.Hosting.Bun package - Add TypeScript AppHost examples alongside C# examples - Add BunAppResource to the resource types list in javascript.mdx Documents API surface changes from microsoft/aspire#17700 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1153
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1153 targeting Updated Files modified:
Note This draft PR needs human review before merging. |
Auto-generated update to the API surface to compare current surface vs latest release. This should only be merged once this surface area ships in a new release.