-
Notifications
You must be signed in to change notification settings - Fork 765
[release/13.0] Generate fallback Dockerfile for Python apps without UV #12640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12640Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12640" |
- Modified PythonAppResourceBuilderExtensions to generate Dockerfiles for all Python apps - Split Dockerfile generation into two methods: GenerateUvDockerfile and GenerateFallbackDockerfile - Fallback Dockerfiles detect requirements.txt and run pip install if present - Uses same runtime image (python:X.Y-slim-bookworm) as UV workflow for consistency - Updated PythonVersionDetector to accept nullable VirtualEnvironment parameter - Added comprehensive tests for fallback Dockerfile generation Co-authored-by: davidfowl <[email protected]>
…move empty line Co-authored-by: davidfowl <[email protected]>
a616bfb to
809b552
Compare
|
|
||
| var pythonVersion = pythonEnvironmentAnnotation.Version ?? PythonVersionDetector.DetectVersion(appDirectory, pythonEnvironmentAnnotation.VirtualEnvironment!); | ||
| // Try to get Python environment annotation | ||
| context.Resource.TryGetLastAnnotation<PythonEnvironmentAnnotation>(out var pythonEnvironmentAnnotation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels weird to the discard the value of the TryGet here. It seems like we could still set the fallback Python version even if we did include the if-statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic that is here is what we want. Including an if on the TryGet would make the logic harder to read/follow IMO. The TryGet will output a null annotation if one isn't found, so doing the null checks seems like the easiest way.
Backport of #12627 to release/13.0
/cc @eerhardt @copilot
Customer Impact
Previously, Dockerfiles were only generated for Python apps using UV. Non-UV Python apps had no automatic containerization support.
Testing
Added new automated tests. Manually tested the
playground\python\instrumented_scriptapp produces a docker image successfully.Risk
Only affects Python apps, and only if they aren't using UV.
Regression?
No