Skip to content

Commit

Permalink
Update from reviewer comments
Browse files Browse the repository at this point in the history
- Make a normal background job (sorry)
- Make it run once a day
  • Loading branch information
johnml1135 committed Feb 6, 2024
1 parent 2c5ff85 commit 8d6702d
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -405,29 +405,10 @@ public static IMachineBuilder AddBuildJobService(this IMachineBuilder builder)
return builder;
}

public static IMachineBuilder AddModelCleanupJob(this IMachineBuilder builder, string? connectionString = null)
public static IMachineBuilder AddModelCleanupService(this IMachineBuilder builder)
{
connectionString ??= builder.Configuration?.GetConnectionString("Hangfire");
if (connectionString is null)
throw new InvalidOperationException("Hangfire connection string is required");

var mongoUrl = MongoUrl.Create(connectionString);
JobStorage.Current = new MongoStorage(
MongoClientSettings.FromUrl(mongoUrl),
mongoUrl.DatabaseName,
GetMongoStorageOptions()
);
builder.Services.AddSingleton<ICleanupOldModelsJob, CleanupOldModelsJob>();
RecurringJob.AddOrUpdate<ICleanupOldModelsJob>(
recurringJobId: "cleanup-job",
queue: "cleanup-job",
methodCall: o => o.RunAsync(),
cronExpression: Cron.Minutely
);
builder.Services.AddHangfireServer(o =>
{
o.Queues = ["cleanup-job"];
});
builder.Services.AddSingleton<CleanupOldModelsService>();
builder.Services.AddHostedService(p => p.GetRequiredService<CleanupOldModelsService>());
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
namespace SIL.Machine.AspNetCore.Services;

public class CleanupOldModelsJob(
public class CleanupOldModelsService(
IServiceProvider services,
ISharedFileService sharedFileService,
IRepository<TranslationEngine> engines,
ILogger<CleanupOldModelsJob> logger
) : ICleanupOldModelsJob
ILogger<CleanupOldModelsService> logger
) : RecurrentTask("Cleanup Old Models Service", services, RefreshPeriod, logger)
{
public ISharedFileService SharedFileService { get; } = sharedFileService;
private ILogger<CleanupOldModelsJob> _logger = logger;
private ILogger<CleanupOldModelsService> _logger = logger;
private IRepository<TranslationEngine> _engines = engines;
private List<string> _filesPreviouslyMarkedForDeletion = [];
private readonly List<string> _filesNewlyMarkedForDeletion = [];
private static readonly TimeSpan RefreshPeriod = TimeSpan.FromDays(1);

public async Task RunAsync()
protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken)
{
await CheckModelsAsync();
}

public async Task CheckModelsAsync()
{
_logger.LogDebug("Running model cleanup job");
_logger.LogInformation("Running model cleanup job");
var paths = await SharedFileService.ListFilesAsync(ISharedFileService.ModelDirectory);
foreach (string path in paths)
{
Expand Down
6 changes: 0 additions & 6 deletions src/SIL.Machine.AspNetCore/Services/ICleanupJob.cs

This file was deleted.

3 changes: 2 additions & 1 deletion src/SIL.Machine.Serval.EngineServer/Program.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Hangfire;
using OpenTelemetry.Trace;
using SIL.Machine.AspNetCore.Services;

var builder = WebApplication.CreateBuilder(args);

Expand All @@ -10,7 +11,7 @@
.AddMongoHangfireJobClient()
.AddServalTranslationEngineService()
.AddBuildJobService()
.AddModelCleanupJob()
.AddModelCleanupService()
.AddClearMLService();

if (builder.Environment.IsDevelopment())
Expand Down
11 changes: 6 additions & 5 deletions tests/SIL.Machine.AspNetCore.Tests/Services/CleanupJobTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,22 @@ async Task WriteFileStub(string path, string content)
}

[Test]
public async Task RunAsync_ValidFiles()
public async Task DoWorkAsync_ValidFiles()
{
await SetUpAsync();
var cleanupJob = new CleanupOldModelsJob(
var cleanupJob = new CleanupOldModelsService(
Substitute.For<IServiceProvider>(),
_sharedFileService,
_engines,
Substitute.For<ILogger<CleanupOldModelsJob>>()
Substitute.For<ILogger<CleanupOldModelsService>>()
);
await cleanupJob.RunAsync();
await cleanupJob.CheckModelsAsync();
// both valid and invalid files still exist after running once
Assert.That(
_sharedFileService.ListFilesAsync("models").Result.ToHashSet(),
Is.EquivalentTo(validFiles.Concat(invalidFiles).ToHashSet())
);
await cleanupJob.RunAsync();
await cleanupJob.CheckModelsAsync();
// only valid files exist after running twice
Assert.That(
_sharedFileService.ListFilesAsync("models").Result.ToHashSet(),
Expand Down

0 comments on commit 8d6702d

Please sign in to comment.