-
Notifications
You must be signed in to change notification settings - Fork 120
Fixes #716 Improve environment-mapping behavior in optional config fields #734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #716 Improve environment-mapping behavior in optional config fields #734
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes issue #716 related to deployment failures when using folder_exclude_regex by improving environment-specific configuration handling. The changes introduce a clearer distinction between required and optional configuration fields, centralizing value extraction logic, and enhancing validation to provide appropriate logging (errors, warnings, or debug messages) based on field importance.
Changes:
- Introduced
get_config_valuehelper function to centralize environment-specific value extraction logic - Enhanced validation to distinguish required fields (causing errors) from optional fields (logging warnings/debug)
- Updated documentation to clearly explain environment-specific configuration behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/fabric_cicd/_common/_config_utils.py |
Added get_config_value helper and refactored extraction functions to use centralized logic |
src/fabric_cicd/_common/_config_validator.py |
Enhanced _validate_environment_exists to differentiate required/optional fields with appropriate logging |
docs/how_to/config_deployment.md |
Added comprehensive documentation on environment-specific configuration and field requirements |
tests/test_config_validator.py |
Updated tests to validate required/optional field behavior and logging |
tests/test_deploy_with_config.py |
Added comprehensive tests for get_config_value and environment-specific settings extraction |
|
Thank you @aviatco and @ayeshurun for reviewing. I've addressed the comments. Please review updated PR when you have a chance. Thanks |
Fixes #716
This pull request improves environment-specific configuration handling and validation for deployment configs. It clarifies documentation, centralizes logic for extracting environment-specific values, and enhances validation to distinguish between required and optional fields—logging errors, warnings, or debug messages as appropriate. Tests are updated to reflect these changes.
Environment-specific configuration and validation improvements:
docs/how_to/config_deployment.mdto clearly explain environment-specific configuration, distinguishing required and optional fields and their validation/logging behaviors.src/fabric_cicd/_common/_config_validator.pynow differentiates required fields (which cause errors if missing for an environment) from optional fields (which log warnings or debug messages), and includes new fields in the validation matrix. [1] [2] [3]Configuration extraction and code simplification:
get_config_valuehelper insrc/fabric_cicd/_common/_config_utils.pyto centralize logic for retrieving environment-specific or single values, reducing repetition and improving maintainability. This helper is now used in all relevant settings extraction functions. [1] [2] [3]Testing updates:
tests/test_config_validator.pyto cover new fields, verify required/optional logic, and ensure correct error/warning/debug behaviors for missing environment mappings. [1] [2] [3]