From 2c5ff8520db60abcdd584744fbb1087f00f87bd1 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Tue, 6 Feb 2024 15:12:16 -0500 Subject: [PATCH] First round reviewer comments --- .../Configuration/IMachineBuilderExtensions.cs | 2 +- src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs | 11 +++++++++++ .../Models/TranslationEngine.cs | 2 +- .../Services/ITranslationEngineService.cs | 2 +- .../Services/NmtClearMLBuildJobFactory.cs | 2 +- .../Services/NmtEngineService.cs | 8 ++++---- .../Services/NmtTrainBuildJob.cs | 2 +- .../Services/SmtTransferEngineService.cs | 4 ++-- src/SIL.Machine/Translation/ModelDownloadUrl.cs | 11 ----------- .../Services/CleanupJobTests.cs | 4 ++-- 10 files changed, 24 insertions(+), 24 deletions(-) create mode 100644 src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs delete mode 100644 src/SIL.Machine/Translation/ModelDownloadUrl.cs diff --git a/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs b/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs index 18df31a36..8755aa7b4 100644 --- a/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs +++ b/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs @@ -211,7 +211,7 @@ public static IMachineBuilder AddHangfireJobServer( builder.Services.AddHangfireServer(o => { - o.Queues = [.. queues]; + o.Queues = queues.ToArray(); }); return builder; } diff --git a/src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs b/src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs new file mode 100644 index 000000000..f48d73014 --- /dev/null +++ b/src/SIL.Machine.AspNetCore/Models/ModelDownloadUrl.cs @@ -0,0 +1,11 @@ +using System; + +namespace SIL.Machine.AspNetCore.Models +{ + public class ModelDownloadUrl + { + public string Url { get; set; } = default!; + public int ModelRevision { get; set; } = default!; + public DateTime ExipiresAt { get; set; } = default!; + } +} diff --git a/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs b/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs index 53d478672..0c982f34a 100644 --- a/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs +++ b/src/SIL.Machine.AspNetCore/Models/TranslationEngine.cs @@ -7,7 +7,7 @@ public class TranslationEngine : IEntity public string EngineId { get; set; } = default!; public string SourceLanguage { get; set; } = default!; public string TargetLanguage { get; set; } = default!; - public bool IsModelRetrievable { get; set; } = false; + public bool IsModelPersisted { get; set; } = false; public int BuildRevision { get; set; } public Build? CurrentBuild { get; set; } } diff --git a/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs b/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs index 50abb5773..33193382f 100644 --- a/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs +++ b/src/SIL.Machine.AspNetCore/Services/ITranslationEngineService.cs @@ -9,7 +9,7 @@ Task CreateAsync( string? engineName, string sourceLanguage, string targetLanguage, - bool isModelRetrievable = false, + bool isModelPersisted = false, CancellationToken cancellationToken = default ); Task DeleteAsync(string engineId, CancellationToken cancellationToken = default); diff --git a/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs b/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs index af58dccb2..87fa8528a 100644 --- a/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs +++ b/src/SIL.Machine.AspNetCore/Services/NmtClearMLBuildJobFactory.cs @@ -44,7 +44,7 @@ public async Task CreateJobScriptAsync( + $" 'shared_file_uri': '{baseUri}',\n" + $" 'shared_file_folder': '{folder}',\n" + (buildOptions is not null ? $" 'build_options': '''{buildOptions}''',\n" : "") - + (engine.IsModelRetrievable ? $" 'save_model': '{engineId}_{engine.BuildRevision + 1}',\n" : "") + + (engine.IsModelPersisted ? $" 'save_model': '{engineId}_{engine.BuildRevision + 1}',\n" : "") + $" 'clearml': True\n" + "}\n" + "run(args)\n"; diff --git a/src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs b/src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs index dd97ff51d..962879a32 100644 --- a/src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs +++ b/src/SIL.Machine.AspNetCore/Services/NmtEngineService.cs @@ -36,7 +36,7 @@ public async Task CreateAsync( string? engineName, string sourceLanguage, string targetLanguage, - bool isModelRetrievable = false, + bool isModelPersisted = false, CancellationToken cancellationToken = default ) { @@ -47,7 +47,7 @@ await _engines.InsertAsync( EngineId = engineId, SourceLanguage = sourceLanguage, TargetLanguage = targetLanguage, - IsModelRetrievable = isModelRetrievable + IsModelPersisted = isModelPersisted }, cancellationToken ); @@ -120,10 +120,10 @@ public async Task GetModelDownloadUrlAsync( ) { TranslationEngine engine = await GetEngineAsync(engineId, cancellationToken); - if (!engine.IsModelRetrievable) + if (!engine.IsModelPersisted) throw new InvalidOperationException( "The model cannot be downloaded. " - + "To enable downloading the model, recreate the engine with IsModelRetrievable property to true." + + "To enable downloading the model, recreate the engine with IsModelPersisted property to true." ); if (engine.BuildRevision == 0) throw new InvalidOperationException("The engine has not been built yet."); diff --git a/src/SIL.Machine.AspNetCore/Services/NmtTrainBuildJob.cs b/src/SIL.Machine.AspNetCore/Services/NmtTrainBuildJob.cs index 925cd5061..d3919a623 100644 --- a/src/SIL.Machine.AspNetCore/Services/NmtTrainBuildJob.cs +++ b/src/SIL.Machine.AspNetCore/Services/NmtTrainBuildJob.cs @@ -69,7 +69,7 @@ await PipInstallModuleAsync( + $" 'shared_file_uri': '{_sharedFileService.GetBaseUri()}',\n" + (buildOptions is not null ? $" 'build_options': '''{buildOptions}''',\n" : "") + ( - engine.IsModelRetrievable + engine.IsModelPersisted ? $" 'save_model': '{engine.Id}_{engine.BuildRevision + 1}',\n" : "" ) diff --git a/src/SIL.Machine.AspNetCore/Services/SmtTransferEngineService.cs b/src/SIL.Machine.AspNetCore/Services/SmtTransferEngineService.cs index 5846ad41b..7722970fb 100644 --- a/src/SIL.Machine.AspNetCore/Services/SmtTransferEngineService.cs +++ b/src/SIL.Machine.AspNetCore/Services/SmtTransferEngineService.cs @@ -32,7 +32,7 @@ public async Task CreateAsync( string? engineName, string sourceLanguage, string targetLanguage, - bool isModelRetrievable = false, + bool isModelPersisted = false, CancellationToken cancellationToken = default ) { @@ -43,7 +43,7 @@ await _engines.InsertAsync( EngineId = engineId, SourceLanguage = sourceLanguage, TargetLanguage = targetLanguage, - IsModelRetrievable = isModelRetrievable + IsModelPersisted = isModelPersisted }, cancellationToken ); diff --git a/src/SIL.Machine/Translation/ModelDownloadUrl.cs b/src/SIL.Machine/Translation/ModelDownloadUrl.cs deleted file mode 100644 index ceb0f4d46..000000000 --- a/src/SIL.Machine/Translation/ModelDownloadUrl.cs +++ /dev/null @@ -1,11 +0,0 @@ -using System; - -namespace SIL.Machine.Translation -{ - public class ModelDownloadUrl - { - public string Url { get; set; } = default; - public int ModelRevision { get; set; } = default; - public DateTime ExipiresAt { get; set; } = default; - } -} diff --git a/tests/SIL.Machine.AspNetCore.Tests/Services/CleanupJobTests.cs b/tests/SIL.Machine.AspNetCore.Tests/Services/CleanupJobTests.cs index 9b2aefc72..3f87e5c1e 100644 --- a/tests/SIL.Machine.AspNetCore.Tests/Services/CleanupJobTests.cs +++ b/tests/SIL.Machine.AspNetCore.Tests/Services/CleanupJobTests.cs @@ -26,7 +26,7 @@ private async Task SetUpAsync() SourceLanguage = "es", TargetLanguage = "en", BuildRevision = 1, - IsModelRetrievable = true + IsModelPersisted = true } ); _engines.Add( @@ -37,7 +37,7 @@ private async Task SetUpAsync() SourceLanguage = "es", TargetLanguage = "en", BuildRevision = 2, - IsModelRetrievable = true + IsModelPersisted = true } ); async Task WriteFileStub(string path, string content)