Fix AddPythonApp parameter ambiguity using OverloadResolutionPriority (#11892)

* Initial plan

* Fix AddPythonApp parameter ambiguity by introducing IEnumerable<string> overload

Co-authored-by: adamint <[email protected]>

* Update API surface file for AddPythonApp changes

Co-authored-by: adamint <[email protected]>

* Implement OverloadResolutionPriority pattern with deprecated overloads and WithScriptArgs

Co-authored-by: davidfowl <[email protected]>

* Suppress obsolete warnings in tests

Co-authored-by: davidfowl <[email protected]>

* Remove WithScriptArgs method per review feedback - callers can use WithArgs

Co-authored-by: davidfowl <[email protected]>

* Refactor AddPythonAppCore to not take scriptArgs - use WithArgs instead

Co-authored-by: davidfowl <[email protected]>

* Implement WithVirtualEnvironment using WithCommand to update venv after resource creation

Co-authored-by: davidfowl <[email protected]>

* Update src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs

* Fix typo in ApplicationBuilder reference

* Update src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs

* Add CompatibilitySuppressions.xml and refactor Python resource handling

- Introduced CompatibilitySuppressions.xml to manage diagnostic suppressions.
- Refactored PythonAppResourceBuilderExtensions to use GetExecutable instead of GetRequiredExecutable for better flexibility.
- Updated virtual environment handling in WithVirtualEnvironment method.
- Removed deprecated PythonProjectResource and PythonProjectResourceBuilderExtensions classes.

* Remove deprecated PythonProjectResource tests and update references to PythonAppResource

* Update example usage and improve null argument checks in PythonAppResourceBuilderExtensions

* Add unit tests for AddPythonApp method to validate virtual environment handling

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: adamint <[email protected]>
Co-authored-by: davidfowl <[email protected]>
Co-authored-by: David Fowler <[email protected]>

Author: 3 people

src/Aspire.Hosting.Python/Aspire.Hosting.Python.csproj

@@ -9,6 +9,7 @@
 
   <ItemGroup>
     <Compile Include="$(SharedDir)PathNormalizer.cs" Link="Utils\PathNormalizer.cs" />
+    <Compile Include="$(SharedDir)OverloadResolutionPriorityAttribute.cs" Link="Utils\OverloadResolutionPriorityAttribute.cs" />
   </ItemGroup>
 
   <ItemGroup>

src/Aspire.Hosting.Python/CompatibilitySuppressions.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
+<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
+  <Suppression>
+    <DiagnosticId>CP0001</DiagnosticId>
+    <Target>T:Aspire.Hosting.Python.PythonProjectResource</Target>
+    <Left>lib/net8.0/Aspire.Hosting.Python.dll</Left>
+    <Right>lib/net8.0/Aspire.Hosting.Python.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0001</DiagnosticId>
+    <Target>T:Aspire.Hosting.PythonProjectResourceBuilderExtensions</Target>
+    <Left>lib/net8.0/Aspire.Hosting.Python.dll</Left>
+    <Right>lib/net8.0/Aspire.Hosting.Python.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+</Suppressions>

src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs

@@ -1,6 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.ComponentModel;
+using System.Runtime.CompilerServices;
 #pragma warning disable ASPIREEXTENSION001
 using Aspire.Hosting.ApplicationModel;
 using Aspire.Hosting.Python;
@@ -14,25 +16,22 @@ namespace Aspire.Hosting;
 public static class PythonAppResourceBuilderExtensions
 {
     /// <summary>
-    /// Adds a python application with a virtual environment to the application model.
+    /// Adds a python application to the application model.
     /// </summary>
     /// <param name="builder">The <see cref="IDistributedApplicationBuilder"/> to add the resource to.</param>
     /// <param name="name">The name of the resource.</param>
     /// <param name="appDirectory">The path to the directory containing the python app files.</param>
     /// <param name="scriptPath">The path to the script relative to the app directory to run.</param>
-    /// <param name="scriptArgs">The arguments for the script.</param>
     /// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
     /// <remarks>
     /// <para>
-    /// The virtual environment must be initialized before running the app. By default the virtual environment folder is expected
-    /// to be named <c>.venv</c> and be located in the app directory. If the virtual environment is located in a different directory
-    /// this default can be specified by using the <see cref="AddPythonApp(IDistributedApplicationBuilder, string, string, string, string, string[])"/>
-    /// overload of this method.
+    /// By default, the virtual environment folder is expected to be named <c>.venv</c> and located in the app directory.
+    /// Use <see cref="WithVirtualEnvironment(IResourceBuilder{PythonAppResource}, string)"/> to specify a different virtual environment path.
+    /// Use <c>WithArgs</c> to pass arguments to the script.
     /// </para>
     /// <para>
-    /// The virtual environment is setup individually for each app to allow each app to use a different version of
-    /// Python and dependencies. To setup a virtual environment use the <c>python -m venv .venv</c> command in the app
-    /// directory. This will create a virtual environment in the <c>.venv</c> directory.
+    /// The virtual environment must be initialized before running the app. To setup a virtual environment use the 
+    /// <c>python -m venv .venv</c> command in the app directory.
     /// </para>
     /// <para>
     /// To restore dependencies in the virtual environment first activate the environment by executing the activation
@@ -44,23 +43,49 @@ public static class PythonAppResourceBuilderExtensions
     /// your Python app.
     /// </para>
     /// <example>
-    /// Add a python app or executable to the application model. In this example the python code entry point is located in the <c>PythonApp</c> directory
-    /// if this path is relative then it is assumed to be relative to the AppHost directory, and the virtual environment path if relative
-    /// is relative to the app directory. In the example below, if the app host directory is <c>$HOME/repos/MyApp/src/MyApp.AppHost</c> then
-    /// the AppPath would be <c>$HOME/repos/MyApp/src/MyApp.AppHost/PythonApp</c> and the virtual environment path (defaulted) would
-    /// be <c>$HOME/repos/MyApp/src/MyApp.AppHost/PythonApp/.venv</c>.
+    /// Add a python app to the application model:
     /// <code lang="csharp">
     /// var builder = DistributedApplication.CreateBuilder(args);
     ///
-    /// builder.AddPythonApp("python-app", "PythonApp", "main.py");
+    /// builder.AddPythonApp("python-app", "../python-app", "main.py")
+    ///        .WithArgs("arg1", "arg2");
     ///
     /// builder.Build().Run();
     /// </code>
     /// </example>
     /// </remarks>
+    [OverloadResolutionPriority(1)]
+    public static IResourceBuilder<PythonAppResource> AddPythonApp(
+        this IDistributedApplicationBuilder builder, [ResourceName] string name, string appDirectory, string scriptPath)
+        => AddPythonAppCore(builder, name, appDirectory, scriptPath, ".venv");
+
+    /// <summary>
+    /// Adds a python application with a virtual environment to the application model.
+    /// </summary>
+    /// <param name="builder">The <see cref="IDistributedApplicationBuilder"/> to add the resource to.</param>
+    /// <param name="name">The name of the resource.</param>
+    /// <param name="appDirectory">The path to the directory containing the python app files.</param>
+    /// <param name="scriptPath">The path to the script relative to the app directory to run.</param>
+    /// <param name="scriptArgs">The arguments for the script.</param>
+    /// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
+    /// <remarks>
+    /// <para>
+    /// This overload is obsolete. Use the overload without parameters and chain with <c>WithArgs</c>:
+    /// <code lang="csharp">
+    /// builder.AddPythonApp("name", "dir", "script.py")
+    ///        .WithArgs("arg1", "arg2");
+    /// </code>
+    /// </para>
+    /// </remarks>
+    [Obsolete("Use AddPythonApp(builder, name, appDirectory, scriptPath) and chain with .WithArgs(...) instead.")]
+    [EditorBrowsable(EditorBrowsableState.Never)]
     public static IResourceBuilder<PythonAppResource> AddPythonApp(
         this IDistributedApplicationBuilder builder, string name, string appDirectory, string scriptPath, params string[] scriptArgs)
-        => AddPythonApp(builder, name, appDirectory, scriptPath, ".venv", scriptArgs);
+    {
+        ThrowIfNullOrContainsIsNullOrEmpty(scriptArgs);
+        return AddPythonAppCore(builder, name, appDirectory, scriptPath, ".venv")
+            .WithArgs(scriptArgs);
+    }
 
     /// <summary>
     /// Adds a python application with a virtual environment to the application model.
@@ -74,58 +99,47 @@ public static IResourceBuilder<PythonAppResource> AddPythonApp(
     /// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
     /// <remarks>
     /// <para>
-    /// The virtual environment is setup individually for each app to allow each app to use a different version of
-    /// Python and dependencies. To setup a virtual environment use the <c>python -m venv .venv</c> command in the app
-    /// directory. This will create a virtual environment in the <c>.venv</c> directory (where <c>.venv</c> is the name of your
-    /// virtual environment directory).
-    /// </para>
-    /// <para>
-    /// To restore dependencies in the virtual environment first activate the environment by executing the activation
-    /// script and then use the <c>pip install -r requirements.txt</c> command to restore dependencies.
-    /// </para>
-    /// <para>
-    /// To receive traces, logs, and metrics from the python app in the dashboard, the app must be instrumented with OpenTelemetry.
-    /// You can instrument your app by adding the <c>opentelemetry-distro</c>, and <c>opentelemetry-exporter-otlp</c> to
-    /// your Python app.
-    /// </para>
-    /// <example>
-    /// Add a python app or executable to the application model. In this example the python code is located in the <c>PythonApp</c> directory
-    /// if this path is relative then it is assumed to be relative to the AppHost directory, and the virtual environment path if relative
-    /// is relative to the app directory. In the example below, if the app host directory is <c>$HOME/repos/MyApp/src/MyApp.AppHost</c> then
-    /// the AppPath would be <c>$HOME/repos/MyApp/src/MyApp.AppHost/PythonApp</c> and the virtual environment path (defaulted) would
-    /// be <c>$HOME/repos/MyApp/src/MyApp.AppHost/PythonApp/.venv</c>.
+    /// This overload is obsolete. Use the overload without parameters and chain with <c>WithVirtualEnvironment</c> and <c>WithArgs</c>:
     /// <code lang="csharp">
-    /// var builder = DistributedApplication.CreateBuilder(args);
-    ///
-    /// builder.AddPythonApp("python-app", "PythonApp", "main.py");
-    ///
-    /// builder.Build().Run();
+    /// builder.AddPythonApp("name", "dir", "script.py")
+    ///        .WithVirtualEnvironment("myenv")
+    ///        .WithArgs("arg1", "arg2");
     /// </code>
-    /// </example>
+    /// </para>
     /// </remarks>
+    [Obsolete("Use AddPythonApp(builder, name, appDirectory, scriptPath) and chain with .WithVirtualEnvironment(...).WithArgs(...) instead.")]
+    [EditorBrowsable(EditorBrowsableState.Never)]
     public static IResourceBuilder<PythonAppResource> AddPythonApp(
         this IDistributedApplicationBuilder builder, string name, string appDirectory, string scriptPath,
         string virtualEnvironmentPath, params string[] scriptArgs)
+    {
+        ThrowIfNullOrContainsIsNullOrEmpty(scriptArgs);
+        return AddPythonAppCore(builder, name, appDirectory, scriptPath, virtualEnvironmentPath)
+            .WithArgs(scriptArgs);
+    }
+
+    private static IResourceBuilder<PythonAppResource> AddPythonAppCore(
+        IDistributedApplicationBuilder builder, string name, string appDirectory, string scriptPath,
+        string virtualEnvironmentPath)
     {
         ArgumentNullException.ThrowIfNull(builder);
         ArgumentException.ThrowIfNullOrEmpty(name);
-        ArgumentException.ThrowIfNullOrEmpty(appDirectory);
+        ArgumentNullException.ThrowIfNull(appDirectory);
         ArgumentException.ThrowIfNullOrEmpty(scriptPath);
-        ArgumentException.ThrowIfNullOrEmpty(virtualEnvironmentPath);
-        ThrowIfNullOrContainsIsNullOrEmpty(scriptArgs);
+        ArgumentNullException.ThrowIfNull(virtualEnvironmentPath);
 
         appDirectory = PathNormalizer.NormalizePathForCurrentPlatform(Path.Combine(builder.AppHostDirectory, appDirectory));
         var virtualEnvironment = new VirtualEnvironment(Path.IsPathRooted(virtualEnvironmentPath)
             ? virtualEnvironmentPath
             : Path.Join(appDirectory, virtualEnvironmentPath));
 
-        var pythonExecutable = virtualEnvironment.GetRequiredExecutable("python");
+        var pythonExecutable = virtualEnvironment.GetExecutable("python");
 
         var resource = new PythonAppResource(name, pythonExecutable, appDirectory);
 
         var resourceBuilder = builder.AddResource(resource).WithArgs(context =>
         {
-            AddArguments(scriptPath, scriptArgs, context);
+            context.Args.Add(scriptPath);
         });
 
         resourceBuilder.WithOtlpExporter();
@@ -153,16 +167,6 @@ public static IResourceBuilder<PythonAppResource> AddPythonApp(
         return resourceBuilder;
     }
 
-    private static void AddArguments(string scriptPath, string[] scriptArgs, CommandLineArgsCallbackContext context)
-    {
-        context.Args.Add(scriptPath);
-
-        foreach (var arg in scriptArgs)
-        {
-            context.Args.Add(arg);
-        }
-    }
-
     private static void ThrowIfNullOrContainsIsNullOrEmpty(string[] scriptArgs)
     {
         ArgumentNullException.ThrowIfNull(scriptArgs);
@@ -179,4 +183,25 @@ private static void ThrowIfNullOrContainsIsNullOrEmpty(string[] scriptArgs)
             }
         }
     }
+
+    /// <summary>
+    /// Configures a custom virtual environment path for the Python application.
+    /// </summary>
+    /// <param name="builder">The resource builder.</param>
+    /// <param name="virtualEnvironmentPath">The path to the virtual environment. Can be absolute or relative to the app directory.</param>
+    /// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
+    public static IResourceBuilder<PythonAppResource> WithVirtualEnvironment(
+        this IResourceBuilder<PythonAppResource> builder, string virtualEnvironmentPath)
+    {
+        ArgumentNullException.ThrowIfNull(builder);
+        ArgumentException.ThrowIfNullOrEmpty(virtualEnvironmentPath);
+
+        var virtualEnvironment = new VirtualEnvironment(Path.IsPathRooted(virtualEnvironmentPath)
+            ? virtualEnvironmentPath
+            : Path.Join(builder.Resource.WorkingDirectory, virtualEnvironmentPath));
+        // Update the command to use the new virtual environment
+        builder.WithCommand(virtualEnvironment.GetExecutable("python"));
+
+        return builder;
+    }
 }