Move Python starter template to TypeScript AppHost and CLI template factory#15574
Move Python starter template to TypeScript AppHost and CLI template factory#15574
Conversation
…actory
Migrates aspire-py-starter from a dotnet new template (C# AppHost) to a
CLI embedded template (TypeScript AppHost), matching the pattern used by
aspire-ts-starter.
Changes:
- Add py-starter embedded template directory with apphost.ts, Python
FastAPI backend (app/), React/Vite frontend, and TS AppHost infra
- Add CliTemplateFactory.PythonStarterTemplate.cs with conditional Redis
support using block markers ({{#redis}}/{{/redis}})
- Register aspire-py-starter in CliTemplateFactory with --use-redis-cache
and --localhost-tld options
- Remove aspire-py-starter from DotNetTemplateFactory
- Remove old template from Aspire.ProjectTemplates
- Update tests to reflect the template migration
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nsive tests Moved ProcessBlock/ProcessConditionalBlocks from CliTemplateFactory.PythonStarterTemplate into a standalone ConditionalBlockProcessor class. Added 29 focused unit tests covering: - Basic include/exclude behavior - Comment style variations (// vs # vs bare vs indented) - Multiple blocks, adjacent blocks - Position edge cases (start of file, end without trailing newline, entire file) - Empty blocks, multiline content, curly braces, template tokens in content - Missing/mismatched markers (graceful no-op) - Multiple conditions via Process() dictionary API - Realistic Python template scenarios with redis/no-redis conditions - Whitespace/formatting preservation - Block name edge cases (hyphens, similar prefixes) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
requested review from
JamesNK,
eerhardt,
mitchdenny and
radical
as code owners
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15574Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15574" |
Replace custom JsonNode manipulation in AddRedisPackageToConfig with the established AspireConfigFile.LoadOrCreate/AddOrUpdatePackage/Save API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Migrates the Python starter template (aspire-py-starter) from the legacy dotnet new template pipeline to the embedded CLI template pipeline, switching the AppHost from C# to TypeScript to reduce required toolchains and align with the existing TS starter pattern.
Changes:
- Removed
aspire-py-starterfromDotNetTemplateFactory/ old project template assets and updated tests accordingly. - Added a new embedded CLI template tree for
py-starter(TypeScript AppHost + FastAPI backend + Vite React frontend). - Introduced
ConditionalBlockProcessor(with unit tests) and added a new CLI template factory implementation for the Python starter, including a--use-redis-cacheoption and conditional template processing.
Reviewed changes
Copilot reviewed 36 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Templates.Tests/LocalhostTldHostnameTests.cs | Removes py-starter coverage from dotnet-template hostname tests since it’s no longer a dotnet template. |
| tests/Aspire.Cli.Tests/Templating/DotNetTemplateFactoryTests.cs | Updates expectations so py-starter is no longer returned by DotNetTemplateFactory. |
| tests/Aspire.Cli.Tests/Templating/ConditionalBlockProcessorTests.cs | Adds unit coverage for the new conditional block processing utility. |
| src/Aspire.ProjectTemplates/Aspire.ProjectTemplates.csproj | Stops packaging the old py-starter dotnet template content. |
| src/Aspire.Cli/Aspire.Cli.csproj | Embeds the new py-starter template resources into the CLI. |
| src/Aspire.Cli/Templating/DotNetTemplateFactory.cs | Removes py-starter from dotnet template enumeration and associated prompt logic. |
| src/Aspire.Cli/Templating/KnownTemplateId.cs | Adds a PythonStarter known template ID constant. |
| src/Aspire.Cli/Templating/ConditionalBlockProcessor.cs | Adds reusable conditional block marker processing for templates. |
| src/Aspire.Cli/Templating/CliTemplateFactory.cs | Registers the Python starter in CLI template definitions and adds --use-redis-cache option plumbing. |
| src/Aspire.Cli/Templating/CliTemplateFactory.PythonStarterTemplate.cs | Implements the new CLI template application logic, conditional Redis inclusion, and aspire.config.json mutation. |
| src/Aspire.Cli/Templating/Templates/py-starter/** | Adds the new embedded Python starter template content (AppHost + backend + frontend). |
| src/Aspire.ProjectTemplates/templates/aspire-py-starter/** | Deletes the legacy dotnet-template assets for the Python starter. |
Files not reviewed (1)
- src/Aspire.Cli/Templating/Templates/py-starter/package-lock.json: Language not supported
| var ports = GenerateRandomPorts(); | ||
| var hostName = useLocalhostTld ? $"{projectNameLower}.dev.localhost" : "localhost"; | ||
| var conditions = new Dictionary<string, bool> | ||
| { | ||
| ["redis"] = useRedisCache, | ||
| ["no-redis"] = !useRedisCache, | ||
| }; | ||
| string ApplyAllTokens(string content) => ConditionalBlockProcessor.Process( | ||
| ApplyTokens(content, projectName, projectNameLower, aspireVersion, ports, hostName), |
There was a problem hiding this comment.
When --localhost-tld is enabled, the hostname is derived from projectName.ToLowerInvariant() without any DNS-safe normalization (e.g., replacing underscores/dots, trimming hyphens). Project names like Test_App.1 will produce invalid *.dev.localhost hostnames, unlike the .NET template behavior validated by LocalhostTldHostnameTests.
Consider normalizing the project name to a DNS-compliant label (replace invalid chars with hyphens, collapse repeats, trim leading/trailing hyphens) before appending .dev.localhost.
There was a problem hiding this comment.
This is a pre-existing pattern shared by all CLI templates — both CliTemplateFactory.TypeScriptStarterTemplate.cs (line 55) and CliTemplateFactory.EmptyTemplate.cs (line 167) use the same projectName.ToLowerInvariant() without DNS normalization. This PR just follows the established pattern. A DNS-safe normalization improvement could be done as a follow-up across all templates via the shared ApplyTokens method.
davidfowl
force-pushed
the
davidfowl/python-template-ts
branch
from
fada218 to
eb4b0e8
Compare
Add [CaptureWorkspaceOnFailure] attribute that uses xUnit v3's TestContext.Current.TestState to detect failure, then copies the workspace directory to testresults/workspaces/ for CI artifact upload. Tests register their workspace via RegisterWorkspaceForCapture() and add the attribute — no wrapping or extra indentation needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
davidfowl/python-template-ts
branch
from
eb4b0e8 to
c190fc3
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| while (true) | ||
| { | ||
| var startIdx = content.IndexOf(startPattern, StringComparison.Ordinal); | ||
| if (startIdx < 0) | ||
| { | ||
| break; |
There was a problem hiding this comment.
If a start marker exists but the matching end marker is misspelled or missing, the processor silently returns the content unchanged — leaving raw {{#name}} markers in the generated template output. This would be very hard to catch since there's no warning. Consider logging a warning (or throwing) when startIdx >= 0 but endIdx < 0, so template authoring typos don't produce silently broken output.
| } | ||
| } | ||
| catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) | ||
| { | ||
| _interactionService.DisplayError($"Failed to create project files: {ex.Message}"); | ||
| return new TemplateResult(ExitCodeConstants.FailedToCreateNewProject); | ||
| } | ||
|
|
||
| _interactionService.DisplaySuccess($"Created Python starter project at {outputPath.EscapeMarkup()}"); | ||
| DisplayPostCreationInstructions(outputPath); | ||
|
|
||
| return templateResult; | ||
| } | ||
|
|
||
| private async Task<bool> ResolveUseRedisCacheAsync(System.CommandLine.ParseResult parseResult, CancellationToken cancellationToken) | ||
| { | ||
| var redisCacheOptionSpecified = parseResult.Tokens.Any(token => | ||
| string.Equals(token.Value, "--use-redis-cache", StringComparisons.CliInputOrOutput)); | ||
| var useRedisCache = parseResult.GetValue(_useRedisCacheOption); | ||
| if (!redisCacheOptionSpecified) | ||
| { | ||
| if (!_hostEnvironment.SupportsInteractiveInput) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| useRedisCache = await _interactionService.PromptForSelectionAsync( | ||
| TemplatingStrings.UseRedisCache_Prompt, | ||
| [TemplatingStrings.Yes, TemplatingStrings.No], | ||
| choice => choice, | ||
| cancellationToken) switch | ||
| { | ||
| var choice when string.Equals(choice, TemplatingStrings.Yes, StringComparisons.CliInputOrOutput) => true, | ||
| var choice when string.Equals(choice, TemplatingStrings.No, StringComparisons.CliInputOrOutput) => false, | ||
| _ => throw new InvalidOperationException(TemplatingStrings.UseRedisCache_UnexpectedChoice) |
There was a problem hiding this comment.
This method is a near-duplicate of ResolveUseLocalhostTldAsync — same check-tokens-then-prompt-interactively pattern. Both check parseResult.Tokens for a specific option name, call parseResult.GetValue, fall back to PromptForSelectionAsync with Yes/No, and display a confirmation message. Consider extracting a shared helper like ResolveBooleanOptionAsync(parseResult, option, promptText, confirmationText, cancellationToken) to reduce duplication, especially since more templates with boolean options will likely follow.
| internal static string Process(string content, IReadOnlyDictionary<string, bool> conditions) | ||
| { | ||
| foreach (var (blockName, include) in conditions) | ||
| { | ||
| content = ProcessBlock(content, blockName, include); | ||
| } | ||
|
|
||
| return content; |
There was a problem hiding this comment.
Process() iterates IReadOnlyDictionary<string, bool> whose enumeration order is not contractually guaranteed. For the current redis/no-redis usage with non-overlapping blocks this works fine, but the API shape invites future callers to add conditions with nested or overlapping blocks, where the behavior would become silently order-dependent.
Minor suggestion: either document the non-overlapping requirement with a comment here, or accept an IReadOnlyList<(string Name, bool Include)> to make ordering explicit.
Verify .modules/aspire.ts is generated with addUvicornApp after aspire new, and cd into the project directory before running. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| content = ProcessBlock(content, blockName, include); | ||
| } | ||
|
|
||
| return content; |
There was a problem hiding this comment.
Add a debug assert that there are no conditional blocks in the content once complete? i.e. have a regex check for {{#ALPHA_CHARACTERS}} and {{/ALPHA_CHARACTERS}} and throw an error if there is any match
List all add* functions and .modules files to help diagnose why addUvicornApp is missing from the generated SDK. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 2 jobs were identified as retry-safe transient failures in the CI run attempt.
|
Docker-created directories like .modules may have root ownership, causing CopyDirectory to fail. Handle errors per-file/per-directory so partial captures still succeed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
upload-artifact uses include-hidden-files: false by default, which skips directories like .modules and .aspire. Renaming them to use _ prefix (e.g. _modules) ensures they appear in artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 49 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23547924657 |
Match the original test shape: aspire new then aspire run. The aspire run command regenerates the SDK correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Migrates the
aspire-py-startertemplate from the olddotnet newtemplate system (DotNetTemplateFactory with C# AppHost) to the new embedded CLI template system (CliTemplateFactory with TypeScript AppHost).Motivation: Narrowing down the toolchains required — by moving to a TypeScript AppHost, the Python starter no longer requires the .NET SDK/C# toolchain for the AppHost project, aligning it with the same pattern as
aspire-ts-starter.What changed
src/Aspire.Cli/Templating/Templates/py-starter/with a TypeScript AppHost usingaddUvicornApp, conditional Redis support via{{#redis}}/{{/redis}}block markers, and a Vite React frontendCliTemplateFactory.PythonStarterTemplate.cs— factory logic for the Python starter, including Redis conditional processing andaspire.config.jsonpackage injectionConditionalBlockProcessorutility class — extracted reusable conditional block processing with 29 unit tests covering comment styles, edge cases, multiple conditions, and realistic template scenariosAspire.ProjectTemplates(dotnet new aspire-py-starteris replaced byaspire new aspire-py-starter)aspire-py-startermoved fromDotNetTemplateFactorytoCliTemplateFactorywith--use-redis-cacheand--localhost-tldoptionsDotNetTemplateFactoryTestsupdated to expect py-starter not in dotnet templates; hostname tests removed for py-starterValidation
Aspire.CliandAspire.ProjectTemplates)ConditionalBlockProcessorTestspassDotNetTemplateFactoryTestspassNewCommandTestspass (confirmsaspire-py-starterlisted in CLI help)aspire new aspire-py-startergenerates correct output for both--use-redis-cache falseand--use-redis-cache truevariantsChecklist
aspire.devissue: