Skip to content

Commit 9d07882

Browse files
committed
Remove redundant logging of found config file (#1669)
- With the overhaul of config system PR #1402 , we started seeing redundant logs about which config file is being used. - Store the environment based config file name found at construction time in the property `ConfigFileName`. - Remove Console writeline statements while checking for existence of file since this leads to multiple logs - one from CLI before starting the engine and second from within the engine itself when calling TryPargeConfig - Add a single log of the loaded file in CLI - Also catch exception for incorrect connection string parsing - Manually, using the `start` command of CLI as well as triggering the engine directly with argument: `--ConfigFileName` 1. Starting with default config: **Before**: ![image](https://github.com/Azure/data-api-builder/assets/3513779/a29d4aff-19db-49c7-a745-b6bbdb5c62cc) **After**: ![image](https://github.com/Azure/data-api-builder/assets/3513779/62f65222-de59-4525-b0b1-ead407d43b4e) 2. Starting with user provided config: **Before**: ![image](https://github.com/Azure/data-api-builder/assets/3513779/b762c873-ade6-47bb-b094-44960176d90f) **After**: ![image](https://github.com/Azure/data-api-builder/assets/3513779/2f977a72-bfdf-43fb-9cc9-806ea717b546) 3. Trying to start with missing file name - same behvior as before ![image](https://github.com/Azure/data-api-builder/assets/3513779/5a80f62e-8a06-4e7c-aa22-c120dade6801)
1 parent 0ad34c1 commit 9d07882

File tree

4 files changed

+40
-25
lines changed

4 files changed

+40
-25
lines changed

src/Cli/ConfigGenerator.cs

+7-3
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,7 @@ public static bool VerifyCanUpdateRelationship(RuntimeConfig runtimeConfig, stri
948948
/// It will use the config provided by the user, else based on the environment value
949949
/// it will either merge the config if base config and environmentConfig is present
950950
/// else it will choose a single config based on precedence (left to right) of
951-
/// overrides < environmentConfig < defaultConfig
951+
/// overrides > environmentConfig > defaultConfig
952952
/// Also preforms validation to check connection string is not null or empty.
953953
/// </summary>
954954
public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
@@ -965,15 +965,19 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun
965965
return false;
966966
}
967967

968-
loader.UpdateBaseConfigFileName(runtimeConfigFile);
968+
loader.UpdateConfigFileName(runtimeConfigFile);
969969

970970
// Validates that config file has data and follows the correct json schema
971971
// Replaces all the environment variables while deserializing when starting DAB.
972972
if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true))
973973
{
974-
_logger.LogError("Failed to parse the config file: {configFile}.", runtimeConfigFile);
974+
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
975975
return false;
976976
}
977+
else
978+
{
979+
_logger.LogInformation("Loaded config file: {runtimeConfigFile}", runtimeConfigFile);
980+
}
977981

