Use named HTTP clients for ClearML

- move ClearML connection string to "ConnectionStrings" section - fixes #108
sillsdev · Oct 18, 2023 · 0347d2c · 0347d2c
1 parent 03a3aa4
commit 0347d2c
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 37 deletions.
diff --git a/src/SIL.Machine.AspNetCore/Configuration/ClearMLOptions.cs b/src/SIL.Machine.AspNetCore/Configuration/ClearMLOptions.cs
@@ -4,7 +4,6 @@ public class ClearMLOptions
 {
     public const string Key = "ClearML";
 
-    public string ApiServer { get; set; } = "http://localhost:8008";
     public string Queue { get; set; } = "default";
     public string AccessKey { get; set; } = "";
     public string SecretKey { get; set; } = "";

diff --git a/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs b/src/SIL.Machine.AspNetCore/Configuration/IMachineBuilderExtensions.cs
@@ -101,33 +101,39 @@ public static IMachineBuilder AddUnigramTruecaser(this IMachineBuilder builder)
         return builder;
     }
 
-    private static IMachineBuilder AddClearMLBuildJobRunner(this IMachineBuilder builder)
+    public static IMachineBuilder AddClearMLService(this IMachineBuilder builder, string? connectionString = null)
     {
-        builder.Services.AddSingleton<IClearMLService, ClearMLService>();
-        //Add retry policy; fail after approx. 2 + 4 + 8 = 14 seconds
+        connectionString ??= builder.Configuration.GetConnectionString("ClearML");
         builder.Services
-            .AddHttpClient<ClearMLService>()
+            .AddHttpClient("ClearML")
+            .ConfigureHttpClient(httpClient => httpClient.BaseAddress = new Uri(connectionString))
+            // Add retry policy; fail after approx. 2 + 4 + 8 = 14 seconds
             .AddTransientHttpErrorPolicy(
                 b => b.WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)))
             );
 
+        builder.Services.AddSingleton<IClearMLService, ClearMLService>();
+
         // workaround register satisfying the interface and as a hosted service.
         builder.Services.AddSingleton<IClearMLAuthenticationService, ClearMLAuthenticationService>();
         builder.Services.AddHostedService(p => p.GetRequiredService<IClearMLAuthenticationService>());
-        // Add retry policy; fail after approx. 2 + 4 + 8 = 14 seconds
+
         builder.Services
-            .AddHttpClient<IClearMLAuthenticationService, ClearMLAuthenticationService>()
-            .AddTransientHttpErrorPolicy(
-                b => b.WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)))
-            );
+            .AddHttpClient("ClearML-NoRetry")
+            .ConfigureHttpClient(httpClient => httpClient.BaseAddress = new Uri(connectionString));
 
+        builder.Services.AddHealthChecks().AddCheck<ClearMLHealthCheck>("ClearML Health Check");
+
+        return builder;
+    }
+
+    private static IMachineBuilder AddClearMLBuildJobRunner(this IMachineBuilder builder)
+    {
         builder.Services.AddScoped<IBuildJobRunner, ClearMLBuildJobRunner>();
         builder.Services.AddScoped<IClearMLBuildJobFactory, NmtClearMLBuildJobFactory>();
         builder.Services.AddSingleton<ClearMLMonitorService>();
         builder.Services.AddHostedService(p => p.GetRequiredService<ClearMLMonitorService>());
 
-        builder.Services.AddHealthChecks().AddCheck<ClearMLHealthCheck>("ClearML Health Check");
-
         return builder;
     }
 

diff --git a/src/SIL.Machine.AspNetCore/Services/ClearMLAuthenticationService.cs b/src/SIL.Machine.AspNetCore/Services/ClearMLAuthenticationService.cs
@@ -14,13 +14,13 @@ public class ClearMLAuthenticationService : RecurrentTask, IClearMLAuthenticatio
 
     public ClearMLAuthenticationService(
         IServiceProvider services,
-        HttpClient httpClient,
+        IHttpClientFactory httpClientFactory,
         IOptionsMonitor<ClearMLOptions> options,
         ILogger<ClearMLAuthenticationService> logger
     )
         : base("ClearML authentication service", services, RefreshPeriod, logger)
     {
-        _httpClient = httpClient;
+        _httpClient = httpClientFactory.CreateClient("ClearML");
         _options = options;
         _logger = logger;
     }
