Should the Elasticsearch host and telemetry settings be overridable from the environment? #60

Where this comes from

The Helm chart used to set ES_HOST, OPENTELEMETRY_ENABLED and PORT on the container, but NodeNorm never read them: it doesn't use biothings' settings loader (the one that honours environment variables) and takes its settings only from src/nodenorm/config/config.default.json with docker/configuration/config.json merged over it. PR #50 removes the three variables from the chart (commit c51e730) rather than wire them in. Separately, NODENORM_DEPLOYMENT_ENV (#56–#58) is now read from the environment, so the app does honour one variable.

While merging main into #50 the question came up: should the other deployment settings be overridable from the environment too? This issue is to let the developers and ITRB decide rather than settle it inside a config PR.

The question for you

Does any deployment need the Elasticsearch host (or the telemetry settings) to differ between CI, Test and Prod without rebuilding the image?

If yes, env overrides are the obvious route and are small to add (see below). If no, the current arrangement — one reviewed, checked-in host in docker/configuration/config.json — is the simpler one to keep.

Why it was deferred rather than done in #50

• No demonstrated need. The chart passed ES_HOST for years and the app ignored it, and every deployment worked, so the baked-in in-cluster host (elasticsearch.es-core-components.svc.cluster.local:9200) has fitted CI, Test and Prod alike.
• PORT must never be honoured. The chart's port: 8000 is what Caddy listens on (docker/configuration/Caddyfile); the app runs on 9001 behind it (supervisord.conf passes --port=9001). Reading PORT would make the two collide, so any implementation covers ES_HOST and OPENTELEMETRY_ENABLED only.
• Honouring env vars would activate values that are inert today. On main, deploy/values.yaml ships the placeholder es_host: ES_HOST_VALUE for ITRB's values files to substitute; Apply the configured Tornado settings, with debug mode off when deployed, and trim the deployment config to its own settings #50 removes that key, but the same placeholder remains in the gov.nih.ncats.appconn1host annotation, and the NCATS annotations give the host and port separately rather than as the URL the Elasticsearch client accepts. If an environment rendered the placeholder unsubstituted, or set a bare hostname, nothing happens now; once honoured, the client would get a non-URL and the pod would fail its startup probe. An implementation needs URL validation with a clear startup error.
• A checked-in host is a reasonable property for a production pointer. It is reviewed when it changes, which is the same reasoning Apply the configured Tornado settings, with debug mode off when deployed, and trim the deployment config to its own settings #50 uses for pinning ES_INDEX and ES_ALIAS in the deployment config.

The counter-argument is consistency: with NODENORM_DEPLOYMENT_ENV in place, "the app reads no env vars" is a current fact rather than a principle.

What implementing it would take

About twenty lines plus tests and docs, all in one place:

• In load_configuration() (src/nodenorm/namespace.py), after merge_configuration, apply a table of ENV_VAR → (section, key) for ES_HOST and OPENTELEMETRY_ENABLED, coercing each value to the type of the default (strings and true/1 booleans, as biothings' ConfigModule does). Precedence: environment > --conf file > default.
• Validate ES_HOST as a URL with a scheme and fail at startup with a message naming the variable.
• Tests in tests/test_configuration.py, next to the existing merge tests.
• Restore env: entries in deploy/templates/deployment.yaml for those two variables only, and a README note on precedence.

🤖 Generated with Claude Code