environment types

[johnml1135]

addresses #178

---

This change is 

[codecov-commenter]

Codecov Report

All modified lines are covered by tests ✅

📢 Thoughts on this report? Let us know!.

[ddaspit]

Do we want to take this opportunity to do #157?

Reviewed 9 of 10 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @johnml1135)

---

deploy/qa-ext-values.yaml line 9 at r2 (raw file):

servalImage: ghcr.io/sillsdev/serval:0.7.1
machineImage: ghcr.io/sillsdev/machine:3.3.2
machineEnvironment: DummyBuild

ASP.NET Core has three deployment environments built-in: Production, Staging, and Development. This looks like it should be Staging.

---

deploy/qa-int-values.yaml line 9 at r2 (raw file):

servalImage: ghcr.io/sillsdev/serval:0.7.1
machineImage: ghcr.io/sillsdev/machine:3.3.2
machineEnvironment: DummyBuild

This looks like it should be Staging.

[johnml1135]

I did - it was just that there wasn't as much to pull away - 80% is networking config.

[johnml1135]

deploy/qa-ext-values.yaml line 9 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

ASP.NET Core has three deployment environments built-in: Production, Staging, and Development. This looks like it should be Staging.

Ok - then I will make sure that on the Serval Readme that "Staging" does "dummy builds" that don't produce helpful output.

[johnml1135]

deploy/qa-int-values.yaml line 9 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

This looks like it should be Staging.

ok

[ddaspit]

I was really hoping to move everything that isn't a connection string or secret to the appsettings. What about ClearML__Queue, ClearML__DockerImage, ClearML__Project, TranslationEngines in Machine?

Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johnml1135)

---

deploy/values.yaml line 11 at r3 (raw file):

machineImage: ghcr.io/sillsdev/machine:3.3.2
ClearMLDockerImage: ghcr.io/sillsdev/machine.py:0.9.5
ClearMLMaxSteps: 20000

I don't think this is needed anymore.

[johnml1135]

I don't want to have to release a new docker image for Machine just because there is an update to Machine.py - nor would I want the Machine and Machine.py images embedded with deployment parameters, such as the URL that it is deployed at. Those params belong in the helm charts. If I want to make a local deployment to minikube, I would want to choose between production, staging and development, potentially using a different ClearML instance and S3 bucket, etc.

[johnml1135]

deploy/values.yaml line 11 at r3 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I don't think this is needed anymore.

Done.

[ddaspit]

Ok, so we can leave ClearML__DockerImage out of the appsettings, but what about the other config options: ClearML__Queue, ClearML__Project, and TranslationEngines?

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)

[johnml1135]

Queue and project are dependent on the deployment. Translation engines - I could go either way, but as of now, these "deployments" should have both Nmt and SmtTransfer engines. Why not put them in the generic appsettings then?

[ddaspit]

Actually, I don't think we even need to specify TranslationEngines. By default, it uses both SmtTransfer and Nmt. If we want to make it explicit, I think it would make sense to set TranslationEngines to appsettings.json. For now, we can just remove it from here.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)

[johnml1135]

Ok, I'll remove them.

[ddaspit]

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)