Skip to content

Commit

Permalink
Fix startup of extractor when config is incorrect (#460)
Browse files Browse the repository at this point in the history
* Fix startup of extractor when config is incorrect

Also removes some old superfluous code

* Fix build

* Correct docs on publishing-interval

* Fix test
  • Loading branch information
einarmo authored Aug 4, 2023
1 parent 60fd91b commit 49625c1
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 37 deletions.
4 changes: 2 additions & 2 deletions Extractor/Pushers/Writers/WriterUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Cognite.OpcUa.Pushers.Writers
{
public static class WriterUtils
{
public static void AddWriters(this IServiceCollection services, CancellationToken token, FullConfig config)
public static void AddWriters(this IServiceCollection services, FullConfig config)
{
services.AddSingleton<ITimeseriesWriter>(provider => {
var destination = provider.GetRequiredService<CogniteDestination>();
Expand Down Expand Up @@ -54,7 +54,7 @@ public static void AddWriters(this IServiceCollection services, CancellationToke
}
if (config.Cognite?.MetadataTargets?.DataModels != null && config.Cognite.MetadataTargets.DataModels.Enabled)
{
services.AddSingleton<FDMWriter>(provider => {
services.AddSingleton(provider => {
var destination = provider.GetRequiredService<CogniteDestination>();
return new FDMWriter(provider.GetRequiredService<FullConfig>(), destination,
provider.GetRequiredService<ILogger<FDMWriter>>());
Expand Down
29 changes: 6 additions & 23 deletions ExtractorLauncher/ExtractorStarter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ private static void VerifyAndBuildConfig(
FullConfig config,
BaseExtractorParams setup,
ExtractorRunnerParams<FullConfig, UAExtractor>? options,
string configRoot)
string configRoot,
ServiceCollection services)
{
config.Source.ConfigRoot = configRoot;
if (!string.IsNullOrEmpty(setup.EndpointUrl)) config.Source.EndpointUrl = setup.EndpointUrl;
Expand All @@ -213,6 +214,7 @@ private static void VerifyAndBuildConfig(
config.Logger.File.Path = setup.LogDir;
}
}
services.AddWriters(config);
config.Source.AutoAccept |= setup.AutoAccept;
config.Source.ExitOnFailure |= setup is ExtractorParams p2 && p2.Exit;
config.DryRun |= setup.DryRun;
Expand Down Expand Up @@ -282,7 +284,7 @@ public static async Task RunConfigTool(ILogger? log, ConfigToolParams setup, Ser
setup.BaseConfig = ConfigurationUtils.TryReadConfigFromFile<FullConfig>(configFile, 1);
}

VerifyAndBuildConfig(log, setup.Config, setup, null, configDir);
VerifyAndBuildConfig(log, setup.Config, setup, null, configDir, services);

if (setup.NoConfig)
{
Expand Down Expand Up @@ -334,30 +336,12 @@ public static async Task RunExtractor(ILogger? log, ExtractorParams setup, Servi
var ver = Extractor.Metrics.Version.GetVersion(Assembly.GetExecutingAssembly());


FullConfig? config;
FullConfig? config = null;
if (setup.NoConfig)
{
config = new FullConfig();
config.GenerateDefaults();
}
else
{
try
{
string configFile = setup.ConfigFile ?? Path.Join(configDir, "config.yml");
config = ConfigurationUtils.TryReadConfigFromFile<FullConfig>(configFile, 1);
config.GenerateDefaults();
}
catch
{
config = null;
}
}

if (config != null && config.Cognite == null)
{
config.Cognite = new CognitePusherConfig();
}

services.AddSingleton<IPusher, CDFPusher>(provider =>
{
Expand All @@ -383,7 +367,6 @@ public static async Task RunExtractor(ILogger? log, ExtractorParams setup, Servi
});

services.AddSingleton<UAClient>();
services.AddWriters(token, config!);

var options = new ExtractorRunnerParams<FullConfig, UAExtractor>
{
Expand All @@ -395,7 +378,7 @@ public static async Task RunExtractor(ILogger? log, ExtractorParams setup, Servi
AddLogger = true,
AddMetrics = true,
Restart = !setup.Exit,
ConfigCallback = (config, options, services) => VerifyAndBuildConfig(log, config, setup, options, configDir),
ConfigCallback = (config, options, services) => VerifyAndBuildConfig(log, config, setup, options, configDir, services),
ExtServices = services,
StartupLogger = log,
Config = config,
Expand Down
19 changes: 10 additions & 9 deletions Test/Integration/LauncherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,8 @@ public void TestVerifyAndBuildConfig()
config.GenerateDefaults();
config.Source.EndpointUrl = tester.EndpointUrl;
var setup = new ExtractorParams();
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, null, "config" });
var services = new ServiceCollection();
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, null, "config", services });

// Bind setup
setup.EndpointUrl = "opc.tcp://localhost:60000";
Expand All @@ -431,7 +432,7 @@ public void TestVerifyAndBuildConfig()
setup.AutoAccept = true;
setup.Exit = true;

method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, null, "config" });
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, null, "config", services });
Assert.Equal("config", config.Source.ConfigRoot);
Assert.Equal("opc.tcp://localhost:60000", config.Source.EndpointUrl);
Assert.Equal("user", config.Source.Username);
Expand All @@ -454,7 +455,7 @@ public void TestVerifyAndBuildConfig()
setup.Exit = true;
var options = new ExtractorRunnerParams<FullConfig, UAExtractor>();

method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" });
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services });

Assert.Equal("logs2", config.Logger.File.Path);
Assert.Equal("debug", config.Logger.File.Level);
Expand All @@ -468,37 +469,37 @@ public void TestVerifyAndBuildConfig()
setup = new ExtractorParams();

var exc = Assert.Throws<TargetInvocationException>(() =>
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" }));
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services }));
Assert.Equal("Invalid config: Missing endpoint-url", exc.InnerException.Message);

config.Source.EndpointUrl = "invalidurl";
exc = Assert.Throws<TargetInvocationException>(() =>
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" }));
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services }));
Assert.Equal("Invalid config: EndpointUrl is not a valid URI", exc.InnerException.Message);

// Get warnings
log.Events.Clear();
config.Source.EndpointUrl = tester.EndpointUrl;
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" });
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services });
Assert.Equal(3, log.Events.Where(evt => evt.LogLevel == Microsoft.Extensions.Logging.LogLevel.Warning).Count());

