refactor(config): implement three-phase Parse → Validate → Complete architecture#2696
Merged
tempusfrangit merged 10 commits intomainfrom Feb 9, 2026
Merged
refactor(config): implement three-phase Parse → Validate → Complete architecture#2696tempusfrangit merged 10 commits intomainfrom
tempusfrangit merged 10 commits intomainfrom
Conversation
markphelps
commented
Feb 6, 2026
73957d3 to
7260c9b
Compare
…rchitecture This refactors the config system to use a cleaner three-phase architecture: 1. **Parse**: YAML → ConfigFile (raw representation with pointer fields) 2. **Validate**: Pure validation with no mutations, returns structured errors 3. **Complete**: ConfigFile → Config with defaults and CUDA resolution - errors.go: Structured error types (ParseError, SchemaError, ValidationError, etc.) - config_file.go: Raw ConfigFile type mirroring YAML with pointer fields - parse.go: Parse(), ParseBytes(), ParseReader(), FromYAML() functions - validate.go: ValidateConfigFile() with functional options pattern - complete.go: CompleteConfig() for transforming ConfigFile → Config - schema.go: GenerateSchema() using github.com/invopop/jsonschema - build_options.go: BuildOptions struct for CLI flags (alternative to globals) - validator.go: Old schema validation (replaced by validate.go) - validator_test.go: Old tests (replaced by validate_test.go) - Load() returns LoadResult with Config, Warnings, and RootDir - Complete() replaces ValidateAndComplete() for programmatic configs - FromYAML() is now a test helper that doesn't run completion - Removed GetConfig(), GetRawConfig(), loadConfigFromFile() - Renamed JSON files for consistency (*_matrix.json → *_compatibility.json) - Renamed env_variables.go → env.go - Updated callers to use new APIs
Signed-off-by: Mark Phelps <[email protected]>
Bug fixes: - Fix major version check in validateConcurrency using splitPythonVersion - Use filepath.Join instead of path.Join for OS-specific path handling - Remove dead code (empty if !found block) in validateFrameworkCompatibility - Remove unused schemaVersion constant Dead code removal: - Remove unused Version, Image, Stats structs from version.go - Remove unused Example struct from config.go Unexport internal code: - ValidateCudaVersion → validateCudaVersion - CUDABaseImageFor → cudaBaseImageFor - ConfigFile/BuildFile/RunItemFile/etc → lowercase versions - EnvironmentVariableDenyList → environmentVariableDenyList
- Use filepath.Join instead of path.Join in load.go for OS-specific paths - Fix ValidateModelPythonVersion to check major version for concurrency - Remove unused getter methods from config_file.go (GetPythonVersion, GetPythonRequirements, GetCUDA, GetCuDNN, GetMax) - Standardize error messages to lowercase per Go conventions - Use %w for error wrapping in findConfigPathInDirectory
29481c8 to
60dcbf2
Compare
- Fix golangci-lint issues: rename shadowed 'max' variable, use type conversion - Remove duplicate ValidateModelPythonVersion function from config.go - Remove redundant validation call from build.go (Load already validates) - Update integration tests to match new ValidationError message format
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
tempusfrangit
previously approved these changes
Feb 9, 2026
Contributor
tempusfrangit
left a comment
There was a problem hiding this comment.
This seems pretty reasonable. Please make sure wip is removed from the PR title. I'd also like @michaeldwan 's review on this before we merge.
| if len(c.Build.PythonPackages) > 0 { | ||
| c.Build.pythonRequirementsContent = reqs | ||
| } else if len(c.Build.PythonPackages) > 0 { | ||
| // Backwards compatibility: if using deprecated python_packages, populate requirements content |
Contributor
There was a problem hiding this comment.
Maybe it's time to drop this and error. we're doing a TON of big restructuring and this has been deprecated for a while. We can do this as a separate PR before release. I expect 0.17.0 can include this PR and the future removal of deprecated things.
Contributor
There was a problem hiding this comment.
Love this sentiment. Let's delete more things!
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
tempusfrangit
approved these changes
Feb 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Config Refactor
1. Clear Three-Phase Architecture (Parse → Validate → Complete)
Before (main): Config loading was a monolithic process where parsing, validation, and CUDA resolution were interleaved and hard to follow.
After: Clean separation of concerns:
parse.go) - Only YAML parsing, no side effectsvalidate.go) - Semantic validation with typed errorsconfig.go) - CUDA resolution, requirements loadingThis makes the code easier to understand, test, and maintain.
2. Typed Error System
Before: Generic
errorreturns with string formatting, hard to programmatically handle different error types.After: Rich error types in
errors.go:ParseError- YAML parsing failuresSchemaError- Schema validation issuesValidationError- Semantic validation failuresDeprecationWarning- Deprecated field usageCompatibilityError- Version incompatibilitiesCallers can use
errors.As()to handle specific error types differently.3. Separation of Raw Config from Completed Config
Before: Single
Configstruct used for both raw YAML and completed config, with nullable fields mixed with populated fields.After:
configFile(unexported) - Raw YAML with pointer fields to distinguish "not set" from "zero value"Config- Completed config with resolved values4. Better Deprecation Handling
Before: Deprecation warnings were ad-hoc and mixed with errors.
After:
ValidationResultseparates errors from warnings:Can optionally treat deprecations as errors with
WithStrictDeprecations().5. Functional Options Pattern for Validation
Before: Hard-coded validation behavior.
After: Flexible configuration:
6. Bug Fixes
filepath.Joinvspath.Join)7. Reduced API Surface
configFile,buildFile, etc.)validateCudaVersion,cudaBaseImageFor)Example,Version,Statsstructs)8. Better Testability
WithRequirementsFS()allows testing without real filesystem9. Idiomatic Go
%wslicespackage with stdlib