Skip to content

[Cherry pick] Remove redundant logging of found config file #1670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/Cli/ConfigGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ public static bool VerifyCanUpdateRelationship(RuntimeConfig runtimeConfig, stri
/// It will use the config provided by the user, else based on the environment value
/// it will either merge the config if base config and environmentConfig is present
/// else it will choose a single config based on precedence (left to right) of
/// overrides < environmentConfig < defaultConfig
/// overrides > environmentConfig > defaultConfig
/// Also preforms validation to check connection string is not null or empty.
/// </summary>
public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
Expand All @@ -965,15 +965,19 @@ public static bool TryStartEngineWithOptions(StartOptions options, FileSystemRun
return false;
}

loader.UpdateBaseConfigFileName(runtimeConfigFile);
loader.UpdateConfigFileName(runtimeConfigFile);

// Validates that config file has data and follows the correct json schema
// Replaces all the environment variables while deserializing when starting DAB.
if (!loader.TryLoadKnownConfig(out RuntimeConfig? deserializedRuntimeConfig, replaceEnvVar: true))
{
_logger.LogError("Failed to parse the config file: {configFile}.", runtimeConfigFile);
_logger.LogError("Failed to parse the config file: {runtimeConfigFile}.", runtimeConfigFile);
return false;
}
else
{
_logger.LogInformation("Loaded config file: {runtimeConfigFile}", runtimeConfigFile);
}

if (string.IsNullOrWhiteSpace(deserializedRuntimeConfig.DataSource.ConnectionString))
{
Expand Down
39 changes: 21 additions & 18 deletions src/Config/FileSystemRuntimeConfigLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace Azure.DataApiBuilder.Config;
/// </remarks>
public class FileSystemRuntimeConfigLoader : RuntimeConfigLoader
{
// This stores either the default config name e.g. dab-config.json
// or user provided config file.
private string _baseConfigFileName;

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

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

public FileSystemRuntimeConfigLoader(IFileSystem fileSystem, string baseConfigFileName = DEFAULT_CONFIG_FILE_NAME, string? connectionString = null)
: base(connectionString)
{
_fileSystem = fileSystem;
_baseConfigFileName = baseConfigFileName;
ConfigFileName = GetFileNameForEnvironment(Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"), false);
}

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

config = null;
return false;
Expand Down Expand Up @@ -178,21 +193,7 @@ public static string GetEnvironmentFileName(string fileName, string environmentV
public bool DoesFileExistInCurrentDirectory(string fileName)
{
string currentDir = _fileSystem.Directory.GetCurrentDirectory();
// Unable to use ILogger because this code is invoked before LoggerFactory
// is instantiated.
if (_fileSystem.File.Exists(_fileSystem.Path.Combine(currentDir, fileName)))
{
// This config file is logged as being found, but may not actually be used!
Console.WriteLine($"Found config file: {fileName}.");
return true;
}
else
{
// Unable to use ILogger because this code is invoked before LoggerFactory
// is instantiated.
Console.WriteLine($"Unable to find config file: {fileName} does not exist.");
return false;
}
return _fileSystem.File.Exists(_fileSystem.Path.Combine(currentDir, fileName));
}

/// <summary>
Expand Down Expand Up @@ -251,11 +252,13 @@ public static string GetMergedFileNameForEnvironment(string fileName, string env
}

/// <summary>
/// Allows the base config file name to be updated. This is commonly done when the CLI is starting up.
/// Allows the base config file and the actually loaded config file name(tracked by the property ConfigFileName)
/// to be updated. This is commonly done when the CLI is starting up.
/// </summary>
/// <param name="fileName"></param>
public void UpdateBaseConfigFileName(string fileName)
public void UpdateConfigFileName(string fileName)
{
_baseConfigFileName = fileName;
ConfigFileName = fileName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public void ValidateEasyAuthConfig()
new MockFileData(config.ToJson())
);

// Since we added the config file to the filesystem above after the config loader was initialized
// in TestInitialize, we need to update the ConfigfileName, otherwise it will be an empty string.
_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

try
{
_runtimeConfigValidator.ValidateConfig();
Expand All @@ -71,6 +75,8 @@ public void ValidateJwtConfigParamsSet()
new MockFileData(config.ToJson())
);

_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

try
{
_runtimeConfigValidator.ValidateConfig();
Expand All @@ -90,6 +96,8 @@ public void ValidateAuthNSectionNotNecessary()
new MockFileData(config.ToJson())
);

_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

try
{
_runtimeConfigValidator.ValidateConfig();
Expand Down Expand Up @@ -117,6 +125,8 @@ public void ValidateFailureWithIncompleteJwtConfig()
new MockFileData(config.ToJson())
);

_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

Assert.ThrowsException<NotSupportedException>(() =>
{
_runtimeConfigValidator.ValidateConfig();
Expand Down Expand Up @@ -150,6 +160,8 @@ public void ValidateFailureWithUnneededEasyAuthConfig()
new MockFileData(config.ToJson())
);

_runtimeConfigLoader.UpdateConfigFileName(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME);

Assert.ThrowsException<NotSupportedException>(() =>
{
_runtimeConfigValidator.ValidateConfig();
Expand Down
4 changes: 0 additions & 4 deletions src/Service/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,6 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeC
{
// Config provided before starting the engine.
isRuntimeReady = PerformOnConfigChangeAsync(app).Result;
if (_logger is not null)
{
_logger.LogDebug("Loaded config file from {filePath}", fileSystemRuntimeConfigLoader.ConfigFileName);
}

if (!isRuntimeReady)
{
Expand Down