Add support for running python projects #4142
Conversation
dotnet-issue-labeler
bot
added
the
area-components
dotnet-policy-service
bot
added
the
community-contribution
src/Aspire.Hosting.Python/FlaskProjectResourceExtensionBuilder.cs
Outdated
Show resolved
Hide resolved
|
@davidfowl I've been thinking about things like Node.js (and now Python). I'm wondering if whether to make things easier for people we should create a boilerplate Dockerfile if one doesn't already exist. The idea being that it gives people a starting point. So if someone does |
|
I don't think we should be writing files to disk using these APIs, but I like the idea that they would publish as a docker file by default. The publish operation would throw because it couldn't find one. That would instruct the user to add one for their language. |
src/Aspire.Hosting.Python/FlaskProjectResourceExtensionBuilder.cs
Outdated
Show resolved
Hide resolved
|
I can see I broke the build because I forgot to update the playground apphost. I will do that tomorrow morning (GMT+1) |
| public void AddPythonProject_SetsResourcePropertiesCorrectly() | ||
| { | ||
| var pythonProjectDirectory = Path.Combine(s_playgroundDirectory, "script_only"); | ||
| var appBuilder = DistributedApplication.CreateBuilder(); | ||
|
|
||
| appBuilder.AddPythonProject("python", pythonProjectDirectory, "main.py"); | ||
|
|
||
| var app = appBuilder.Build(); | ||
| var appModel = app.Services.GetRequiredService<DistributedApplicationModel>(); | ||
| var executableResources = appModel.GetExecutableResources(); | ||
|
|
||
| var pythonProjectResource = Assert.Single(executableResources); | ||
|
|
||
| Assert.Equal("python", pythonProjectResource.Name); | ||
| Assert.Equal(pythonProjectDirectory, pythonProjectResource.WorkingDirectory); | ||
| } |
There was a problem hiding this comment.
I would love to see some pure unit tests like this https://github.com/dotnet/aspire/blob/main/tests/Aspire.Hosting.Tests/Garnet/AddGarnetTests.cs
There was a problem hiding this comment.
@davidfowl I need a little advice on this. The python virtual env layout varies across platforms, on Windows there's a scripts directory while on linux and mac there's a bin directory for the executables. I could add OS detection and assume that the scripts/binaries are in the right spot. But I felt that it would be nicer to actually check on disk if the binaries are there.
Because of this choice I figured it would make sense to have local facts with an actual python environment instead of writing pure unit-tests.
I can make purer unit-tests by either: Making the assumption about the virtual env layout and not checking the disk. Or I can write temp dummy files to disk to make the unit-test pass.
What do you prefer? Or do you have other ideas?
| string[] allowedExtensions = [".exe", ".cmd", ".bat", ""]; | ||
| string[] executableDirectories = ["bin", "Scripts"]; |
There was a problem hiding this comment.
Filter this based on operating system?
| /// <param name="executablePath">The path to the executable used to run the python project.</param> | ||
| /// <param name="projectDirectory">The path to the directory containing the python project.</param> | ||
| public class PythonProjectResource(string name, string executablePath, string projectDirectory) | ||
| : ExecutableResource(name, executablePath, projectDirectory), IResourceWithServiceDiscovery |
There was a problem hiding this comment.
Do we want this to implement IResourceWithServiceDiscovery?
wmeints
force-pushed
the
python-hosting-support
branch
from
0f772cc
to
30c850f
Compare
This PR introduces support for running Python projects as part of an Aspire solution. I took the NodeJS extension as inspiration but had to add a couple of extra pieces of logic since Python is a lot less standardized when it comes to how projects are structured.
Initially I created this project: https://github.com/wmeints/aspire-python but I feel that this PR is a much more useful way to add support for Python. I think it's a great resolution for #2760.
Assumptions made by the extension
.venv. Although you can change that.