diff --git a/csharp/src/DatabricksParameters.cs b/csharp/src/DatabricksParameters.cs index 90ae9b47..b4587ec2 100644 --- a/csharp/src/DatabricksParameters.cs +++ b/csharp/src/DatabricksParameters.cs @@ -360,7 +360,7 @@ public class DatabricksParameters : SparkParameters /// /// Whether to enable the feature flag cache for fetching remote configuration from the server. /// When enabled, the driver fetches feature flags from the Databricks server and merges them with local properties. - /// Default value is true if not specified. + /// Default value is false if not specified. /// public const string FeatureFlagCacheEnabled = "adbc.databricks.feature_flag_cache_enabled"; diff --git a/csharp/src/FeatureFlagCache.cs b/csharp/src/FeatureFlagCache.cs index 264cb71a..d59c61fc 100644 --- a/csharp/src/FeatureFlagCache.cs +++ b/csharp/src/FeatureFlagCache.cs @@ -307,9 +307,9 @@ public async Task> MergePropertiesWithFeatur try { - // Check if feature flag cache is enabled (default: true) - if (localProperties.TryGetValue(DatabricksParameters.FeatureFlagCacheEnabled, out string? enabledStr) && - bool.TryParse(enabledStr, out bool enabled) && + // Check if feature flag cache is enabled (default: false) + if (!localProperties.TryGetValue(DatabricksParameters.FeatureFlagCacheEnabled, out string? enabledStr) || + !bool.TryParse(enabledStr, out bool enabled) || !enabled) { activity?.AddEvent(new ActivityEvent("feature_flags.skipped", diff --git a/csharp/src/Telemetry/TelemetryConfiguration.cs b/csharp/src/Telemetry/TelemetryConfiguration.cs index 6074dadb..1b560cc0 100644 --- a/csharp/src/Telemetry/TelemetryConfiguration.cs +++ b/csharp/src/Telemetry/TelemetryConfiguration.cs @@ -81,9 +81,9 @@ public sealed class TelemetryConfiguration /// /// Gets or sets whether telemetry is enabled. - /// Default is true. + /// Default is false. /// - public bool Enabled { get; set; } = true; + public bool Enabled { get; set; } = false; /// /// Gets or sets the batch size for telemetry metrics. diff --git a/csharp/test/E2E/FeatureFlagCacheE2ETest.cs b/csharp/test/E2E/FeatureFlagCacheE2ETest.cs index 84f84e5b..ba4c7c53 100644 --- a/csharp/test/E2E/FeatureFlagCacheE2ETest.cs +++ b/csharp/test/E2E/FeatureFlagCacheE2ETest.cs @@ -17,6 +17,7 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Apache.Arrow.Adbc; using Apache.Arrow.Adbc.Tests; using Xunit; using Xunit.Abstractions; @@ -49,7 +50,7 @@ public async Task TestFeatureFlagCacheInitialization() Skip.If(string.IsNullOrEmpty(hostName), "Cannot determine host name from test configuration"); // Act - Create a connection which initializes the feature flag cache - using var connection = NewConnection(TestConfiguration); + using var connection = NewConnectionWithFeatureFlagCache(); // Assert - The connection should be created successfully Assert.NotNull(connection); @@ -97,8 +98,8 @@ public async Task TestFeatureFlagCacheSharedAcrossConnections() var cache = FeatureFlagCache.GetInstance(); // Act - Create two connections to the same host - using var connection1 = NewConnection(TestConfiguration); - using var connection2 = NewConnection(TestConfiguration); + using var connection1 = NewConnectionWithFeatureFlagCache(); + using var connection2 = NewConnectionWithFeatureFlagCache(); // Assert - Both connections should work properly Assert.NotNull(connection1); @@ -134,7 +135,7 @@ public async Task TestFeatureFlagCachePersistsAfterConnectionClose() OutputHelper?.WriteLine($"[FeatureFlagCacheE2ETest] Initial cache count: {cache.CachedHostCount}"); // Act - Create and close a connection - using (var connection = NewConnection(TestConfiguration)) + using (var connection = NewConnectionWithFeatureFlagCache()) { // Connection is active, cache should have a context for this host Assert.NotNull(connection); @@ -176,7 +177,7 @@ public async Task TestFeatureFlagCachePersistsAfterConnectionClose() public async Task TestConnectionWithFeatureFlagsExecutesQueries() { // Arrange - using var connection = NewConnection(TestConfiguration); + using var connection = NewConnectionWithFeatureFlagCache(); // Act - Execute multiple queries to ensure feature flags don't interfere var queries = new[] @@ -202,6 +203,18 @@ public async Task TestConnectionWithFeatureFlagsExecutesQueries() } } + /// + /// Creates a connection with feature flag cache explicitly enabled. + /// + private AdbcConnection NewConnectionWithFeatureFlagCache() + { + var parameters = GetDriverParameters(TestConfiguration); + parameters[DatabricksParameters.FeatureFlagCacheEnabled] = "true"; + var driver = new DatabricksDriver(); + var database = driver.Open(parameters); + return database.Connect(new Dictionary()); + } + /// /// Gets the normalized host name from test configuration. /// Strips protocol prefix if present (e.g., "https://host" -> "host"). diff --git a/csharp/test/Unit/FeatureFlagCacheTests.cs b/csharp/test/Unit/FeatureFlagCacheTests.cs index 2852f24b..b0326ff0 100644 --- a/csharp/test/Unit/FeatureFlagCacheTests.cs +++ b/csharp/test/Unit/FeatureFlagCacheTests.cs @@ -567,6 +567,64 @@ public async Task FeatureFlagContext_BackgroundRefreshError_ContinuesRefreshLoop #endregion + #region MergePropertiesWithFeatureFlagsAsync Default Behavior Tests + + [Fact] + public async Task MergePropertiesWithFeatureFlagsAsync_PropertyNotSet_ReturnsLocalProperties() + { + // Arrange - No FeatureFlagCacheEnabled property set (default: false) + var localProperties = new Dictionary + { + ["host"] = TestHost, + ["some_property"] = "some_value" + }; + var cache = FeatureFlagCache.GetInstance(); + + // Act + var result = await cache.MergePropertiesWithFeatureFlagsAsync(localProperties, DriverVersion); + + // Assert - Should return local properties unchanged (feature flags skipped) + Assert.Same(localProperties, result); + } + + [Fact] + public async Task MergePropertiesWithFeatureFlagsAsync_PropertySetToFalse_ReturnsLocalProperties() + { + // Arrange - FeatureFlagCacheEnabled explicitly set to false + var localProperties = new Dictionary + { + ["host"] = TestHost, + [DatabricksParameters.FeatureFlagCacheEnabled] = "false" + }; + var cache = FeatureFlagCache.GetInstance(); + + // Act + var result = await cache.MergePropertiesWithFeatureFlagsAsync(localProperties, DriverVersion); + + // Assert - Should return local properties unchanged (feature flags skipped) + Assert.Same(localProperties, result); + } + + [Fact] + public async Task MergePropertiesWithFeatureFlagsAsync_PropertySetToInvalidValue_ReturnsLocalProperties() + { + // Arrange - FeatureFlagCacheEnabled set to a non-boolean value + var localProperties = new Dictionary + { + ["host"] = TestHost, + [DatabricksParameters.FeatureFlagCacheEnabled] = "notabool" + }; + var cache = FeatureFlagCache.GetInstance(); + + // Act + var result = await cache.MergePropertiesWithFeatureFlagsAsync(localProperties, DriverVersion); + + // Assert - Should return local properties unchanged (can't parse as bool) + Assert.Same(localProperties, result); + } + + #endregion + #region Helper Methods /// diff --git a/csharp/test/Unit/Telemetry/TelemetryConfigurationTests.cs b/csharp/test/Unit/Telemetry/TelemetryConfigurationTests.cs index 62beb2e8..806eed4a 100644 --- a/csharp/test/Unit/Telemetry/TelemetryConfigurationTests.cs +++ b/csharp/test/Unit/Telemetry/TelemetryConfigurationTests.cs @@ -50,7 +50,7 @@ public void TelemetryConfiguration_DefaultValues_AreCorrect() var config = new TelemetryConfiguration(); // Assert - Assert.True(config.Enabled); + Assert.False(config.Enabled); Assert.Equal(100, config.BatchSize); Assert.Equal(5000, config.FlushIntervalMs); Assert.Equal(3, config.MaxRetries); @@ -106,7 +106,7 @@ public void TelemetryConfiguration_InvalidProperty_UsesDefault() // Assert - should use defaults for invalid values Assert.Equal(100, config.BatchSize); // default - Assert.True(config.Enabled); // default + Assert.False(config.Enabled); // default Assert.Equal(5000, config.FlushIntervalMs); // default } @@ -117,7 +117,7 @@ public void TelemetryConfiguration_NullProperties_UsesDefaults() var config = TelemetryConfiguration.FromProperties(null); // Assert - Assert.True(config.Enabled); + Assert.False(config.Enabled); Assert.Equal(100, config.BatchSize); Assert.Equal(5000, config.FlushIntervalMs); } @@ -132,7 +132,7 @@ public void TelemetryConfiguration_EmptyProperties_UsesDefaults() var config = TelemetryConfiguration.FromProperties(properties); // Assert - Assert.True(config.Enabled); + Assert.False(config.Enabled); Assert.Equal(100, config.BatchSize); Assert.Equal(5000, config.FlushIntervalMs); } @@ -187,7 +187,7 @@ public void TelemetryConfiguration_InvalidEnvironmentVariable_UsesDefault() // Assert - should use defaults for invalid environment variables Assert.Equal(100, config.BatchSize); - Assert.True(config.Enabled); + Assert.False(config.Enabled); } [Fact]