Skip to content

Commit

Permalink
Address PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
Enkidu93 committed Oct 18, 2024
1 parent 6ddce79 commit 113ba28
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 69 deletions.
2 changes: 1 addition & 1 deletion src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public abstract class EntityServiceBase<T>(IRepository<T> entities)
{
protected IRepository<T> Entities { get; } = entities;

public async Task<T> GetAsync(string id, CancellationToken cancellationToken = default)
public virtual async Task<T> GetAsync(string id, CancellationToken cancellationToken = default)
{
T? entity = await Entities.GetAsync(id, cancellationToken);
if (entity is null)
Expand Down
14 changes: 10 additions & 4 deletions src/Serval/src/Serval.Translation/Services/BuildService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ public class BuildService(IRepository<Build> builds) : EntityServiceBase<Build>(
{
public async Task<IEnumerable<Build>> GetAllAsync(string parentId, CancellationToken cancellationToken = default)
{
return await Entities.GetAllAsync(e => e.EngineRef == parentId, cancellationToken);
return await Entities.GetAllAsync(e => e.EngineRef == parentId && (e.IsInitialized ?? true), cancellationToken);
}

public Task<Build?> GetActiveAsync(string parentId, CancellationToken cancellationToken = default)
{
return Entities.GetAsync(
b => b.EngineRef == parentId && (b.State == JobState.Active || b.State == JobState.Pending),
b =>
b.EngineRef == parentId
&& (b.IsInitialized ?? true)
&& (b.State == JobState.Active || b.State == JobState.Pending),
cancellationToken
);
}
Expand All @@ -21,7 +24,7 @@ public Task<EntityChange<Build>> GetNewerRevisionAsync(
CancellationToken cancellationToken = default
)
{
return GetNewerRevisionAsync(e => e.Id == id, minRevision, cancellationToken);
return GetNewerRevisionAsync(e => e.Id == id && (e.IsInitialized ?? true), minRevision, cancellationToken);
}

public Task<EntityChange<Build>> GetActiveNewerRevisionAsync(
Expand All @@ -31,7 +34,10 @@ public Task<EntityChange<Build>> GetActiveNewerRevisionAsync(
)
{
return GetNewerRevisionAsync(
b => b.EngineRef == parentId && (b.State == JobState.Active || b.State == JobState.Pending),
b =>
b.EngineRef == parentId
&& (b.IsInitialized ?? true)
&& (b.State == JobState.Active || b.State == JobState.Pending),
minRevision,
cancellationToken
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,7 @@
using Microsoft.Extensions.DependencyInjection;

namespace Serval.Translation.Services;

public class EngineCleanupService(
IServiceProvider services,
ILogger<EngineCleanupService> logger,
TimeSpan? timeout = null
) : UninitializedCleanupService<Engine>(services, logger, timeout)
{
public EngineService? EngineService { get; set; }

protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken cancellationToken)
{
EngineService = scope.ServiceProvider.GetRequiredService<EngineService>();
await base.DoWorkAsync(scope, cancellationToken);
}

protected override async Task DeleteEntityAsync(
IRepository<Engine> engines,
Engine engine,
CancellationToken cancellationToken
)
{
if (EngineService == null)
return;
await EngineService.DeleteAsync(engine.Id, cancellationToken);
}
}
) : UninitializedCleanupService<Engine>(services, logger, timeout) { }
10 changes: 9 additions & 1 deletion src/Serval/src/Serval.Translation/Services/EngineService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ IScriptureDataFileService scriptureDataFileService
private readonly ILogger<EngineService> _logger = loggerFactory.CreateLogger<EngineService>();
private readonly IScriptureDataFileService _scriptureDataFileService = scriptureDataFileService;

public override async Task<Engine> GetAsync(string id, CancellationToken cancellationToken = default)
{
Engine engine = await base.GetAsync(id, cancellationToken);
if (!(engine.IsInitialized ?? true))
throw new EntityNotFoundException($"Could not find the {typeof(Engine).Name} '{id}'.");
return engine;
}

public async Task<Models.TranslationResult> TranslateAsync(
string engineId,
string segment,
Expand Down Expand Up @@ -153,7 +161,7 @@ await Entities.UpdateAsync(
u.Set(e => e.IsInitialized, true);
u.Set(e => e.IsModelPersisted, engine.IsModelPersisted);
},
cancellationToken: cancellationToken
cancellationToken: CancellationToken.None
);
}
catch (RpcException rpcex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ protected override async Task DoWorkAsync(IServiceScope scope, CancellationToken
public async Task CheckEntitiesAsync(IRepository<T> entities, CancellationToken cancellationToken)
{
var now = DateTime.UtcNow;
IEnumerable<T> uninitializedEntities = (await entities.GetAllAsync(cancellationToken)).Where(b =>
!(b.IsInitialized ?? true) && (now - (b.DateCreated ?? DateTime.UtcNow)) > _timeout
IEnumerable<T> uninitializedEntities = await entities.GetAllAsync(
e => !(e.IsInitialized ?? true) && e.DateCreated != null && (now - e.DateCreated) > _timeout,
cancellationToken
);

foreach (T entity in uninitializedEntities)
{
_logger.LogInformation(
"Deleting {type} {id} because it was never successfully started",
"Deleting {type} {id} because it was never successfully initialized.",
typeof(T),
entity.Id
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
using Google.Protobuf.WellKnownTypes;
using MassTransit.Mediator;
using Serval.Translation.V1;

namespace Serval.Translation.Services;

[TestFixture]
Expand Down Expand Up @@ -54,28 +50,6 @@ public TestEnvironment()
Substitute.For<ILogger<EngineCleanupService>>(),
TimeSpan.Zero
);

var translationServiceClient = Substitute.For<TranslationEngineApi.TranslationEngineApiClient>();
translationServiceClient.DeleteAsync(Arg.Any<DeleteRequest>()).Returns(CreateAsyncUnaryCall(new Empty()));

GrpcClientFactory grpcClientFactory = Substitute.For<GrpcClientFactory>();
grpcClientFactory
.CreateClient<TranslationEngineApi.TranslationEngineApiClient>("Nmt")
.Returns(translationServiceClient);

var engineService = new EngineService(
Engines,
new MemoryRepository<Build>(),
new MemoryRepository<Pretranslation>(),
Substitute.For<IScopedMediator>(),
grpcClientFactory,
Substitute.For<IOptionsMonitor<DataFileOptions>>(),
new MemoryDataAccessContext(),
new LoggerFactory(),
Substitute.For<IScriptureDataFileService>()
);

Service.EngineService = engineService;
}

public EngineCleanupService Service { get; }
Expand All @@ -84,16 +58,5 @@ public async Task CheckEnginesAsync()
{
await Service.CheckEntitiesAsync(Engines, CancellationToken.None);
}

private static AsyncUnaryCall<TResponse> CreateAsyncUnaryCall<TResponse>(TResponse response)
{
return new AsyncUnaryCall<TResponse>(
Task.FromResult(response),
Task.FromResult(new Metadata()),
() => Status.DefaultSuccess,
() => new Metadata(),
() => { }
);
}
}
}

0 comments on commit 113ba28

Please sign in to comment.