Make it easier to choose Azure SQL SKU #8887

yorek · 2025-04-19T19:08:21Z

Description

Using the Azure SQL DB Free Offer as default deployment option. Added also the ability to specify the desired SKU.

Fixes # (issue)

#8876

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
    - No
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
    - No
- No
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
    - No
- No
Does the change require an update in our Aspire docs?
- Yes
  - Link to aspire-docs issue (consider using one of the following templates):
- No

yorek · 2025-04-19T19:09:39Z

@dotnet-policy-service agree company="Microsoft"

davidfowl · 2025-04-19T21:09:28Z

You made a breaking API change

yorek · 2025-04-20T02:13:39Z

You made a breaking API change

I added a new optional parameter to AddDatabase, is that considered a breaking change?

I guess, otherwise, you are referring to the change to the AzureSqlDatabaseResource that now requires the SKU, but this is used only in the AddDatabase method, which still allows for the same syntax as before this change.

Let me know what you think should be changed

davidfowl · 2025-04-20T03:02:54Z

You can’t add a new required parameters to the constructor. That’s why I suggested a WithSku method. The sku is optional

yorek · 2025-04-20T15:36:53Z

You can’t add a new required parameters to the constructor. That’s why I suggested a WithSku method. The sku is optional

Ok. I checked where the constructor was used and it was used only in the AddDatabase so I thought it would have been fine, as it seems more natural to me to specify which SKU do you want when you call the AddDatabase method instead of having to discover and learn that there is another method to use to set the SKU.

I'll do the change in the next days it shouldn't take long

davidfowl · 2025-04-20T20:59:02Z

You can do it there but it’s still optional on the database itself

yorek · 2025-04-21T00:45:29Z

Done, it was easier that I thought :) Let me know if this is better.

src/Aspire.Hosting.Azure.Sql/AzureSqlDatabaseResource.cs

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

src/Aspire.Hosting.Azure.Sql/AzureSqlServerResource.cs

eerhardt · 2025-04-21T23:28:35Z

This is looking pretty good.

Can you open a breaking change doc issue: New breaking-change template, since we are changing the default SKU here. We should have it in our docs that this change happened.

yorek · 2025-04-22T00:47:00Z

This is looking pretty good.

Can you open a breaking change doc issue: New breaking-change template, since we are changing the default SKU here. We should have it in our docs that this change happened.

Done: dotnet/docs-aspire#3144

davidfowl · 2025-04-22T01:32:06Z

src/Aspire.Hosting.Azure.Sql/AzureSqlDatabaseResource.cs

+    /// <summary>
+    /// Gets or Sets the database SKU name
+    /// </summary>
+    internal string SkuName


This should be public

I made the get public and kept the set internal, as we want to make sure developers use the WithSku method to set the sku, right?

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

davidfowl · 2025-04-22T01:34:18Z

Shouldn’t we be exposing the cdk type?

eerhardt · 2025-04-22T14:33:41Z

Shouldn’t we be exposing the cdk type?

What does this mean, specifically?

We have a ConfigureInfrastructure on the Sql server resource, where you can get the child cdk objects for the SqlDatabase, and manipulate them.

Co-authored-by: David Fowler <[email protected]>

…o azuresqldb-8876

davidfowl · 2025-04-23T01:49:42Z

I believe the underlying Azure.Provisioning is a SqlSku type

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

eerhardt

@sebastienros - any other thoughts?

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

src/Aspire.Hosting.Azure.Sql/README.md

eerhardt · 2025-04-24T16:02:54Z

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

+                Name = databaseName,
+            };
+
+            sqlDatabase.Sku = new SqlSku() { Name = AzureSqlDatabaseResource.FREE_DB_SKU };


The code that calls into this method is obsolete. I don't think we should change its behavior. We should be deleting the API when we move to a new major version. So let's leave this path using the same behavior as before.

