Add polyglot withImagePushOptions support#15809
Open
sebastienros wants to merge 3 commits intomainfrom
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sebastienros
requested review from
JamesNK,
danegsta,
davidfowl,
karolz-ms and
mitchdenny
as code owners
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15809Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15809" |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds polyglot AppHost parity for WithImagePushOptions (issue #15705) by exporting the capability to ATS/polyglot SDKs and updating polyglot fixtures + generated snapshots to validate the new surface area across TypeScript, Python, and Java.
Changes:
- Export
withImagePushOptionsfor polyglot hosts and adjust export/ignore annotations accordingly. - Expose
ContainerImagePushOptionsCallbackContextandContainerImagePushOptionsproperties to ATS so polyglot callbacks can read/modify options. - Update polyglot AppHost fixtures and codegen snapshots (TS/Java/Python) to cover the new API.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Exports withImagePushOptions and removes stale remarks about polyglot unavailability. |
| src/Aspire.Hosting/ApplicationModel/ContainerImagePushOptionsCallbackContext.cs | Marks callback context for ATS property exposure. |
| src/Aspire.Hosting/ApplicationModel/ContainerImagePushOptions.cs | Marks options type for ATS property exposure. |
| tests/PolyglotAppHosts/Aspire.Hosting/TypeScript/apphost.ts | Adds fixture usage of withImagePushOptions in TypeScript. |
| tests/PolyglotAppHosts/Aspire.Hosting/Python/apphost.py | Adds fixture usage of with_image_push_options in Python. |
| tests/PolyglotAppHosts/Aspire.Hosting/Java/AppHost.java | Adds fixture usage of withImagePushOptions in Java. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.ts | Updates generated TS SDK snapshot with new types/capabilities. |
| tests/Aspire.Hosting.CodeGeneration.TypeScript.Tests/Snapshots/HostingContainerResourceCapabilities.verified.txt | Updates TS capability snapshot to include withImagePushOptions. |
| tests/Aspire.Hosting.CodeGeneration.Python.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.py | Updates generated Python SDK snapshot with new types/capabilities. |
| tests/Aspire.Hosting.CodeGeneration.Java.Tests/Snapshots/TwoPassScanningGeneratedAspire.verified.java | Updates generated Java SDK snapshot with new types/capabilities. |
Comments suppressed due to low confidence (2)
src/Aspire.Hosting/ResourceBuilderExtensions.cs:3676
- For polyglot callbacks, this repo typically exports the async Func<..., Task> overload and ignores the Action overload (e.g., WithEnvironment exports withEnvironmentCallback and ignores the Action overload) to avoid sync-blocking remote callback invocation. Here, withImagePushOptions is exported on the Action overload, which will invoke the polyglot callback via a synchronous wait; consider exporting the Func<ContainerImagePushOptionsCallbackContext, Task> overload instead (and ignoring Action) for consistency and better scalability.
[Experimental("ASPIREPIPELINES003", UrlFormat = "https://aka.ms/aspire/diagnostics#{0}")]
[AspireExport("withImagePushOptions", Description = "Sets image push options via callback")]
public static IResourceBuilder<T> WithImagePushOptions<T>(
this IResourceBuilder<T> builder,
Action<ContainerImagePushOptionsCallbackContext> callback)
where T : IComputeResource
src/Aspire.Hosting/ResourceBuilderExtensions.cs:3716
- The async WithImagePushOptions(Func<..., Task>) overload is currently ignored for polyglot even though polyglot callback plumbing supports async delegates and other APIs in this file prefer exporting the async overload. If you switch the export to the async overload, update/remove this AspireExportIgnore reason accordingly to match the established pattern.
[Experimental("ASPIREPIPELINES003", UrlFormat = "https://aka.ms/aspire/diagnostics#{0}")]
[AspireExportIgnore(Reason = "Polyglot app hosts use the synchronous Action<> overload via withImagePushOptions.")]
public static IResourceBuilder<T> WithImagePushOptions<T>(
this IResourceBuilder<T> builder,
Func<ContainerImagePushOptionsCallbackContext, Task> callback)
where T : IComputeResource
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🎬 CLI E2E Test Recordings — 55 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23919961297 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue #15705 asks for TypeScript AppHost parity for
WithImagePushOptionsso polyglot app hosts can customize image push settings without falling back to narrower helpers. This keeps the change scoped to the existing generated callback model rather than introducing a new TypeScript-specific API shape.This PR exports a single
withImagePushOptionsoverload for polyglot hosts, marksContainerImagePushOptionsCallbackContextandContainerImagePushOptionsfor ATS property exposure, and updates the generated snapshots that prove the surface across TypeScript, Java, and Python. It also moves coverage to the shared polyglot apphost fixtures in those languages and removes stale comments that said the API was unavailable in polyglot hosts.Dependencies: N/A
Fixes #15705
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: