From a3e61e022e5c0f3709387c277e0b221c988b1e36 Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod <5160559+rkm@users.noreply.github.com> Date: Tue, 7 Jan 2020 12:01:53 +0000 Subject: [PATCH 1/5] DicomReprocessor: Fix --auto-run argument being ignored --- .../Execution/MongoDbReader.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/microservices/Microservices.DicomReprocessor/Execution/MongoDbReader.cs b/src/microservices/Microservices.DicomReprocessor/Execution/MongoDbReader.cs index 104827c1d..89faec34f 100644 --- a/src/microservices/Microservices.DicomReprocessor/Execution/MongoDbReader.cs +++ b/src/microservices/Microservices.DicomReprocessor/Execution/MongoDbReader.cs @@ -40,6 +40,7 @@ public bool WasCancelled private readonly CancellationTokenSource _tokenSource = new CancellationTokenSource(); private bool _stopping; + private readonly bool _autoRun; public MongoDbReader(MongoDbOptions mongoOptions, DicomReprocessorCliOptions reprocessorOptions, string appId) { @@ -57,10 +58,12 @@ public MongoDbReader(MongoDbOptions mongoOptions, DicomReprocessorCliOptions rep // https://docs.mongodb.com/manual/reference/method/cursor.batchSize/ if (reprocessorOptions.MongoDbBatchSize > 1) _findOptionsBase.BatchSize = reprocessorOptions.MongoDbBatchSize; + + _autoRun = reprocessorOptions.AutoRun; } - public async Task RunQuery(string query, IDocumentProcessor processor, DicomReprocessorOptions options, bool autoRun = false) + public async Task RunQuery(string query, IDocumentProcessor processor, DicomReprocessorOptions options) { DateTime start; @@ -72,7 +75,7 @@ public async Task RunQuery(string query, IDocumentProcessor processor, _logger.Info($"Batch size is: {(_findOptionsBase.BatchSize.HasValue ? _findOptionsBase.BatchSize.ToString() : "unspecified")}"); _logger.Info($"Sleeping for {options.SleepTime.TotalMilliseconds}ms between batches"); - if (!autoRun) + if (!_autoRun) { LogManager.Flush(); From f947616a34b0cc1905ac6f00f954e0844723a03b Mon Sep 17 00:00:00 2001 From: Ruairidh MacLeod <5160559+rkm@users.noreply.github.com> Date: Tue, 7 Jan 2020 18:03:40 +0000 Subject: [PATCH 2/5] Improve some debug messages --- .../Smi.Common/Execution/MicroserviceHost.cs | 9 ++++++++- .../Messaging/ControlMessageConsumer.cs | 4 +--- src/common/Smi.Common/RabbitMQAdapter.cs | 17 ++++++++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/common/Smi.Common/Execution/MicroserviceHost.cs b/src/common/Smi.Common/Execution/MicroserviceHost.cs index 2506e0234..866025471 100644 --- a/src/common/Smi.Common/Execution/MicroserviceHost.cs +++ b/src/common/Smi.Common/Execution/MicroserviceHost.cs @@ -48,10 +48,12 @@ protected MicroserviceHost(GlobalOptions globals, bool loadSmiLogConfig = true) { HostProcessName = Assembly.GetEntryAssembly()?.GetName().Name ?? throw new ApplicationException("Couldn't get the Assembly name!"); + string logConfigPath = null; + // We may not want to do this during tests, however this should always be true otherwise if (loadSmiLogConfig) { - string logConfigPath = !string.IsNullOrWhiteSpace(globals.FileSystemOptions.LogConfigFile) + logConfigPath = !string.IsNullOrWhiteSpace(globals.FileSystemOptions.LogConfigFile) ? globals.FileSystemOptions.LogConfigFile : Path.Combine(globals.CurrentDirectory, "Smi.NLog.config"); @@ -75,6 +77,9 @@ protected MicroserviceHost(GlobalOptions globals, bool loadSmiLogConfig = true) Logger = LogManager.GetLogger(GetType().Name); Logger.Info("Host logger created with " + (loadSmiLogConfig ? "SMI" : "existing") + " logging config"); + if (!string.IsNullOrWhiteSpace(logConfigPath)) + Logger.Debug($"Logging config loaded from {logConfigPath}"); + if (!globals.MicroserviceOptions.TraceLogging) LogManager.GlobalThreshold = LogLevel.Debug; @@ -160,6 +165,8 @@ public virtual void Stop(string reason) _stopCalled = true; + Logger.Debug("Shutting down RabbitMQ connections"); + // Attempt to destroy the control queue try diff --git a/src/common/Smi.Common/Messaging/ControlMessageConsumer.cs b/src/common/Smi.Common/Messaging/ControlMessageConsumer.cs index 365fd25e5..2772e4379 100644 --- a/src/common/Smi.Common/Messaging/ControlMessageConsumer.cs +++ b/src/common/Smi.Common/Messaging/ControlMessageConsumer.cs @@ -64,8 +64,6 @@ public ControlMessageConsumer(MicroserviceHost host, RabbitOptions options, stri /// public override void ProcessMessage(BasicDeliverEventArgs e) { - Logger.Info("Control message received"); - try { ProcessMessageImpl(null, e); @@ -119,7 +117,7 @@ protected override void ProcessMessageImpl(IMessageHeader header, BasicDeliverEv Logger.Debug("Control command did not match this service"); return; } - + // Handle any general actions - just stop and ping for now if (action.StartsWith("stop")) diff --git a/src/common/Smi.Common/RabbitMQAdapter.cs b/src/common/Smi.Common/RabbitMQAdapter.cs index 802225a90..2761b2a94 100644 --- a/src/common/Smi.Common/RabbitMQAdapter.cs +++ b/src/common/Smi.Common/RabbitMQAdapter.cs @@ -175,6 +175,7 @@ public Guid StartConsumer(ConsumerOptions consumerOptions, IConsumer consumer, b }; consumerTask.Start(); + _logger.Debug($"Consumer task started [ID={consumerTask.Id}]"); return taskId; } @@ -426,6 +427,13 @@ private class RabbitResources : IDisposable protected readonly object OResourceLock = new object(); + protected readonly ILogger Logger; + + public RabbitResources() + { + Logger = LogManager.GetLogger(GetType().Name); + } + public void Dispose() { @@ -456,19 +464,22 @@ private class ConsumerResources : RabbitResources public bool Shutdown(int timeout = 5000) { + bool exitOk; lock (OResourceLock) { TokenSource.Cancel(); // Consumer task can't directly shut itself down, as it will block here - bool exitOk = ConsumerTask.Wait(timeout); + exitOk = ConsumerTask.Wait(timeout); Subscription.Close(); Dispose(); - - return exitOk; } + + Logger.Debug($"Consumer task shutdown [ID={ConsumerTask.Id}]"); + + return exitOk; } } From 0935c6b4ea6fb599b738056f33f9444dd27a29ec Mon Sep 17 00:00:00 2001 From: Thomas Nind Date: Wed, 8 Jan 2020 08:00:14 +0000 Subject: [PATCH 3/5] Changed SELECT INTO to use explicit column names - Previously migration of RAW to Staging used `insert into A select * from B`. This could cause issues if column order was somehow different between tables - Now we intersect columns in both tables and use `insert into A(col1,col2) select col1,col2 from B`. --- CHANGELOG.md | 4 ++++ .../MigrateRawToStagingWithSelectIntoStatements.cs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 250e9af0a..d4f9ee058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Fixed + +- RAW to STAGING migration now lists columns explicitly (previously used `SELECT *` which could cause problems if RAW and STAGING column orders somehow differed) + ## [1.2.1] - 2020-01-06 ### Added diff --git a/src/microservices/Microservices.DicomRelationalMapper/Execution/MigrateRawToStagingWithSelectIntoStatements.cs b/src/microservices/Microservices.DicomRelationalMapper/Execution/MigrateRawToStagingWithSelectIntoStatements.cs index a63f008df..be486a4c5 100644 --- a/src/microservices/Microservices.DicomRelationalMapper/Execution/MigrateRawToStagingWithSelectIntoStatements.cs +++ b/src/microservices/Microservices.DicomRelationalMapper/Execution/MigrateRawToStagingWithSelectIntoStatements.cs @@ -12,6 +12,7 @@ using System.Collections.Generic; using System.Data.Common; using System.Diagnostics; +using System.Linq; using System.Threading.Tasks; using ReusableLibraryCode; @@ -53,9 +54,16 @@ public override ExitCodeType Run(IDataLoadJob job, GracefulCancellationToken can IQuerySyntaxHelper syntaxHelper = table.GetQuerySyntaxHelper(); - string sql = string.Format(@"INSERT INTO {1} SELECT DISTINCT * FROM {0}", + var fromCols = server.ExpectDatabase(fromDb).ExpectTable(fromTable).DiscoverColumns(); + var toCols = server.ExpectDatabase(toDb).ExpectTable(toTable).DiscoverColumns(); + + //Migrate only columns that appear in both tables + var commonColumns = fromCols.Select(f => f.GetRuntimeName()).Intersect(toCols.Select(t => t.GetRuntimeName())).ToArray(); + + string sql = string.Format(@"INSERT INTO {1}({2}) SELECT DISTINCT {2} FROM {0}", syntaxHelper.EnsureFullyQualified(fromDb, null, fromTable), - syntaxHelper.EnsureFullyQualified(toDb, null, toTable)); + syntaxHelper.EnsureFullyQualified(toDb, null, toTable), + string.Join(",",commonColumns.Select(c=>syntaxHelper.EnsureWrapped(c)))); job.OnNotify(this, new NotifyEventArgs(ProgressEventType.Information, "About to send SQL:" + sql)); From b7a0ca8c06f9d61957c6e9b29cde12b24a5f8354 Mon Sep 17 00:00:00 2001 From: Thomas Nind Date: Wed, 8 Jan 2020 08:51:16 +0000 Subject: [PATCH 4/5] Removed bad config data --- src/common/Smi.Common/app.config | 13 ------------- .../Microservices.CohortExtractor/App.config | 16 ---------------- .../App.config | 16 ---------------- 3 files changed, 45 deletions(-) diff --git a/src/common/Smi.Common/app.config b/src/common/Smi.Common/app.config index beb97d940..116fbcb4c 100644 --- a/src/common/Smi.Common/app.config +++ b/src/common/Smi.Common/app.config @@ -33,17 +33,4 @@ - - - - - - - - - - - - - \ No newline at end of file diff --git a/src/microservices/Microservices.CohortExtractor/App.config b/src/microservices/Microservices.CohortExtractor/App.config index cade04092..4c6002d02 100644 --- a/src/microservices/Microservices.CohortExtractor/App.config +++ b/src/microservices/Microservices.CohortExtractor/App.config @@ -41,20 +41,4 @@ - - - - - - - - - - - - - - - - diff --git a/src/microservices/Microservices.DicomRelationalMapper/App.config b/src/microservices/Microservices.DicomRelationalMapper/App.config index bbb2f899f..86c304821 100644 --- a/src/microservices/Microservices.DicomRelationalMapper/App.config +++ b/src/microservices/Microservices.DicomRelationalMapper/App.config @@ -41,20 +41,4 @@ - - - - - - - - - - - - - - - - From 765f619a09154a7060395f914ce1dcb2bb7dd66d Mon Sep 17 00:00:00 2001 From: Thomas Nind Date: Wed, 8 Jan 2020 09:33:39 +0000 Subject: [PATCH 5/5] Bumped version to 1.2.2 --- CHANGELOG.md | 7 ++++++- README.md | 2 +- src/SharedAssemblyInfo.cs | 6 +++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4f9ee058..48bcd20f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +... + +## [1.2.2] - 2020-01-08 + ### Fixed - RAW to STAGING migration now lists columns explicitly (previously used `SELECT *` which could cause problems if RAW and STAGING column orders somehow differed) @@ -96,7 +100,8 @@ First stable release after importing the repository from the private [SMIPlugin] - Anonymous `MappingTableName` must now be fully specified to pass validation (e.g. `mydb.mytbl`). Previously skipping database portion was supported. -[Unreleased]: https://github.com/SMI/SmiServices/compare/v1.2.1...develop +[Unreleased]: https://github.com/SMI/SmiServices/compare/v1.2.2...develop +[1.2.2]: https://github.com/SMI/SmiServices/compare/v1.2.1...v1.2.2 [1.2.1]: https://github.com/SMI/SmiServices/compare/1.2.0...v1.2.1 [1.2.0]: https://github.com/SMI/SmiServices/compare/1.1.0-rc1...1.2.0 [1.2.0-rc1]: https://github.com/SMI/SmiServices/compare/1.1.0...1.2.0-rc1 diff --git a/README.md b/README.md index 1dfd31776..0bbc34916 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ ![GitHub](https://img.shields.io/github/license/SMI/SmiServices) [![Total alerts](https://img.shields.io/lgtm/alerts/g/SMI/SmiServices.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/SMI/SmiServices/alerts/) -Version: `1.2.1` +Version: `1.2.2` # SMI Services diff --git a/src/SharedAssemblyInfo.cs b/src/SharedAssemblyInfo.cs index 762dde286..a389a0afc 100644 --- a/src/SharedAssemblyInfo.cs +++ b/src/SharedAssemblyInfo.cs @@ -7,6 +7,6 @@ [assembly: AssemblyCulture("")] // These should be overwritten by release builds -[assembly: AssemblyVersion("1.2.1")] -[assembly: AssemblyFileVersion("1.2.1")] -[assembly: AssemblyInformationalVersion("1.2.1")] // This one can have the extra build info after it +[assembly: AssemblyVersion("1.2.2")] +[assembly: AssemblyFileVersion("1.2.2")] +[assembly: AssemblyInformationalVersion("1.2.2")] // This one can have the extra build info after it