Move environment option into build stanza and document it#2807
Conversation
The environment cog.yaml option is a build-time setting that adds environment variables to the Docker image, so it belongs in the build stanza rather than at the top level. - Move Environment field from Config to Build struct - Accept top-level environment with a deprecation warning for backward compatibility; error if set in both places - Add environment to the JSON schema under build properties - Add documentation to docs/yaml.md - Update integration test and YAML overwrite test fixtures - Regenerate docs/llms.txt
|
LGTM |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Moves environment from top-level to build stanza with backward-compatibility migration and deprecation warnings.
Issues:
-
Missing unit tests for backward-compatibility logic (high) — The new
resolveEnvironment()function invalidate.gohas three branches (top-level only → warning, both set → error, build-only → no-op), but none are covered by unit tests. Thevalidate_test.gofile should have tests verifying each of these behaviors, similar to the existingTestValidateConfigFileRunPredictCompatibilitytests. -
Missing unit tests for
configFileToConfig()environment migration (medium) —parse.gonow migrates top-levelenvironmentintoBuild.Environmentduring parsing (for theFromYAML()path which skips validation). There are no tests verifying this migration works correctly. A test inconfig_test.goshould verify thatFromYAML()with top-levelenvironmentcorrectly populatesconfig.Build.Environment.
|
I'm Bonk, and I've done a quick review of your PR. Moves Issues:
|
Summary
environmentcog.yaml option from the top level into thebuild:stanza, since it's a build-time setting that adds env vars to the Docker imageenvironmentindocs/yaml.mdenvironmentstill works but emits a deprecation warningChanges
Config structs:
EnvironmentandparsedEnvironmentmoved fromConfigtoBuild. The canonical location is nowbuild.environment.Backward compatibility: A new
resolveEnvironment()validation step handles migration:build.environmentwith a deprecation warningbuild.environmentonly → no-opJSON schema:
environmentadded tobuildproperties; top-level kept with deprecation note.Documentation: New
### environmentsection under## buildindocs/yaml.md(alphabetically betweencudaandgpu). Documents theNAME=valueformat, provides an example, and notes reserved variable names.Tests: Integration test and YAML overwrite test fixtures updated to use the new location.
Example