-
Notifications
You must be signed in to change notification settings - Fork 670
Changing default SKU for App Service Plan to P0V3 #9280
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
Changing default SKU for App Service Plan to P0V3 #9280
Conversation
src/Aspire.Hosting.Azure.AppService/AzureAppServiceWebsiteContext.cs
Outdated
Show resolved
Hide resolved
@@ -214,7 +214,7 @@ public void BuildWebSite(AzureResourceInfrastructure infra) | |||
AppServicePlanId = appServicePlanParameter, | |||
SiteConfig = new SiteConfigProperties() | |||
{ | |||
LinuxFxVersion = BicepFunction.Interpolate($"DOCKER|{containerImage}"), | |||
LinuxFxVersion = "SITECONTAINERS", |
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.
Confused by this change. Why do we need sidecars? Or is this just the new way of using containers?
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.
yes this is what we want to do going fwd instead of overloading the linuxFX version. There is no sidecar being configured here
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.
Why is the port hardcoded to 8080?
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.
Removed setting the port as it is inferred by the platform. Validated with the changes in fork code
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.
Doesn't this need to be set to the internal port of the http endpoint?
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.
When TargetPort is not specified, our container orchestrator can infer the exposed PORT in the container image and use it for serving the http traffic. So, explicitly mentioning TargetPort is not required.
@dotnet-policy-service agree company="Microsoft" |
Name = "P0V3", | ||
Tier = "Premium" |
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'm having second thoughts about this one.
/backport to release/9.3 |
Started backporting to release/9.3: https://github.com/dotnet/aspire/actions/runs/15047478952 |
Description
Fixes # (issue)
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template