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

Update Azure Sql to use new API pattern #6094

Merged
merged 3 commits into from
Oct 4, 2024
Merged

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Oct 4, 2024

Description

We are moving to a new API pattern for Azure resources that can also be used locally as a container.Update Azure Sql Server to follow the same pattern.

  • The existing [Publish]AsAzureSqlDatabase methods are marked Obsolete
  • New AddAzureSqlServer method is introduced for when you want an Azure resource.
  • To use a non-Azure container locally, a new RunAsContainer method is added.

Fix #5998

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
  • Does the change make any security assumptions or guarantees?
    • No
  • Does the change require an update in our Aspire docs?
Microsoft Reviewers: Open in CodeFlow

We are moving to a new API pattern for Azure resources that can also be used locally as a container.Update Azure Sql Server to follow the same pattern.

* The existing [Publish]AsAzureSqlDatabase methods are marked Obsolete
* New AddAzureSqlServer method is introduced for when you want an Azure resource.
* To use a non-Azure container locally, a new RunAsContainer method is added.

Fix dotnet#5998
Comment on lines +6 to +7
Aspire.Hosting.Azure.AzureSqlDatabaseResource.Parent.get -> Aspire.Hosting.Azure.AzureSqlServerResource!
Aspire.Hosting.Azure.AzureSqlServerResource.AzureSqlServerResource(string! name, System.Action<Aspire.Hosting.ResourceModuleConstruct!>! configureConstruct) -> void
Copy link
Member Author

Choose a reason for hiding this comment

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

@mitchdenny @davidfowl - the nominclature here is really hard. Is this what we want?

  • AzureSqlServerResource (existing) - The "Server" resource
  • AzureSqlDatabaseResource (new) - The child "database" resource.

Copy link
Member

Choose a reason for hiding this comment

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

The official service names don't help much :) I think AzureSqlDatabaseResource and AzureSqlServerResource is probably the best option.

[Experimental("AZPROVISION001", UrlFormat = "https://aka.ms/dotnet/aspire/diagnostics#{0}")]
public static IResourceBuilder<SqlServerServerResource> AsAzureSqlDatabase(this IResourceBuilder<SqlServerServerResource> builder, Action<IResourceBuilder<AzureSqlServerResource>, ResourceModuleConstruct, SqlServer, IEnumerable<SqlDatabase>>? configureResource)
{
return builder.PublishAsAzureSqlDatabase(configureResource, useProvisioner: true);
}

/// <summary>
/// Adds an Azure SQL Database (server) resource to the application model.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is where the nominclature gets worse. Azure SQL Database is the name of the service (AFAICT). So this is the Azure SQL Database "server" resource.

I was following existing XML docs above here.

Copy link
Member

Choose a reason for hiding this comment

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

I think colloquially people call Azure SQL Database ... Azure SQL.

Copy link
Member

Choose a reason for hiding this comment

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

... to finish that thought. I think then it is fine to do AddAzureSqlServer(...) because effectively that is what you are doing ... creating a server resource. On the backend I seem to remember that Azure SQL Database Servers were somehow a virtual concept but things could have changed.

@eerhardt eerhardt merged commit 07f1f34 into dotnet:main Oct 4, 2024
9 checks passed
@eerhardt eerhardt deleted the Fix5998 branch October 4, 2024 20:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Azure Sql to use new API pattern
3 participants