So should just I mark it as obsolete and preserve the previous behavior (which means no SKU will be explicitly specified and whatever is the default for Azure SQL at that moment will be used)?

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

sebastienros · 2025-04-24T21:30:22Z

src/Aspire.Hosting.Azure.Sql/AzureSqlServerResource.cs

+    /// <summary>
+    /// A dictionary where the key is the resource name and the value is the Azure SQL database name.
+    /// </summary>
+    public IReadOnlyDictionary<string, string> Databases => _databases.ToDictionary(


@eerhardt should this be obsolete too? Or just track that this should be removed in 10.0?

We shouldn't remove APIs without obsoleting them for a release first.

If we don't think we want this API anymore, we can obsolete it. I'd be fine with that.

We shouldn't remove APIs without obsoleting them for a release first.

Really? Meaning we won't remove anything in 10.0 (or at least try) if we haven't obsoleted it in 9.3 or anything before 10?

That's my opinion, yes. I'm sure there will be cases where we feel we have to make a API breaking change without obsoleting first. But I think we should try to obsolete for a release first, before removing APIs.

Thoughts @DamianEdwards @davidfowl @mitchdenny ?

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

src/Aspire.Hosting.Azure.Sql/README.md

davidfowl · 2025-04-24T22:01:34Z

@eerhardt make sure we expose Sqlsku and not a string (see primitive obsession)

Co-authored-by: Eric Erhardt <[email protected]>

Co-authored-by: Sébastien Ros <[email protected]>

yorek · 2025-04-26T16:18:51Z

@eerhardt make sure we expose Sqlsku and not a string (see primitive obsession)

If that is needed, then adding WithSku is not enough, as there is no "free" Sku, but there is a free offer that can be applied to the database, and that will use a specific Sku. I propose to introduce a new WithFreeOffer method that can be used as an alternative to WithSku. The new method will set the correct Sku to be used with the free offer and enable the free option.

For example:

var db = sql.AddDatabase("db", "dbName").WithFreeOffer();
Assert.Equal("GP_S_Gen5_2", db.Sku.Name);

using WithSku and WithFreeOffer will result in an exception if the WithSku try to set the sku to something different then GP_S_Gen5_2 (as this is the one used by the Free Offer)

Thoughts?

sebastienros · 2025-04-29T17:34:04Z

What about only WithSku() and use a constant for the free/basic/... offers that return the correct SqlSku?

yorek · 2025-04-29T17:56:14Z

What about only WithSku() and use a constant for the free/basic/... offers that return the correct SqlSku?

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.

davidfowl · 2025-04-29T17:57:46Z

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.

var db = sql.AddDatabase("db", "dbName").WithSku(SqlSku.FreeOffer);

yorek · 2025-04-29T17:59:31Z

Not sure I follow. There is no "free" Sku...you can set the free offer option and then the Sku will be set automatically to "GP_S_Gen5_2"...but you can set that Sku even without benefitting from the free offer.
var db = sql.AddDatabase("db", "dbName").WithSku(SkuName.FreeOffer);

But then when the SqlSku returned would be the "GP_S_Gen5_2", with no information if that is a free offer or not. So if we go that route then a IsFreeOfferEnabled property should be added IMHO.

yorek · 2025-04-29T18:00:34Z

For example:

var db = sql.AddDatabase("db", "dbName").WithSku(SkuName.FreeOffer);
Assert.Equal("GP_S_Gen5_2", db.Sku.Name);
Assert.Equal(true, db.IsFreeOfferEnabled);

yorek · 2025-05-02T20:53:05Z

@davidfowl @sebastienros
Any feedback on this proposal?

sebastienros · 2025-05-06T17:58:36Z

@davidfowl @eerhardt @yorek

Here is a thought after looking at it again.

The free offer can't be defined by setting only a SqlSku property since it requires to also set these:

sqlDatabase.Sku = new SqlSku() { Name = "GP_S_Gen5_2" };
sqlDatabase.UseFreeLimit = true;
sqlDatabase.FreeLimitExhaustionBehavior = FreeLimitExhaustionBehavior.AutoPause;

This means we can't just expose a SqlSku property as @davidfowl is asking.
We can however expose WithFreeOffer() but since we want this to be the default I don't see why we should have this method.

My suggestion is to just configure databases to be free by default by setting the correct bicep properties, and if someone wants a custom SqlSku they should use ConfigureInfrastructure to change the values, like it's done for any other Azure resource.

Optionally, if we decide to change the default to non-Bicep defaults, I would also suggest creating a WithDefaultSku() instead that would reset the "free" to whatever is the default provisioning is. This will provide an easy way for users to get the same behavior as 9.2 and earlier if they had to.

eerhardt · 2025-05-06T20:23:17Z

This means we can't just expose a SqlSku property as @davidfowl is asking.
We can however expose WithFreeOffer() but since we want this to be the default I don't see why we should have this method.
My suggestion is to just configure databases to be free by default by setting the correct bicep properties, and if someone wants a custom SqlSku they should use ConfigureInfrastructure to change the values, like it's done for any other Azure resource.

I agree with this. We don't expose a "WithSku" helper conveinence method on any other Azure resource.

Optionally, if we decide to change the default to non-Bicep defaults, I would also suggest creating a WithDefaultSku() instead that would reset the "free" to whatever is the default provisioning is. This will provide an easy way for users to get the same behavior as 9.2 and earlier if they had to.

I can see the value in having a method like this. My only comment is that I wouldn't want the method to be named WithDefaultSku() because it is confusing. We are defaulting the Sku to "free", so that is what people would consider the default.

Maybe if we called it WithDefaultAzureSku() and when that is called, we left the SKU unset in bicep - then you get the default from Azure.

yorek · 2025-05-07T15:57:01Z

So, to summarize:

No WithSku() method
When creating the Azure SQL resource, the Aspire Azure SQL resource will deploy the free Azure SQL DB offer by default
A new method WithDefaultAzureSku() allows developer to resort to the default Azure SKU (basically, it omits completely the SKU specification in the generated bicep file)

Is that correct, and all agree that this is the way to go?

eerhardt · 2025-05-07T16:40:44Z

Is that correct, and all agree that this is the way to go?

That seems to be the best approach to me.

yorek · 2025-05-07T20:00:59Z

I made the changes, but while doing it I realized that having a WithSku is really just better. With that you can define the sku for each database, so you can have a server with 3 databases, each with a different sku. When using ConfigureInfrastructure instead, you do it at the server level (AFAIK), which means all databases in that server will have the defined SKU.

src/Aspire.Hosting.Azure.Sql/AzureSqlDatabaseResource.cs

sebastienros · 2025-05-07T20:03:34Z

When using ConfigureInfrastructure instead, you do it at the server level (AFAIK), which means all databases in that server will have the defined SKU.

You can pick which database you want to alter

var sql2 = builder.AddAzureSqlServer("sql2")
        .ConfigureInfrastructure(c =>
        {
            const string FREE_DB_SKU = "GP_S_Gen5_2";

            foreach (var database in c.GetProvisionableResources().OfType<SqlDatabase>())
            {
                database.Sku = new SqlSku() { Name = FREE_DB_SKU };
            }
        });

first pass at dotnet#8876

5ad0d72

github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Apr 19, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 19, 2025

yorek mentioned this pull request Apr 19, 2025

Make it easier to choose Azure SQL SKU #8876

Open

1 task

davidfowl requested a review from eerhardt April 19, 2025 21:11

introduced WithSku to manage database SKU

c4d6734

eerhardt reviewed Apr 21, 2025

View reviewed changes

eerhardt requested a review from sebastienros April 21, 2025 15:14

yorek added 3 commits April 21, 2025 08:53

Improved sku comparison code

d2257c1

changes as per comments

ff1fa57

formatting improvements

4202efa

davidfowl reviewed Apr 22, 2025

View reviewed changes

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs Outdated Show resolved Hide resolved

yorek and others added 3 commits April 22, 2025 07:59

Update src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

d090962

Co-authored-by: David Fowler <[email protected]>

get is now public, added test

9acc9ad

Merge branch 'azuresqldb-8876' of https://github.com/yorek/aspire int…

177b074

…o azuresqldb-8876

RussKie changed the title ~~first pass at #8876~~ Make it easier to choose Azure SQL SKU Apr 22, 2025

IEvangelist reviewed Apr 23, 2025

View reviewed changes

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs Outdated Show resolved Hide resolved

eerhardt reviewed Apr 24, 2025

View reviewed changes

Move private member up in the class

73d5043

sebastienros reviewed Apr 24, 2025

View reviewed changes

src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs Outdated Show resolved Hide resolved

sebastienros reviewed Apr 24, 2025

View reviewed changes

src/Aspire.Hosting.Azure.Sql/README.md Outdated Show resolved Hide resolved

yorek and others added 5 commits April 26, 2025 09:00

Update src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

e0d27f7

Co-authored-by: Eric Erhardt <[email protected]>

Update src/Aspire.Hosting.Azure.Sql/README.md

d63a90c

Co-authored-by: Eric Erhardt <[email protected]>

Update src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

56acbd7

Co-authored-by: Eric Erhardt <[email protected]>

Update src/Aspire.Hosting.Azure.Sql/AzureSqlExtensions.cs

93847b8

Co-authored-by: Sébastien Ros <[email protected]>

Update src/Aspire.Hosting.Azure.Sql/README.md

d5b3c65

Co-authored-by: Sébastien Ros <[email protected]>

Fix missing xml tag

a0119a3

yorek added 3 commits May 7, 2025 12:03

Merge branch 'dotnet:main' into azuresqldb-8876

ee95da2

removed WithSku and added WithDefaultAzureSku

9ff69c1

Updated readme

d99070d

sebastienros reviewed May 7, 2025

View reviewed changes

src/Aspire.Hosting.Azure.Sql/AzureSqlDatabaseResource.cs Outdated Show resolved Hide resolved

Made UseDefaultAzureSku all internal

76e5648

Make it easier to choose Azure SQL SKU #8887

Are you sure you want to change the base?

Make it easier to choose Azure SQL SKU #8887

Conversation

yorek commented Apr 19, 2025 • edited Loading

Description

Checklist

yorek commented Apr 19, 2025

davidfowl commented Apr 19, 2025

yorek commented Apr 20, 2025 • edited Loading

davidfowl commented Apr 20, 2025

yorek commented Apr 20, 2025

davidfowl commented Apr 20, 2025

yorek commented Apr 21, 2025

eerhardt commented Apr 21, 2025

yorek commented Apr 22, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl commented Apr 22, 2025

eerhardt commented Apr 22, 2025

davidfowl commented Apr 23, 2025

eerhardt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl commented Apr 24, 2025

yorek commented Apr 26, 2025

sebastienros commented Apr 29, 2025

yorek commented Apr 29, 2025 • edited Loading

davidfowl commented Apr 29, 2025 • edited Loading

yorek commented Apr 29, 2025 • edited Loading

yorek commented Apr 29, 2025

yorek commented May 2, 2025

sebastienros commented May 6, 2025

eerhardt commented May 6, 2025

yorek commented May 7, 2025

eerhardt commented May 7, 2025

yorek commented May 7, 2025

sebastienros commented May 7, 2025

yorek commented Apr 19, 2025 •

edited

Loading

yorek commented Apr 20, 2025 •

edited

Loading

yorek commented Apr 29, 2025 •

edited

Loading

davidfowl commented Apr 29, 2025 •

edited

Loading

yorek commented Apr 29, 2025 •

edited

Loading