[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

+            var diagnostics = parsedScript.GetDiagnostics();
+            if (diagnostics.Any())
+            {
+                Console.WriteLine($"Generated script: {diagnostics[0]}");


To fix the issue, the Diagnostic class (or its base class) should override the ToString() method to provide a meaningful string representation of diagnostic information. This could include details such as the diagnostic message, severity, and location. Since the definition of the Diagnostic class is not provided, we assume it is part of the Kusto.Language namespace or another external library. If modifying the Diagnostic class is not possible, an alternative is to explicitly format the diagnostic information in the ToUpdateScript method before printing it.

KustoSchemaTools.Tests/Changes/ClusterChangesTest.cs

+
+            // Asserts that there is exactly one property change detected.
+            // Because a nested property changed, the top-level property containing it is marked as changed.
+            var propertyChange = Assert.Single(policyChange.PropertyChanges);


To fix the issue, we need to ensure that policyChange is not null before accessing its PropertyChanges property. This can be achieved by adding a null check after the cast. If policyChange is null, the test should fail with an appropriate assertion message, as this would indicate an unexpected test setup or behavior.

KustoSchemaTools.Tests/Changes/ClusterChangesTest.cs

+            // Assert
+            var policyChange = Assert.Single(changeSet.Changes) as PolicyChange<ClusterCapacityPolicy>;
+            Assert.NotNull(policyChange);
+            Assert.Equal(2, policyChange.PropertyChanges.Count);


To fix the issue, we need to ensure that policyChange is not null before dereferencing it. This can be achieved by adding an explicit null check after the assignment. If policyChange is null, the test should fail with a meaningful assertion message. This approach ensures that the test does not proceed with invalid data and avoids a potential NullReferenceException.

The changes will be made in the GenerateChanges_WithMultiplePropertyChanges_ShouldDetectAllChanges test method in the file 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, we will replace the generic catch (System.Exception ex) block with specific exception types that are relevant to the operation being performed. In this case:

The ExecuteControlCommandAsync method might throw exceptions related to Kusto client operations, such as Kusto.Data.Exceptions.KustoClientException or Kusto.Data.Exceptions.KustoServiceException.

The JsonConvert.DeserializeObject method might throw a JsonException if the JSON is invalid.

We will add separate catch blocks for these exceptions and log appropriate error messages for each. Any other unexpected exceptions will not be caught and will propagate, ensuring they are not silently ignored.

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

+        public async Task LoadAsync_ValidYamlFile_ReturnsClustersList()
+        {
+            // Arrange
+            var testFilePath = 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 treating absolute paths differently, making it a safer alternative in this context. The change will involve modifying line 17 to use Path.Join instead of Path.Combine. No additional imports or dependencies are required, as 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

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

@@ -79,4 +79,4 @@
                         {
-                            Console.WriteLine($"Generated script: {diagnostics[0]}");
+                            var diagnosticMessage = diagnostics[0].ToString() ?? "Unknown diagnostic";
+                            Console.WriteLine($"Generated script: {diagnosticMessage}");
                         }

@@ -51,2 +51,6 @@
                         Assert.NotNull(policyChange);
+                        if (policyChange == null)
+                        {
+                            Assert.Fail("The change in changeSet.Changes is not of type PolicyChange<ClusterCapacityPolicy>.");
+                        }

@@ -78,2 +78,6 @@
                         Assert.NotNull(policyChange);
+                        if (policyChange == null)
+                        {
+                            Assert.Fail("Expected a PolicyChange<ClusterCapacityPolicy> but got null.");
+                        }
                         Assert.Equal(2, policyChange.PropertyChanges.Count);

@@ -41,5 +41,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 (Kusto.Data.Exceptions.KustoServiceException ex)
+                        {
+                            _logger.LogError(ex, "Kusto service error occurred while loading cluster capacity policy.");
+                        }
+                        catch (Newtonsoft.Json.JsonException ex)
+                        {
+                            _logger.LogError(ex, "JSON deserialization error occurred while parsing cluster capacity policy.");
                         }

@@ -16,3 +16,3 @@
                         // Arrange
-                        var testFilePath = Path.Combine("DemoData", "ClusterScopedChanges", "multipleClusters.yml");
+                        var testFilePath = Path.Join("DemoData", "ClusterScopedChanges", "multipleClusters.yml");
                         var handler = new YamlClusterHandler(testFilePath);

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

Uh oh!

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!

Check warning

Copilot Autofix

Check warning

Copilot Autofix

Check warning

Copilot Autofix

Uh oh!

Check notice

Copilot Autofix

Uh oh!

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

Uh oh!

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!

Check warning

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Uh oh!

Check notice

Copilot Autofix

Uh oh!

Check notice

Copilot Autofix

Uh oh!