Skip to content

Add UvicornAppResource deriving from PythonAppResource#12502

Merged
davidfowl merged 5 commits into
mainfrom
copilot/decide-uvicornappresource-derivation
Oct 30, 2025
Merged

Add UvicornAppResource deriving from PythonAppResource#12502
davidfowl merged 5 commits into
mainfrom
copilot/decide-uvicornappresource-derivation

Conversation

Copilot AI commented Oct 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Implements decision from #12265 to create a specialized UvicornAppResource that derives from PythonAppResource, following the pattern established by ViteAppResource : NodeAppResource.

Changes

  • Added UvicornAppResource class in Aspire.Hosting.Python namespace

    • Derives from PythonAppResource
    • Represents Uvicorn-based ASGI applications (FastAPI, Starlette, etc.)

  • Updated AddUvicornApp signature to return IResourceBuilder<UvicornAppResource>

    • Implementation now creates UvicornAppResource instances
    • Uses CreateResourceBuilder to maintain proper type after base class extension methods

  • Added test verifying AddUvicornApp produces correct resource type

Example

// Returns IResourceBuilder<UvicornAppResource> instead of IResourceBuilder<PythonAppResource>
var api = builder.AddUvicornApp("api", "../fastapi-app", "main:app")
    .WithUvEnvironment()
    .WithExternalHttpEndpoints();

All existing usages in templates and playground continue to work due to inheritance.

Original prompt

This section details on the original issue you should resolve

<issue_title>Decide if we want a UvicornAppResource deriving from PythonAppResource</issue_title>
<issue_description>We'll need to decide if we make this UvicornAppResource : PythonAppResource

Originally posted by @davidfowl in #12265 (comment)
</issue_description>

Comments on the Issue (you are @copilot in this section)

@davidfowl Yes, we do.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@eerhardt
Add UvicornAppResource class deriving from PythonAppResource
c14d783 
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Copilot AI changed the title [WIP] Decide if UvicornAppResource should derive from PythonAppResource Add UvicornAppResource deriving from PythonAppResource Oct 29, 2025
Copilot AI requested a review from eerhardt October 29, 2025 22:26
@eerhardt
Remove duplicated code.
0d39af7 
Make methods generic to support the derived type.
@github-actions

github-actions Bot commented Oct 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12502

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12502"

@eerhardt eerhardt marked this pull request as ready for review October 29, 2025 23:52
@eerhardt eerhardt requested review from Copilot and davidfowl October 29, 2025 23:52
Copilot AI reviewed Oct 30, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new UvicornAppResource class to explicitly represent Uvicorn-based Python applications in the Aspire hosting model. The change refactors AddUvicornApp to return the specialized resource type instead of the generic PythonAppResource, and updates several extension methods to be generic to support derived resource types.

Key changes:

  • Added UvicornAppResource class that inherits from PythonAppResource with comprehensive XML documentation for a public API
  • Made WithVirtualEnvironment, WithDebugging, and WithEntrypoint extension methods generic to support derived types
  • Updated AddUvicornApp return type from IResourceBuilder<PythonAppResource> to IResourceBuilder<UvicornAppResource>
  • Fixed XML documentation references to use generic type parameters
  • Added test coverage for the new UvicornAppResource type

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/Aspire.Hosting.Python/UvicornAppResource.cs Introduces new UvicornAppResource class with comprehensive XML documentation
src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs Refactors extension methods to be generic, updates AddUvicornApp to return UvicornAppResource, and fixes XML doc references
tests/Aspire.Hosting.Python.Tests/AddUvicornAppTests.cs Adds test to verify AddUvicornApp creates the correct resource type

Comment thread src/Aspire.Hosting.Python/UvicornAppResource.cs
Comment thread src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs
/// </para>
/// <para>
/// By default, the virtual environment folder is expected to be named <c>.venv</c> and located in the app directory.
/// Use <see cref="WithVirtualEnvironment"/> to specify a different virtual environment path.

Copilot AI Oct 30, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <see cref> tag is missing the generic type parameter. It should be <see cref=\"WithVirtualEnvironment{T}(IResourceBuilder{T}, string)\"/> to properly reference the generic method signature, ensuring accurate IntelliSense and documentation generation.

Suggested change
/// Use <see cref="WithVirtualEnvironment"/> to specify a different virtual environment path.
/// Use <see cref="WithVirtualEnvironment{T}(IResourceBuilder{T}, string)"/> to specify a different virtual environment path.

Copilot uses AI. Check for mistakes.
Comment thread tests/Aspire.Hosting.Python.Tests/AddUvicornAppTests.cs
Comment on lines +16 to +23
// Arrange
using var builder = TestDistributedApplicationBuilder.Create();
var appDirectory = Path.Combine(Path.GetTempPath(), "test-app");

// Act
var uvicornApp = builder.AddUvicornApp("uvicorn-app", appDirectory, "main:app");

// Assert

Copilot AI Oct 30, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the custom coding guidelines, test methods should not include "Act", "Arrange" or "Assert" comments. Remove these comments to match the coding style used elsewhere in the codebase.

Suggested change
// Arrange
using var builder = TestDistributedApplicationBuilder.Create();
var appDirectory = Path.Combine(Path.GetTempPath(), "test-app");
// Act
var uvicornApp = builder.AddUvicornApp("uvicorn-app", appDirectory, "main:app");
// Assert
using var builder = TestDistributedApplicationBuilder.Create();
var appDirectory = Path.Combine(Path.GetTempPath(), "test-app");
var uvicornApp = builder.AddUvicornApp("uvicorn-app", appDirectory, "main:app");

Copilot uses AI. Check for mistakes.
@davidfowl davidfowl merged commit 2717f1b into main Oct 30, 2025
587 of 591 checks passed
@davidfowl davidfowl deleted the copilot/decide-uvicornappresource-derivation branch October 30, 2025 05:12
@dotnet-policy-service dotnet-policy-service Bot added this to the 13.0 milestone Oct 30, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@davidfowl davidfowl davidfowl approved these changes

@eerhardt eerhardt Awaiting requested review from eerhardt

Assignees

@eerhardt eerhardt

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

13.0

Development

Successfully merging this pull request may close these issues.

Decide if we want a UvicornAppResource deriving from PythonAppResource

4 participants

@davidfowl @eerhardt