From 0f7d71f3cfd61cf53ab7fd784d1b0af6ac064dd6 Mon Sep 17 00:00:00 2001 From: Enkidu93 Date: Tue, 10 Oct 2023 09:09:57 -0400 Subject: [PATCH] Renamed ClearML-related configuration elements Removed maxSteps from yml and manually passed maxSteps=10 in the E2E tests Updated the swagger documentation to provide an explanation about the options parameter Updated the README to point to production-serval swagger documentation --- README.md | 3 ++ docker-compose.yml | 6 ++-- src/Serval.Client/Client.g.cs | 28 ++++++++++++------- .../TranslationEnginesController.cs | 4 +++ tests/Serval.E2ETests/ServalClientHelper.cs | 6 +++- 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index f467eedb..5b37120c 100644 --- a/README.md +++ b/README.md @@ -117,3 +117,6 @@ All C# code should be formatted using [CSharpier](https://csharpier.com/). The b [Here](https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names) is a good overview of naming conventions. [Here](https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/coding-conventions) is a good overview of coding conventions. If you want to get in to even more detail, check out the [Framework design guidelines](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/). +## Documentation + +See the Swagger documentation for Serval [here](https://prod.serval-api.org/swagger/index.html). \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index 9d13514e..fd9a50fa 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -92,8 +92,7 @@ services: - ASPNETCORE_TranslationEngines__1=Nmt - ClearML__ApiServer=https://api.sil.hosted.allegro.ai - ClearML__Queue=production - - ClearML__DockerImage=ghcr.io/sillsdev/machine.py:0.9.5 - - ClearML__MaxSteps=10 + - ClearML__DockerImage=ghcr.io/sillsdev/machine.py:0.9.5.1 - "ClearML__AccessKey=${ClearML_AccessKey:?access key needed}" - "ClearML__SecretKey=${ClearML_SecretKey:?secret key needed}" - SharedFile__Uri=s3://aqua-ml-data/docker-compose/ @@ -138,8 +137,7 @@ services: - ASPNETCORE_TranslationEngines__1=Nmt - ClearML__ApiServer=https://api.sil.hosted.allegro.ai - ClearML__Queue=lambert_24gb - - ClearML__DockerImage=ghcr.io/sillsdev/machine.py:0.9.5 - - ClearML__MaxSteps=10 + - ClearML__DockerImage=ghcr.io/sillsdev/machine.py:0.9.5.1 - "ClearML__AccessKey=${ClearML_AccessKey:?access key needed}" - "ClearML__SecretKey=${ClearML_SecretKey:?secret key needed}" - SharedFile__Uri=s3://aqua-ml-data/docker-compose/ diff --git a/src/Serval.Client/Client.g.cs b/src/Serval.Client/Client.g.cs index e02ca400..0dd43d87 100644 --- a/src/Serval.Client/Client.g.cs +++ b/src/Serval.Client/Client.g.cs @@ -999,6 +999,10 @@ public partial interface ITranslationEnginesClient ///
untranslated text but no translated text. If a corpus is a Paratext project, ///
you may flag a subset of books for pretranslation by including their [abbreviations](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs) ///
in the textIds parameter. If the engine does not support pretranslation, these fields have no effect. + ///
+ ///
The `"options"` parameter of the build config provides the ability to pass build configuration parameters as a JSON string. + ///
A typical use case would be to set `"options"` to `"{\"max_steps\":10}"` in order to configure the maximum + ///
number of training iterations in order to reduce turnaround time for testing purposes. /// /// The translation engine id /// The build config (see remarks) @@ -1011,13 +1015,13 @@ public partial interface ITranslationEnginesClient /// Get a build job /// /// - /// If the `minRevision` is not defined, the current build at whatever state it is + /// If the `minRevision` is not defined, the current build, at whatever state it is, ///
will be immediately returned. If `minRevision` is defined, Serval will wait for ///
up to 40 seconds for the engine to build to the `minRevision` specified, else ///
will timeout. ///
A use case is to actively query the state of the current build, where the subsequent - ///
request sets the `minRevision` to the returned `revision` + 1. Note: this method - ///
should use request throttling. + ///
request sets the `minRevision` to the returned `revision` + 1 and timeouts are handled gracefully. + ///
Note: this method should use request throttling. /// /// The translation engine id /// The build job id @@ -1031,7 +1035,7 @@ public partial interface ITranslationEnginesClient /// Get the currently running build job for a translation engine /// /// - /// See "Get a Build Job" for details on minimum revision. + /// See documentation on endpoint /translation/engines/{id}/builds/{id} - "Get a Build Job" for details on using `minRevision`. /// /// The translation engine id /// The minimum revision @@ -2801,6 +2805,10 @@ public string BaseUrl ///
untranslated text but no translated text. If a corpus is a Paratext project, ///
you may flag a subset of books for pretranslation by including their [abbreviations](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs) ///
in the textIds parameter. If the engine does not support pretranslation, these fields have no effect. + ///
+ ///
The `"options"` parameter of the build config provides the ability to pass build configuration parameters as a JSON string. + ///
A typical use case would be to set `"options"` to `"{\"max_steps\":10}"` in order to configure the maximum + ///
number of training iterations in order to reduce turnaround time for testing purposes. /// /// The translation engine id /// The build config (see remarks) @@ -2922,13 +2930,13 @@ public string BaseUrl /// Get a build job /// /// - /// If the `minRevision` is not defined, the current build at whatever state it is + /// If the `minRevision` is not defined, the current build, at whatever state it is, ///
will be immediately returned. If `minRevision` is defined, Serval will wait for ///
up to 40 seconds for the engine to build to the `minRevision` specified, else ///
will timeout. ///
A use case is to actively query the state of the current build, where the subsequent - ///
request sets the `minRevision` to the returned `revision` + 1. Note: this method - ///
should use request throttling. + ///
request sets the `minRevision` to the returned `revision` + 1 and timeouts are handled gracefully. + ///
Note: this method should use request throttling. /// /// The translation engine id /// The build job id @@ -3020,7 +3028,7 @@ public string BaseUrl if (status_ == 408) { string responseText_ = ( response_.Content == null ) ? string.Empty : await response_.Content.ReadAsStringAsync().ConfigureAwait(false); - throw new ServalApiException("The long polling request timed out", status_, responseText_, headers_, null); + throw new ServalApiException("The long polling request timed out. This is expected behavior if you\'re using long-polling with the minRevision strategy specified in the docs", status_, responseText_, headers_, null); } else if (status_ == 503) @@ -3053,7 +3061,7 @@ public string BaseUrl /// Get the currently running build job for a translation engine /// /// - /// See "Get a Build Job" for details on minimum revision. + /// See documentation on endpoint /translation/engines/{id}/builds/{id} - "Get a Build Job" for details on using `minRevision`. /// /// The translation engine id /// The minimum revision @@ -3146,7 +3154,7 @@ public string BaseUrl if (status_ == 408) { string responseText_ = ( response_.Content == null ) ? string.Empty : await response_.Content.ReadAsStringAsync().ConfigureAwait(false); - throw new ServalApiException("The long polling request timed out. Did you start the build?", status_, responseText_, headers_, null); + throw new ServalApiException("The long polling request timed out. This is expected behavior if you\'re using long-polling with the minRevision strategy specified in the docs", status_, responseText_, headers_, null); } else if (status_ == 503) diff --git a/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval.Translation/Controllers/TranslationEnginesController.cs index eaa55d75..953063bd 100644 --- a/src/Serval.Translation/Controllers/TranslationEnginesController.cs +++ b/src/Serval.Translation/Controllers/TranslationEnginesController.cs @@ -740,6 +740,10 @@ CancellationToken cancellationToken /// untranslated text but no translated text. If a corpus is a Paratext project, /// you may flag a subset of books for pretranslation by including their [abbreviations](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs) /// in the textIds parameter. If the engine does not support pretranslation, these fields have no effect. + /// + /// The `"options"` parameter of the build config provides the ability to pass build configuration parameters as a JSON string. + /// A typical use case would be to set `"options"` to `"{\"max_steps\":10}"` in order to configure the maximum + /// number of training iterations in order to reduce turnaround time for testing purposes. /// /// The translation engine id /// The build config (see remarks) diff --git a/tests/Serval.E2ETests/ServalClientHelper.cs b/tests/Serval.E2ETests/ServalClientHelper.cs index 6a944910..df1b209f 100644 --- a/tests/Serval.E2ETests/ServalClientHelper.cs +++ b/tests/Serval.E2ETests/ServalClientHelper.cs @@ -29,7 +29,11 @@ public ServalClientHelper(string audience, string prefix = "SCE_", bool ignoreSS $"Bearer {GetAuth0Authentication(env["authUrl"], audience, env["clientId"], env["clientSecret"]).Result}" ); _prefix = prefix; - TranslationBuildConfig = new TranslationBuildConfig { Pretranslate = new List() }; + TranslationBuildConfig = new TranslationBuildConfig + { + Pretranslate = new List(), + Options = "{\"max_steps\":10}" + }; } public TranslationBuildConfig TranslationBuildConfig { get; set; }