[WIP] manage cluster capacity policy via KST #106

ashleyvansp · 2025-06-23T20:29:11Z

No description provided.

Copilot

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 ClusterCapacityPolicy model with JSON serialization and script generation
Implement loader (ClusterCapacityPolicyLoader) and writer (ClusterCapacityPolicyWriter)
Wire capacity policy handling into KustoDatabaseHandlerFactory and KustoSchemaHandler

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; and using System.IO; directives, which are required for the [Fact] attribute and for Path/File usage in this test file.

using KustoSchemaTools.Helpers;

KustoSchemaTools/Parser/KustoLoader/ClusterCapacityPolicyLoader.cs:24

The deserialized capacityPolicy is not stored on the database model, so the existing policy won't be tracked for comparison. Consider assigning it to database.CapacityPolicy.

                    var capacityPolicy = JsonConvert.DeserializeObject<ClusterCapacityPolicy>(capacityPolicyData);

Copilot · 2025-06-23T20:30:49Z

KustoSchemaTools/KustoSchemaHandler.cs

+        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 => { });


[nitpick] Instantiating a new LoggerFactory on each call is expensive; consider reusing a shared ILoggerFactory or injecting it once to reduce allocations.

Copilot · 2025-06-23T20:30:49Z

KustoSchemaTools/Parser/KustoLoader/ClusterCapacityPolicyLoader.cs

+                    // For now, we don't need to store it as we're only applying changes
+                }
+            }
+            catch (Exception)


[nitpick] Catching a broad Exception without any logging may hide underlying issues during load; consider logging a warning with exception details.

Copilot · 2025-06-23T20:30:49Z

KustoSchemaTools/Parser/KustoDatabaseHandlerFactory.cs

+            return new KustoDatabaseHandlerFactory<T>(logger)
+                .WithReader<KustoDatabasePrincipalLoader>()


[nitpick] The reader/writer setup chain is duplicated between CreateDefault and CreateDatabaseHandlerWithCapacityPolicy; extract into a shared helper to adhere to DRY principles.

Suggested change

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>()

KustoSchemaTools/Parser/KustoLoader/ClusterCapacityPolicyLoader.cs

KustoSchemaTools/KustoSchemaHandler.cs

KustoSchemaTools/KustoClusterOrchestrator.cs

KustoSchemaTools/Model/ClusterCapacityPolicy.cs

KustoSchemaTools.Tests/Changes/ClusterChangesTest.cs

KustoSchemaTools/KustoClusterOrchestrator.cs

KustoSchemaTools/Parser/KustoClusterHandler.cs

+            catch (System.Exception ex)
+            {
+                _logger.LogError(ex, "Failed to load cluster capacity policy.");
+            }


To fix the issue, replace the generic catch clause with specific exception types that are likely to occur during the execution of the ExecuteControlCommandAsync method. For example:

Catch Kusto.Data.Exceptions.KustoClientException for errors related to the Kusto client.

Catch JsonException for errors during JSON deserialization.

Add a fallback catch block for unexpected exceptions, if necessary, but avoid catching all exceptions indiscriminately.

This approach ensures that only expected errors are handled explicitly, while unexpected errors can propagate or be logged separately.

KustoSchemaTools/Parser/YamlClusterHandler.cs

…Combine Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

KustoSchemaTools.Tests/Parser/YamlClusterHandlerTests.cs

KustoSchemaTools.Tests/Changes/ClusterChangesTest.cs

+            Assert.NotNull(policyChange);
+            Assert.Equal(2, policyChange!.PropertyChanges.Count);
+
+            var ingestionChange = Assert.Single(policyChange.PropertyChanges, p => p.PropertyName == "IngestionCapacity");


To fix the issue, we will ensure that policyChange is not null before it is dereferenced on line 81. This can be achieved by adding a null-forgiving operator (!) to the dereference on line 81, as the Assert.NotNull(policyChange) on line 78 already guarantees that policyChange is not null. This approach maintains the existing functionality while addressing the static analysis warning.

KustoSchemaTools.Tests/KustoClusterOrchestratorTests.cs

