-
Notifications
You must be signed in to change notification settings - Fork 672
Add initial version of Aspire.Hosting.Azure.Kusto #10935
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
base: main
Are you sure you want to change the base?
Conversation
This is my first non-trivial contribution to Aspire, so I'm sure I've done some part of the process incorrectly. Any guidance is greatly appreciated. Thanks! |
Co-authored-by: Copilot <[email protected]>
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.
This is a good first step but there are a few things that will need to be done before we could accept this. We would want this package to be named Aspire.Hosting.Azure.Kusto
since Kusto is realistically an Azure resource - and when you do AddKusto(...)
you would want it to deploy an actual Kusto cluster (unless you added RunAsEmulator(...)). This means we'll need to ccordinate with the Azure SDK team on getting an Azure.Provisioning package set up for Kusto.
We need to add database support as well. Also what can we do about client integration packages here. Kusto has client libraries too - we probably need to figure out how to take them and plug them into the Aspire model.
/// <param name="builder">The <see cref="IDistributedApplicationBuilder"/>.</param> | ||
/// <param name="name">The name of the resource. This name will be used as the connection string name when referenced in a dependency.</param> | ||
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns> | ||
public static IResourceBuilder<KustoResource> AddKusto(this IDistributedApplicationBuilder builder, [ResourceName] string name = "kusto") |
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.
We have this method AddKusto(...)
which creates a Kusto resource, and then an extension method RunAsEmulator(...)
. What is the plan if they want to use a real Kusto resource deployed in Azure? We would need to add support for deploying the Kusto cluster here I think to match the pattern for other Azure based resources.
We'll probably need to work with the Azure SDK team to get the Azure.Provisioning
libraries expanded to cover Kusto clusters.
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.
Agreed. I modelled the resources so that we could add Azure Provisioning once it exists. Until then, is there a way to ship, even perhaps as a preview?
My hope is to allow folks to start using the emulator to allow for tests and local dev, while working / waiting on the other pieces.
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.
Let me know how best to allow for working incrementally and in the open, while also following the required process 🙂
@tg-msft sent you a message about this PR. We are looking for |
@mitchdenny That would be @ArcturusZhang |
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
Introduces initial support for Azure Kusto (Azure Data Explorer) as an Aspire Hosting integration. This PR adds the foundational components to run Kusto as an emulator using the Kustainer container for local development, with database creation capabilities.
Key changes:
- Adds new
Aspire.Hosting.Azure.Kusto
library with server and database resources - Implements emulator support using the Kustainer container
- Includes health checks and database creation capabilities
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/Aspire.Hosting.Azure.Kusto/Aspire.Hosting.Azure.Kusto.csproj | Project file for the new Azure Kusto hosting library |
src/Aspire.Hosting.Azure.Kusto/KustoServerResource.cs | Core server resource definition with connection string support |
src/Aspire.Hosting.Azure.Kusto/KustoDatabaseResource.cs | Database resource with parent-child relationship |
src/Aspire.Hosting.Azure.Kusto/KustoResourceBuilderExtensions.cs | Extension methods for adding Kusto resources and emulator configuration |
src/Aspire.Hosting.Azure.Kusto/KustoEmulatorResource.cs | Emulator-specific resource wrapper for container deployment |
src/Aspire.Hosting.Azure.Kusto/KustoHealthCheck.cs | Health check implementation for Kusto connectivity |
tests/Aspire.Hosting.Azure.Kusto.Tests/*.cs | Comprehensive test coverage including functional and API tests |
Directory.Packages.props | Adds Microsoft.Azure.Kusto.Data package dependency |
Aspire.slnx | Solution file updates to include new projects |
We should create a playground in /playground for this integration which includes usage of client libraries to ingest and query data from the emulator. |
|
||
return resourceBuilder | ||
.WithHealthCheck(healthCheckKey) | ||
.ExcludeFromManifest(); |
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.
/cc @mitchdenny flagging here as a reminder if we want this to happen automatically, or if we want users to have to specify this in the AppHost.
Added a skeleton playground that just shows getting an admin and query client. Will update to add some sample ingestion and queries tomorrow. |
|
||
var builder = DistributedApplication.CreateBuilder(args); | ||
|
||
var kusto = builder.AddKusto("kusto") |
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.
For consistency we'll need to rename this to AddAzureKusto(...)
/// </param> | ||
public KustoCreateDatabaseScriptAnnotation(string script) | ||
{ | ||
Script = script; |
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.
ArgumentNullException.ThrowIfNull(script);
/// The tag for the Kusto emulator container image. | ||
/// </summary> | ||
/// <remarks>latest</remarks> | ||
public static string Tag { get; } = "latest"; |
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.
Will need to find out from the Kusto team whether it is better to use latest here or some other tag. Generally with emulators we've found teams like the use latest because it aligns to service versions (although not perfectly sometimes).
/// <summary> | ||
/// A resource that represents a Kusto cluster. | ||
/// </summary> | ||
public class KustoServerResource : Resource, IResourceWithConnectionString, IResourceWithEndpoints |
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.
We should rename this to AzureKustoClusterResource
, and it will need to derive from AzureProvisioningResource
at some point. For the database resource we should rename it to AzureKustoDatabaseResource
.
/// <param name="builder">The <see cref="IDistributedApplicationBuilder"/>.</param> | ||
/// <param name="name">The name of the resource. This name will be used as the connection string name when referenced in a dependency.</param> | ||
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns> | ||
public static IResourceBuilder<KustoServerResource> AddKusto(this IDistributedApplicationBuilder builder, [ResourceName] string name) |
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.
Maybe AddAzureKustoCluster
....
Description
This pull request introduces initial support for Kusto (Azure Data Explorer) as a Aspire Hosting integration. It adds a new
Aspire.Hosting.Kusto
library, and wires up the Kustainer emulator to allow for local development and testing. The resource is currently always excluded from the manifest as deployment is not yet supported.Contributes to #8233
Checklist
<remarks />
and<code />
elements on your triple slash comments?doc-idea
templatebreaking-change
templatediagnostic
template