978982
if (string.IsNullOrWhiteSpace(deserializedRuntimeConfig.DataSource.ConnectionString))
979983
{

src/Config/FileSystemRuntimeConfigLoader.cs

+21-18
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ namespace Azure.DataApiBuilder.Config;
2525
/// </remarks>
2626
public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader
2727
{
28+
// This stores either the default config name e.g. dab-config.json
29+
// or user provided config file.
2830
private string _baseConfigFileName;
2931

3032
private readonly IFileSystem _fileSystem;
@@ -42,13 +44,20 @@ public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader
4244
/// </summary>
4345
public const string DEFAULT_CONFIG_FILE_NAME = $"{CONFIGFILE_NAME}{CONFIG_EXTENSION}";
4446

45-
public string ConfigFileName => GetFileNameForEnvironment(Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"), false);
47+
/// <summary>
48+
/// Stores the config file name actually loaded by the engine.
49+
/// It could be the base config file (e.g. dab-config.json), any of its derivatives with
50+
/// environment specific suffixes (e.g. dab-config.Development.json) or the user provided
51+
/// config file name.
52+
/// </summary>
53+
public string ConfigFileName { get; internal set; }
4654

4755
public FileSystemRuntimeConfigLoader(IFileSystem fileSystem, string baseConfigFileName = DEFAULT_CONFIG_FILE_NAME, string? connectionString = null)
4856
: base(connectionString)
4957
{
5058
_fileSystem = fileSystem;
5159
_baseConfigFileName = baseConfigFileName;
60+
ConfigFileName = GetFileNameForEnvironment(Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"), false);
5261
}
5362

5463
/// <summary>
@@ -69,6 +78,12 @@ public bool TryLoadConfig(
6978
string json = _fileSystem.File.ReadAllText(path);
7079
return TryParseConfig(json, out config, connectionString: _connectionString, replaceEnvVar: replaceEnvVar);
7180
}
81+
else
82+
{
83+
// Unable to use ILogger because this code is invoked before LoggerFactory
84+
// is instantiated.
85+
Console.WriteLine($"Unable to find config file: {path} does not exist.");
86+
}
7287

7388
config = null;
7489
return false;
@@ -178,21 +193,7 @@ public static string GetEnvironmentFileName(string fileName, string environmentV
178193
public bool DoesFileExistInCurrentDirectory(string fileName)
179194
{
180195
string currentDir = _fileSystem.Directory.GetCurrentDirectory();
181-
// Unable to use ILogger because this code is invoked before LoggerFactory
182-
// is instantiated.
183-
if (_fileSystem.File.Exists(_fileSystem.Path.Combine(currentDir, fileName)))
184-
{
185-
// This config file is logged as being found, but may not actually be used!
186-
Console.WriteLine($"Found config file: {fileName}.");
187-
return true;
188-
}
189-
else
190-
{
191-
// Unable to use ILogger because this code is invoked before LoggerFactory
192-
// is instantiated.
193-
Console.WriteLine($"Unable to find config file: {fileName} does not exist.");
194-
return false;
195-
}
196+
return _fileSystem.File.Exists(_fileSystem.Path.Combine(currentDir, fileName));
196197
}
197198

198199
/// <summary>
@@ -251,11 +252,13 @@ public static string GetMergedFileNameForEnvironment(string fileName, string env
251252
}
252253

253254
/// <summary>
254-
/// Allows the base config file name to be updated. This is commonly done when the CLI is starting up.
255+
/// Allows the base config file and the actually loaded config file name(tracked by the property ConfigFileName)
256+
/// to be updated. This is commonly done when the CLI is starting up.
255257
/// </summary>
256258
/// <param name="fileName"></param>
257-
public void UpdateBaseConfigFileName(string fileName)
259+
public void UpdateConfigFileName(string fileName)
258260
{
259261
_baseConfigFileName = fileName;
262+
ConfigFileName = fileName;
260263
}
261264
}

src/Service.Tests/Configuration/AuthenticationConfigValidatorUnitTests.cs

+12
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ public void ValidateEasyAuthConfig()
4545
new MockFileData(config.ToJson())
4646
);
4747

48+
// Since we added the config file to the filesystem above after the config loader was initialized
49+
// in TestInitialize, we need to update the ConfigfileName, otherwise it will be an empty string.
50+
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);
51+
4852
try
4953
{
5054
_runtimeConfigValidator.ValidateConfig();
@@ -71,6 +75,8 @@ public void ValidateJwtConfigParamsSet()
7175
new MockFileData(config.ToJson())
7276
);
7377

78+
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);
79+
7480
try
7581
{
7682
_runtimeConfigValidator.ValidateConfig();
@@ -90,6 +96,8 @@ public void ValidateAuthNSectionNotNecessary()
9096
new MockFileData(config.ToJson())
9197
);
9298

99+
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);
100+
93101
try
94102
{
95103
_runtimeConfigValidator.ValidateConfig();
@@ -117,6 +125,8 @@ public void ValidateFailureWithIncompleteJwtConfig()
117125
new MockFileData(config.ToJson())
118126
);
119127

128+
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);
129+
120130
Assert.ThrowsException<NotSupportedException>(() =>
121131
{
122132
_runtimeConfigValidator.ValidateConfig();
@@ -150,6 +160,8 @@ public void ValidateFailureWithUnneededEasyAuthConfig()
150160
new MockFileData(config.ToJson())
151161
);
152162

163+
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);
164+
153165
Assert.ThrowsException<NotSupportedException>(() =>
154166
{
155167
_runtimeConfigValidator.ValidateConfig();

src/Service/Startup.cs

-4
Original file line numberDiff line numberDiff line change
@@ -294,10 +294,6 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeC
294294
{
295295
// Config provided before starting the engine.
296296
isRuntimeReady = PerformOnConfigChangeAsync(app).Result;
297-
if (_logger is not null)
298-
{
299-
_logger.LogDebug("Loaded config file from {filePath}", fileSystemRuntimeConfigLoader.ConfigFileName);
300-
}
301297

302298
if (!isRuntimeReady)
303299
{

0 commit comments

Comments
 (0)