Skip to content

Commit

Permalink
Renamed ClearML-related configuration elements
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Enkidu93 committed Oct 10, 2023
1 parent ef591f4 commit 24d7882
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 15 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
6 changes: 2 additions & 4 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -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/
Expand Down
28 changes: 18 additions & 10 deletions src/Serval.Client/Client.g.cs
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,10 @@ public partial interface ITranslationEnginesClient
/// <br/>untranslated text but no translated text. If a corpus is a Paratext project,
/// <br/>you may flag a subset of books for pretranslation by including their [abbreviations](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs)
/// <br/>in the textIds parameter. If the engine does not support pretranslation, these fields have no effect.
/// <br/>
/// <br/>The `"options"` parameter of the build config provides the ability to pass build configuration parameters as a JSON string.
/// <br/>A typical use case would be to set `"options"` to `"{\"max_steps\":10}"` in order to configure the maximum
/// <br/>number of training iterations in order to reduce turnaround time for testing purposes.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="buildConfig">The build config (see remarks)</param>
Expand All @@ -1011,13 +1015,13 @@ public partial interface ITranslationEnginesClient
/// Get a build job
/// </summary>
/// <remarks>
/// 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,
/// <br/>will be immediately returned. If `minRevision` is defined, Serval will wait for
/// <br/>up to 40 seconds for the engine to build to the `minRevision` specified, else
/// <br/>will timeout.
/// <br/>A use case is to actively query the state of the current build, where the subsequent
/// <br/>request sets the `minRevision` to the returned `revision` + 1. Note: this method
/// <br/>should use request throttling.
/// <br/>request sets the `minRevision` to the returned `revision` + 1 and timeouts are handled gracefully.
/// <br/>Note: this method should use request throttling.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="buildId">The build job id</param>
Expand All @@ -1031,7 +1035,7 @@ public partial interface ITranslationEnginesClient
/// Get the currently running build job for a translation engine
/// </summary>
/// <remarks>
/// 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`.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="minRevision">The minimum revision</param>
Expand Down Expand Up @@ -2801,6 +2805,10 @@ public string BaseUrl
/// <br/>untranslated text but no translated text. If a corpus is a Paratext project,
/// <br/>you may flag a subset of books for pretranslation by including their [abbreviations](https://github.com/sillsdev/libpalaso/blob/master/SIL.Scripture/Canon.cs)
/// <br/>in the textIds parameter. If the engine does not support pretranslation, these fields have no effect.
/// <br/>
/// <br/>The `"options"` parameter of the build config provides the ability to pass build configuration parameters as a JSON string.
/// <br/>A typical use case would be to set `"options"` to `"{\"max_steps\":10}"` in order to configure the maximum
/// <br/>number of training iterations in order to reduce turnaround time for testing purposes.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="buildConfig">The build config (see remarks)</param>
Expand Down Expand Up @@ -2922,13 +2930,13 @@ public string BaseUrl
/// Get a build job
/// </summary>
/// <remarks>
/// 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,
/// <br/>will be immediately returned. If `minRevision` is defined, Serval will wait for
/// <br/>up to 40 seconds for the engine to build to the `minRevision` specified, else
/// <br/>will timeout.
/// <br/>A use case is to actively query the state of the current build, where the subsequent
/// <br/>request sets the `minRevision` to the returned `revision` + 1. Note: this method
/// <br/>should use request throttling.
/// <br/>request sets the `minRevision` to the returned `revision` + 1 and timeouts are handled gracefully.
/// <br/>Note: this method should use request throttling.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="buildId">The build job id</param>
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -3053,7 +3061,7 @@ public string BaseUrl
/// Get the currently running build job for a translation engine
/// </summary>
/// <remarks>
/// 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`.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="minRevision">The minimum revision</param>
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </remarks>
/// <param name="id">The translation engine id</param>
/// <param name="buildConfig">The build config (see remarks)</param>
Expand Down
6 changes: 5 additions & 1 deletion tests/Serval.E2ETests/ServalClientHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PretranslateCorpusConfig>() };
TranslationBuildConfig = new TranslationBuildConfig
{
Pretranslate = new List<PretranslateCorpusConfig>(),
Options = "{\"max_steps\":10}"
};
}

public TranslationBuildConfig TranslationBuildConfig { get; set; }
Expand Down

0 comments on commit 24d7882

Please sign in to comment.