[WIP] manage cluster capacity policy via KST #106
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces support for managing Kusto cluster capacity policies via KST by adding a new model, loader, and writer, and wiring them into the schema handler and database factory. It also updates JSON serialization to omit null values.
- Define
ClusterCapacityPolicymodel with JSON serialization and script generation - Implement loader (
ClusterCapacityPolicyLoader) and writer (ClusterCapacityPolicyWriter) - Wire capacity policy handling into
KustoDatabaseHandlerFactoryandKustoSchemaHandler
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Parser/KustoWriter/ClusterCapacityPolicyWriter.cs | Writes .alter-merge cluster policy capacity commands |
| Parser/KustoLoader/ClusterCapacityPolicyLoader.cs | Loads existing cluster capacity policy via .show query |
| Parser/KustoDatabaseHandlerFactory.cs | Adds ClusterCapacityPolicyLoader and writer to factory |
| Model/Clusters.cs | Adds CapacityPolicy property to clusters config |
| Model/ClusterCapacityPolicy.cs | Defines capacity policy model and script creation |
| KustoSchemaHandler.cs | Refactors handler to include capacity policy writer |
| Helpers/Serialization.cs | Ignores nulls in JSON serialization |
| Tests/DemoData/.../partial-cluster-policy.yml | Adds partial policy sample for tests |
| Tests/DemoData/.../comprehensive-cluster-policy.yml | Adds full policy sample for tests |
| KustoSchemaTools.Tests/ClusterCapacityPolicyTests.cs | Tests capacity policy loading, serialization, and script |
Comments suppressed due to low confidence (2)
KustoSchemaTools.Tests/ClusterCapacityPolicyTests.cs:1
- Missing
using Xunit;andusing System.IO;directives, which are required for the[Fact]attribute and forPath/Fileusage in this test file.
using KustoSchemaTools.Helpers;
KustoSchemaTools/Parser/KustoLoader/ClusterCapacityPolicyLoader.cs:24
- The deserialized
capacityPolicyis not stored on thedatabasemodel, so the existing policy won't be tracked for comparison. Consider assigning it todatabase.CapacityPolicy.
var capacityPolicy = JsonConvert.DeserializeObject<ClusterCapacityPolicy>(capacityPolicyData);
| private IDatabaseHandler<T> CreateDatabaseHandlerWithCapacityPolicy(string clusterUrl, string databaseName, ClusterCapacityPolicy? capacityPolicy) | ||
| { | ||
| // Create a logger - in a real application this would come from DI | ||
| var loggerFactory = LoggerFactory.Create(builder => { }); |
There was a problem hiding this comment.
[nitpick] Instantiating a new LoggerFactory on each call is expensive; consider reusing a shared ILoggerFactory or injecting it once to reduce allocations.
Copilot uses AI. Check for mistakes.
| // For now, we don't need to store it as we're only applying changes | ||
| } | ||
| } | ||
| catch (Exception) |
There was a problem hiding this comment.
[nitpick] Catching a broad Exception without any logging may hide underlying issues during load; consider logging a warning with exception details.
Copilot uses AI. Check for mistakes.
| return new KustoDatabaseHandlerFactory<T>(logger) | ||
| .WithReader<KustoDatabasePrincipalLoader>() |
There was a problem hiding this comment.
[nitpick] The reader/writer setup chain is duplicated between CreateDefault and CreateDatabaseHandlerWithCapacityPolicy; extract into a shared helper to adhere to DRY principles.
| return new KustoDatabaseHandlerFactory<T>(logger) | |
| .WithReader<KustoDatabasePrincipalLoader>() | |
| var factory = new KustoDatabaseHandlerFactory<T>(logger); | |
| factory.AddDefaultReadersAndWriters(); | |
| return factory.Create(cluster, database); | |
| } | |
| private void AddDefaultReadersAndWriters() | |
| { | |
| WithReader<KustoDatabasePrincipalLoader>() |
Copilot uses AI. Check for mistakes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var diagnostics = parsedScript.GetDiagnostics(); | ||
| if (diagnostics.Any()) | ||
| { | ||
| Console.WriteLine($"Generated script: {diagnostics[0]}"); |
Check warning
Code scanning / CodeQL
Use of default ToString() Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
To fix the issue, we need to ensure that the Diagnostic class (or its equivalent) overrides the ToString() method to provide a meaningful string representation of its contents. This could include details such as the diagnostic message, severity, and location. Since the Diagnostic class is not defined in the provided snippet, we will assume it is part of the Kusto.Language namespace or another imported library. If it is a custom class, we will need to locate and modify its definition.
If the Diagnostic class is part of an external library and cannot be modified directly, an alternative approach is to explicitly format the diagnostic information in the ToUpdateScript method before passing it to Console.WriteLine.
| @@ -79,3 +79,3 @@ | ||
| { | ||
| Console.WriteLine($"Generated script: {diagnostics[0]}"); | ||
| Console.WriteLine($"Generated script: {diagnostics[0].Message} (Severity: {diagnostics[0].Severity}, Location: {diagnostics[0].Location})"); | ||
|
|
No description provided.