// events idprefix
config.Extraction.IdPrefix = "events.";
exc = Assert.Throws<TargetInvocationException>(() =>
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" }));
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services }));
Assert.Equal("Invalid config: Do not use events. as id-prefix, as it is used internally", exc.InnerException.Message);

// Invalid history start time
config.Extraction.IdPrefix = null;
config.History.StartTime = "2d-agoo";
exc = Assert.Throws<TargetInvocationException>(() =>
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config" }));
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "config", services }));
Assert.Equal("Invalid config: Invalid history start time: 2d-agoo", exc.InnerException.Message);

// Missing opc-ua xml config
config.History.StartTime = null;
exc = Assert.Throws<TargetInvocationException>(() =>
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, "." }));
method.Invoke(typeof(ExtractorStarter), new object[] { log, config, setup, options, ".", services }));
Assert.Equal("Missing opc.ua.net.extractor.Config.xml in config folder .", exc.InnerException.Message);
}
}
Expand Down
2 changes: 1 addition & 1 deletion Test/Unit/CDFPusherTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@ public async Task TestAllDestinationsActive()
};

(handler, pusher) = tester.GetCDFPusher();
var extractor = tester.BuildExtractor(true, null, pusher);
using var extractor = tester.BuildExtractor(true, null, pusher);

var update = new UpdateConfig();
var dt = new UADataType(DataTypeIds.Double);
Expand Down
2 changes: 1 addition & 1 deletion Test/Utils/BaseExtractorTestFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public async Task ClearLiteDB(InfluxDBClient client)
}
CommonTestUtils.AddDummyProvider("test", CDFMockHandler.MockMode.None, true, newServices);
newServices.AddCogniteClient("appid", null, true, true, false);
newServices.AddWriters(Source.Token, Config);
newServices.AddWriters(Config);
var provider = newServices.BuildServiceProvider();
var destination = provider.GetRequiredService<CogniteDestination>();
var pusher = new CDFPusher(Provider.GetRequiredService<ILogger<CDFPusher>>(),
Expand Down
1 change: 0 additions & 1 deletion config/config.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ source:
# Paths are defined in opc.ua.net.extractor.Config.xml
auto-accept: false
# How often the client requests updates to subscribed variables.
# 0 updates at maximum rate set by server
# This decides the maximum rate points are pushed to CDF. Higher reduces network and server load.
publishing-interval: 500
# OPC-UA username, leave blank for anonymous user. (no authentication)
Expand Down

0 comments on commit 49625c1

Please sign in to comment.