-
Notifications
You must be signed in to change notification settings - Fork 2
Refactor loading of configuration #23
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
Refactor loading of configuration #23
Conversation
Use the AWS CDK built in context loader[1] and allow storing and loading configurations from files. [1] https://docs.aws.amazon.com/cdk/v2/guide/context.html
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 PR refactors configuration loading to use AWS CDK's built-in context mechanism and file-based configuration. Configuration is moved from hardcoded environment variables in Python to YAML files, enabling better separation of concerns and leveraging CDK's standard context loading pattern.
Key Changes:
- Replaces inline environment variable dictionary with file-based configuration system
- Adds
load_context_config()utility function supporting YAML/JSON files with base config merging - Updates CDK context from environment variable (
ENV) to CDK context parameter (--context env=<env>)
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils.py | Implements new configuration loader with validation and file format support |
| tests/unit/test_utils.py | Comprehensive test suite for configuration loading functionality |
| config/*.yaml | Environment-specific configuration files for dev, stage, prod, and base |
| app.py | Refactored to use file-based config instead of inline dictionaries |
| tests/unit/test_network_stack.py | Updated to load config from files |
| tests/unit/test_service_stack.py | Updated to load config from files |
| README.md | Updated documentation for new configuration approach |
| .github/workflows/*.yaml | Updated workflows to use CDK context parameter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ConsoleCatzirl
left a comment
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.
This generally looks good, but there's one test that looks like a duplicate of another test (not a big deal).
Use the AWS CDK built in context loader[1] and allow storing and loading configurations from files.
[1] https://docs.aws.amazon.com/cdk/v2/guide/context.html
depends on #25