Skip to content
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

Add Uvicorn Integration #219

Merged
merged 24 commits into from
Nov 13, 2024
Merged

Add Uvicorn Integration #219

merged 24 commits into from
Nov 13, 2024

Conversation

tommasodotNET
Copy link
Contributor

**Closes #207 *

Adds Uvicorn integration. The logic is the same as for official Python integration but it uses uvicorn to run the app.

var uvicorn = builder.AddUvicornApp("uvicornapp", "../uvicornapp-api", "main:app")
    .WithHttpEndpoint(env: "UVICORN_PORT");

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

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

What's your thought on rather than this being a standalone Uvicorn hosting package, that we make it an extensions package like the Node.js one in the repo that adds Uvicorn?

This would mean that if we want to add additional things, like venv setup, pip installation, other Python runtimes, etc., they don't have to be in their own hosting packages.

Copy link
Member

Choose a reason for hiding this comment

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

Can we integrate OTEL into here to show how you can do that end-to-end tracing

@tommasodotNET
Copy link
Contributor Author

I agree. Maybe we should structure this as we did with NodeJS Extensions. Especially considering we also have this #221

@aaronpowell
Copy link
Member

I agree. Maybe we should structure this as we did with NodeJS Extensions. Especially considering we also have this #221

That's exactly where my thinking was going 😉

@aaronpowell aaronpowell self-requested a review November 13, 2024 04:21
Copy link
Member

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

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

Couple of small docs tweeks @tommasodotNET

But make sure you review the moving around of files and everything

@aaronpowell
Copy link
Member

@tommasodotNET I'm working on using the PythonAppResource as the base type, but it's marked as experimental (https://learn.microsoft.com/en-gb/dotnet/aspire/diagnostics/overview#ASPIREHOSTINGPYTHON001) so we'll have to use that too.

Copy link

Code Coverage

Package Line Rate Branch Rate Complexity Health
CommunityToolkit.Aspire.Hosting.Azure.DataApiBuilder 100% 100% 6
CommunityToolkit.Aspire.Hosting.Azure.StaticWebApps 100% 100% 28
CommunityToolkit.Aspire.Hosting.Deno 84% 75% 72
CommunityToolkit.Aspire.Hosting.Golang 94% 50% 16
CommunityToolkit.Aspire.Hosting.Java 98% 71% 58
CommunityToolkit.Aspire.Hosting.Meilisearch 61% 27% 94
CommunityToolkit.Aspire.Hosting.NodeJS.Extensions 90% 68% 92
CommunityToolkit.Aspire.Hosting.Ollama 64% 60% 222
CommunityToolkit.Aspire.Hosting.Python.Extensions 67% 46% 56
CommunityToolkit.Aspire.Hosting.Rust 94% 83% 16
CommunityToolkit.Aspire.Hosting.SqlDatabaseProjects 69% 60% 50
CommunityToolkit.Aspire.Meilisearch 97% 92% 68
CommunityToolkit.Aspire.OllamaSharp 84% 77% 76
Summary 77% (1666 / 2166) 62% (411 / 660) 854

Minimum allowed line rate is 60%

@aaronpowell aaronpowell merged commit 5653101 into main Nov 13, 2024
10 checks passed
@aaronpowell aaronpowell deleted the 207-uvicorn branch November 13, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uvicorn integration
3 participants