@@ -54,7 +54,7 @@ protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken
 
     private async Task AuthorizeAsync(CancellationToken cancellationToken)
     {
-        var request = new HttpRequestMessage(HttpMethod.Post, $"{_options.CurrentValue.ApiServer}/auth.login")
+        var request = new HttpRequestMessage(HttpMethod.Post, "auth.login")
         {
             Content = new StringContent("{}", Encoding.UTF8, "application/json")
         };

diff --git a/src/SIL.Machine.AspNetCore/Services/ClearMLHealthCheck.cs b/src/SIL.Machine.AspNetCore/Services/ClearMLHealthCheck.cs
@@ -6,11 +6,11 @@ public class ClearMLHealthCheck : IHealthCheck
 
     public ClearMLHealthCheck(
         IClearMLAuthenticationService clearMLAuthenticationService,
-        HttpClient httpClient,
+        IHttpClientFactory httpClientFactory,
         IOptionsMonitor<ClearMLOptions> options
     )
     {
-        _httpClient = httpClient;
+        _httpClient = httpClientFactory.CreateClient("ClearML-NoRetry");
         _options = options;
         _clearMLAuthenticationService = clearMLAuthenticationService;
     }
@@ -43,7 +43,7 @@ public async Task<HealthCheckResult> CheckHealthAsync(
         CancellationToken cancellationToken = default
     )
     {
-        var request = new HttpRequestMessage(HttpMethod.Post, $"{_options.CurrentValue.ApiServer}/{service}.{action}")
+        var request = new HttpRequestMessage(HttpMethod.Post, $"{service}.{action}")
         {
             Content = new StringContent(body.ToJsonString(), Encoding.UTF8, "application/json")
         };

diff --git a/src/SIL.Machine.AspNetCore/Services/ClearMLService.cs b/src/SIL.Machine.AspNetCore/Services/ClearMLService.cs
@@ -16,13 +16,13 @@ public class ClearMLService : IClearMLService
     private readonly IClearMLAuthenticationService _clearMLAuthService;
 
     public ClearMLService(
-        HttpClient httpClient,
+        IHttpClientFactory httpClientFactory,
         IOptionsMonitor<ClearMLOptions> options,
         IClearMLAuthenticationService clearMLAuthService,
         IHostEnvironment env
     )
     {
-        _httpClient = httpClient;
+        _httpClient = httpClientFactory.CreateClient("ClearML");
         _options = options;
         _clearMLAuthService = clearMLAuthService;
         _env = env;
@@ -191,7 +191,7 @@ private async Task<IReadOnlyList<ClearMLTask>> GetTasksAsync(
         CancellationToken cancellationToken = default
     )
     {
-        var request = new HttpRequestMessage(HttpMethod.Post, $"{_options.CurrentValue.ApiServer}/{service}.{action}")
+        var request = new HttpRequestMessage(HttpMethod.Post, $"{service}.{action}")
         {
             Content = new StringContent(body.ToJsonString(), Encoding.UTF8, "application/json")
         };

diff --git a/src/SIL.Machine.Serval.EngineServer/Program.cs b/src/SIL.Machine.Serval.EngineServer/Program.cs
@@ -9,7 +9,8 @@
     .AddMongoDataAccess()
     .AddMongoHangfireJobClient()
     .AddServalTranslationEngineService()
-    .AddBuildJobService();
+    .AddBuildJobService()
+    .AddClearMLService();
 if (builder.Environment.IsDevelopment())
     builder.Services
         .AddOpenTelemetry()

diff --git a/src/SIL.Machine.Serval.EngineServer/appsettings.json b/src/SIL.Machine.Serval.EngineServer/appsettings.json
@@ -1,4 +1,7 @@
 {
+  "ConnectionStrings": {
+    "ClearML": "https://api.sil.hosted.allegro.ai"
+  },
   "AllowedHosts": "*",
   "Service": {
     "ServiceId": "machine_engine"
@@ -17,7 +20,6 @@
     "EnginesDir": "/var/lib/machine/engines"
   },
   "ClearML": {
-    "ApiServer": "https://api.sil.hosted.allegro.ai",
     "BuildPollingEnabled": true
   },
   "Logging": {

diff --git a/src/SIL.Machine.Serval.JobServer/Program.cs b/src/SIL.Machine.Serval.JobServer/Program.cs
@@ -8,7 +8,8 @@
     .AddMongoHangfireJobClient()
     .AddHangfireJobServer()
     .AddServalPlatformService()
-    .AddBuildJobService();
+    .AddBuildJobService()
+    .AddClearMLService();
 if (builder.Environment.IsDevelopment())
     builder.Services
         .AddOpenTelemetry()

diff --git a/src/SIL.Machine.Serval.JobServer/appsettings.json b/src/SIL.Machine.Serval.JobServer/appsettings.json
@@ -1,4 +1,7 @@
 {
+  "ConnectionStrings": {
+    "ClearML": "https://api.sil.hosted.allegro.ai"
+  },
   "AllowedHosts": "*",
   "Service": {
     "ServiceId": "machine_job"
@@ -17,7 +20,6 @@
     "EnginesDir": "/var/lib/machine/engines"
   },
   "ClearML": {
-    "ApiServer": "https://api.sil.hosted.allegro.ai",
     "BuildPollingEnabled": false
   },
   "Logging": {

diff --git a/tests/SIL.Machine.AspNetCore.Tests/SIL.Machine.AspNetCore.Tests.csproj b/tests/SIL.Machine.AspNetCore.Tests/SIL.Machine.AspNetCore.Tests.csproj
@@ -11,8 +11,8 @@
 	<ItemGroup>
 		<PackageReference Include="Hangfire.InMemory" Version="0.5.1" />
 		<PackageReference Include="coverlet.collector" Version="6.0.0">
-		  <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-		  <PrivateAssets>all</PrivateAssets>
+			<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+			<PrivateAssets>all</PrivateAssets>
 		</PackageReference>
 		<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.5.0" />
 		<PackageReference Include="NSubstitute" Version="5.0.0" />
@@ -29,7 +29,7 @@
 			<PrivateAssets>all</PrivateAssets>
 			<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
 		</PackageReference>
-		<PackageReference Include="RichardSzalay.MockHttp" Version="6.0.0" />
+		<PackageReference Include="RichardSzalay.MockHttp" Version="7.0.0" />
 	</ItemGroup>
 
 	<ItemGroup>

diff --git a/tests/SIL.Machine.AspNetCore.Tests/Services/ClearMLServiceTests.cs b/tests/SIL.Machine.AspNetCore.Tests/Services/ClearMLServiceTests.cs
@@ -17,20 +17,17 @@ public async Task CreateTaskAsync()
             .WithPartialContent("\\u0027src_lang\\u0027: \\u0027spa_Latn\\u0027")
             .WithPartialContent("\\u0027trg_lang\\u0027: \\u0027eng_Latn\\u0027")
             .Respond("application/json", "{ \"data\": { \"id\": \"projectId\" } }");
+        HttpClient httpClient = mockHttp.ToHttpClient();
+        httpClient.BaseAddress = new Uri(ApiServer);
 
         var options = Substitute.For<IOptionsMonitor<ClearMLOptions>>();
-        options.CurrentValue.Returns(
-            new ClearMLOptions
-            {
-                ApiServer = ApiServer,
-                AccessKey = AccessKey,
-                SecretKey = SecretKey
-            }
-        );
+        options.CurrentValue.Returns(new ClearMLOptions { AccessKey = AccessKey, SecretKey = SecretKey });
         var authService = Substitute.For<IClearMLAuthenticationService>();
         authService.GetAuthTokenAsync().Returns(Task.FromResult("accessToken"));
         var env = new HostingEnvironment { EnvironmentName = Environments.Development };
-        var service = new ClearMLService(mockHttp.ToHttpClient(), options, authService, env);
+        var httpClientFactory = Substitute.For<IHttpClientFactory>();
+        httpClientFactory.CreateClient("ClearML").Returns(httpClient);
+        var service = new ClearMLService(httpClientFactory, options, authService, env);
 
         string script =
             "from machine.jobs.build_nmt_engine import run\n"