+        public async Task GenerateChangesFromFileAsync_ValidYamlFile_ReturnsChanges()
+        {
+            // Arrange
+            var yamlFilePath = Path.Combine("DemoData", "ClusterScopedChanges", "multipleClusters.yml");


To fix the issue, replace the call to Path.Combine with Path.Join. The Path.Join method is designed to concatenate paths without dropping earlier arguments, even if a later argument is an absolute path. This ensures that the resulting path is always constructed as intended.

The change will be made in the GenerateChangesFromFileAsync_ValidYamlFile_ReturnsChanges test method, specifically on line 404. No additional imports or definitions are required, as Path.Join is part of the System.IO namespace already included in the file.

KustoSchemaTools.Tests/KustoClusterOrchestratorTests.cs

+        public async Task GenerateChangesFromFileAsync_VerifyLoggingCalled()
+        {
+            // Arrange
+            var yamlFilePath = Path.Combine("DemoData", "ClusterScopedChanges", "multipleClusters.yml");


To fix the issue, replace the call to Path.Combine with Path.Join. The Path.Join method is designed to concatenate paths without the risk of dropping earlier arguments if an absolute path is encountered. This change will ensure that the code behaves correctly regardless of whether the arguments are relative or absolute paths.

The change will be made on line 543 of the file KustoSchemaTools.Tests/KustoClusterOrchestratorTests.cs. No additional imports or dependencies are required, as Path.Join is part of the System.IO namespace.

KustoSchemaTools.Tests/KustoClusterOrchestratorTests.cs

+        public async Task GenerateChangesFromFileAsync_LoadAsyncThrowsException_PropagatesException()
+        {
+            // Arrange
+            var yamlFilePath = Path.Combine("DemoData", "ClusterScopedChanges", "multipleClusters.yml");


To fix the issue, replace the call to Path.Combine with Path.Join. The Path.Join method is a safer alternative for constructing file paths because it does not ignore earlier arguments when an absolute path is encountered. This change ensures that the code behaves correctly even if any of the arguments are modified to be absolute paths in the future.

The change will be made on line 610 of the file KustoSchemaTools.Tests/KustoClusterOrchestratorTests.cs. No additional imports or dependencies are required since Path.Join is part of the System.IO namespace.

ashleyvansp added 6 commits June 21, 2025 00:10

Add cluster capacity policy model

d511900

Add unit tests

f38ef03

First pass at writing capacity policy

f9d4433

Ignore nulls when serializing

407147e

Add clusters.yml test files for different capacity policy cases

aa036af

remove unused test files

3d7709d

Copilot AI review requested due to automatic review settings June 23, 2025 20:29

Copilot AI reviewed Jun 23, 2025

View reviewed changes

github-advanced-security bot found potential problems Jun 23, 2025

View reviewed changes

KustoSchemaTools/Parser/KustoLoader/ClusterCapacityPolicyLoader.cs Fixed Show fixed Hide fixed

ashleyvansp added 5 commits June 23, 2025 13:41

Merge branch 'main' into ashleyvansp/capacityPolicy

e67a422

Separate generic library functionality from app-specific implementation

ff73111

remove whitespace

e0d5028

fix cluster model

238625f

new cluster orchestrator

cb269f4

github-advanced-security bot found potential problems Jun 25, 2025

View reviewed changes

KustoSchemaTools/KustoSchemaHandler.cs Fixed Show fixed Hide fixed

fix some errors

9c11c41

github-advanced-security bot found potential problems Jun 25, 2025

View reviewed changes

KustoSchemaTools/KustoClusterOrchestrator.cs Fixed Show fixed Hide fixed

ashleyvansp added 12 commits June 25, 2025 15:11

fix build

e0cfb77

unit test data

e67e3fc

rename

15ac317

cluster changes

3e29698

add ;

e6ba13f

small fix

1d11d5b

fix property

5dc3cd2

build succeeds

b24d5a1

move YamlDatabaseParserTests

28e23ce

make model comparable and add first unit test

5bb8be2

undo schema handler change

5626175

genericize the capacity policy diff

82e59c9

github-advanced-security bot found potential problems Jun 26, 2025

View reviewed changes

KustoSchemaTools/Model/ClusterCapacityPolicy.cs Fixed Show fixed Hide fixed

Finish test for generate changes

0093e1f

github-advanced-security bot found potential problems Jun 30, 2025

View reviewed changes

ashleyvansp and others added 6 commits June 30, 2025 08:16

revert serialization change

f48e441

Potential fix for code scanning alert no. 36: Call to System.IO.Path.…

0585bc7

…Combine Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Orchestrator tests

e1fe5b5

yaml cluster handler

59986f1

yaml cluster handler and test

be9c073

fix test path

48fd778

github-advanced-security bot found potential problems Jun 30, 2025

View reviewed changes

KustoSchemaTools.Tests/Parser/YamlClusterHandlerTests.cs Fixed Show fixed Hide fixed

handler changes

2af167b

github-advanced-security bot found potential problems Jul 1, 2025

View reviewed changes

add GenerateChangesFromFileAsync

60b6965

github-advanced-security bot found potential problems Jul 1, 2025

View reviewed changes

@@ -47,5 +47,13 @@
                         }
-                        catch (System.Exception ex)
+                        catch (Kusto.Data.Exceptions.KustoClientException ex)
                         {
-                            _logger.LogError(ex, "Failed to load cluster capacity policy.");
+                            _logger.LogError(ex, "Kusto client error occurred while loading cluster capacity policy.");
+                        }
+                        catch (JsonException ex)
+                        {
+                            _logger.LogError(ex, "JSON deserialization error occurred while processing cluster capacity policy.");
+                        }
+                        catch (Exception ex)
+                        {
+                            _logger.LogError(ex, "An unexpected error occurred while loading cluster capacity policy.");
                         }

@@ -80,3 +80,3 @@
-                        var ingestionChange = Assert.Single(policyChange.PropertyChanges, p => p.PropertyName == "IngestionCapacity");
+                        var ingestionChange = Assert.Single(policyChange!.PropertyChanges, p => p.PropertyName == "IngestionCapacity");
                         Assert.Equal("{\"CoreUtilizationCoefficient\":0.75}", ingestionChange.OldValue);

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] manage cluster capacity policy via KST #106

[WIP] manage cluster capacity policy via KST #106

ashleyvansp commented Jun 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 23, 2025

Uh oh!

Copilot AI Jun 23, 2025

Uh oh!

Copilot AI Jun 23, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Check warning

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Uh oh!

		return new KustoDatabaseHandlerFactory<T>(logger)
		.WithReader<KustoDatabasePrincipalLoader>()

-            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>()

[WIP] manage cluster capacity policy via KST #106

Are you sure you want to change the base?

[WIP] manage cluster capacity policy via KST #106

Conversation

ashleyvansp commented Jun 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Uh oh!

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Uh oh!