Add polyglot exports for Aspire.Hosting.Python#14960
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14960Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14960" |
There was a problem hiding this comment.
Pull request overview
This PR exposes Aspire.Hosting.Python public extension methods to the polyglot capability system via [AspireExport] and adds a TypeScript “ValidationAppHost” playground to exercise the generated TypeScript SDK.
Changes:
- Added
[AspireExport]attributes to Python hosting extension methods for polyglot exports. - Added a TypeScript ValidationAppHost playground (TS config + example apphost).
- Added generated TypeScript SDK modules (transport/base/aspire) and codegen hash metadata.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs | Adds [AspireExport] metadata for Python-related builder extension methods. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/tsconfig.json | Configures TypeScript compilation settings for the ValidationAppHost playground. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/package.json | Defines Node/TS dependencies and scripts for running the playground. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/apphost.ts | Adds a TS apphost that invokes the new exported Python capabilities. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/apphost.run.json | Adds run profile configuration for the playground. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/.modules/transport.ts | Implements the JSON-RPC transport, handle wrapping, callbacks, and cancellation plumbing. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/.modules/base.ts | Adds base SDK types (ReferenceExpression, collection wrappers) used by generated code. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/.modules/aspire.ts | Adds generated TypeScript SDK bindings for capabilities, including Python exports. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/.modules/.codegen-hash | Tracks the generator output hash for the SDK modules. |
| playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/.aspire/settings.json | Declares Aspire tooling settings for running the TS apphost and package mapping. |
Files not reviewed (1)
- playground/polyglot/TypeScript/Aspire.Hosting.Python/ValidationAppHost/package-lock.json: Language not supported
| async toJSON(): Promise<MarshalledHandle> { | ||
| const handle = await this._ensureHandle(); | ||
| return handle.toJSON(); | ||
| } |
There was a problem hiding this comment.
toJSON() is declared async and returns a Promise. JSON.stringify() (and most JSON-RPC serializers) will not await toJSON(), which can result in {} or an incorrect payload on the wire. Make toJSON() synchronous (returning a MarshalledHandle), and expose a separate toHandle()/resolve() method for async resolution if needed (or ensure the handle is eagerly resolved before the object can be serialized).
| async toJSON(): Promise<MarshalledHandle> { | |
| const handle = await this._ensureHandle(); | |
| return handle.toJSON(); | |
| } | |
| /** | |
| * Resolves and returns the underlying handle in marshalled form. | |
| * Use this method when you need to asynchronously obtain a value | |
| * suitable for transport or manual JSON serialization. | |
| */ | |
| async toHandle(): Promise<MarshalledHandle> { | |
| const handle = await this._ensureHandle(); | |
| return handle.toJSON(); | |
| } | |
| /** | |
| * Synchronous JSON representation. | |
| * | |
| * This method cannot perform asynchronous handle resolution, so it will | |
| * throw if called directly. Call {@link toHandle} first and serialize | |
| * the returned value instead. | |
| */ | |
| toJSON(): MarshalledHandle { | |
| throw new Error( | |
| 'AspireDict.toJSON() is synchronous and cannot resolve handles. ' + | |
| 'Call await dict.toHandle() to obtain a serializable handle first.' | |
| ); | |
| } |
| toJSON(): MarshalledHandle { | ||
| if (this._resolvedHandle) { | ||
| return this._resolvedHandle.toJSON(); | ||
| } | ||
| return this._handleOrContext.toJSON(); |
There was a problem hiding this comment.
When the list handle requires resolution via _getterCapabilityId, toJSON() falls back to serializing _handleOrContext (the context handle), not the actual list handle. If an AspireList instance is ever passed as an argument to a capability, this will send the wrong handle type/value and can break server-side invocation. Consider removing toJSON() from AspireList (forcing explicit list operations only), or ensure AspireList always stores/uses a real list handle (eagerly resolve on creation) so toJSON() is always correct.
| toJSON(): MarshalledHandle { | |
| if (this._resolvedHandle) { | |
| return this._resolvedHandle.toJSON(); | |
| } | |
| return this._handleOrContext.toJSON(); | |
| toJSON(): MarshalledHandle { | |
| if (!this._resolvedHandle) { | |
| throw new Error( | |
| 'AspireList handle is not yet resolved and cannot be serialized. ' + | |
| 'Use explicit list operations (e.g., toArray) instead of passing AspireList directly.' | |
| ); | |
| } | |
| return this._resolvedHandle.toJSON(); |
| let currentClient: AspireClient | null = null; | ||
|
|
There was a problem hiding this comment.
currentClient is a mutable singleton shared across the module and overwritten on each AspireClient.connect(). If multiple clients are created (or reconnect logic is added later), cancellation requests can be routed to the wrong connection. A safer design is to associate cancellation registration with a specific AspireClient instance (e.g., move the cancellation registry onto the client, or pass the client into registerCancellation() and store per-client registries).
| // Set up the abort listener | ||
| const onAbort = () => { | ||
| // Send cancel request to host | ||
| if (currentClient?.connected) { | ||
| currentClient.cancelToken(cancellationId).catch(() => { |
There was a problem hiding this comment.
currentClient is a mutable singleton shared across the module and overwritten on each AspireClient.connect(). If multiple clients are created (or reconnect logic is added later), cancellation requests can be routed to the wrong connection. A safer design is to associate cancellation registration with a specific AspireClient instance (e.g., move the cancellation registry onto the client, or pass the client into registerCancellation() and store per-client registries).
| // Set up the abort listener | |
| const onAbort = () => { | |
| // Send cancel request to host | |
| if (currentClient?.connected) { | |
| currentClient.cancelToken(cancellationId).catch(() => { | |
| // Capture the client at registration time to avoid using a later, different client. | |
| const clientForCancellation = currentClient; | |
| // Set up the abort listener | |
| const onAbort = () => { | |
| // Send cancel request to host | |
| if (clientForCancellation?.connected) { | |
| clientForCancellation.cancelToken(cancellationId).catch(() => { |
| currentClient = this; | ||
|
|
There was a problem hiding this comment.
currentClient is a mutable singleton shared across the module and overwritten on each AspireClient.connect(). If multiple clients are created (or reconnect logic is added later), cancellation requests can be routed to the wrong connection. A safer design is to associate cancellation registration with a specific AspireClient instance (e.g., move the cancellation registry onto the client, or pass the client into registerCancellation() and store per-client registries).
| @@ -0,0 +1,19 @@ | |||
| { | |||
| "name": "validationapphost", | |||
| "version": "1.0.0", | |||
There was a problem hiding this comment.
Since this appears to be a playground/sample package, consider adding \"private\": true to avoid accidental npm publish (especially because the name is fairly generic).
| "version": "1.0.0", | |
| "version": "1.0.0", | |
| "private": true, |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22697326569 |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Related to #14069
Adds
[AspireExport]attributes toAspire.Hosting.Pythonpublic extension methods and includes the TypeScript ValidationAppHost playground.Replaces #14897 (rebased on release/13.2).