Skip to content

Commit e47cc8d

Browse files
committed
refactor: remove PlainTextBackend and enhance testing infrastructure
- Remove PlainTextBackend and its tests - Add Python 3.12 support - Set up SonarCloud integration - Update test configuration - Centralize coverage config in pyproject.toml - Simplify tox configuration - Add quality gates - Add Architecture Decision Records (ADRs) - Update design documentation - Add development tasks tracking - Update .gitignore for IDE settings
1 parent a31c758 commit e47cc8d

File tree

17 files changed

+1345
-487
lines changed

17 files changed

+1345
-487
lines changed

.github/workflows/test.yml

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ jobs:
1212
strategy:
1313
fail-fast: false
1414
matrix:
15-
python-version: ['3.9', '3.10', '3.11']
15+
python-version: ['3.9', '3.10', '3.11', '3.12']
1616

1717
steps:
1818
- uses: actions/checkout@v3
19+
with:
20+
fetch-depth: 0 # Shallow clones should be disabled for better relevancy of analysis
1921

2022
- name: Set up Python ${{ matrix.python-version }}
2123
uses: actions/setup-python@v4
@@ -25,7 +27,19 @@ jobs:
2527
- name: Install dependencies
2628
run: |
2729
python -m pip install --upgrade pip
28-
pip install tox tox-gh-actions
30+
pip install tox tox-gh-actions pylint
31+
32+
- name: Run pylint
33+
run: |
34+
pylint helm_values_manager --output-format=text:pylint-report.txt,colorized
35+
continue-on-error: true # Don't fail if pylint has warnings
2936

3037
- name: Test with tox
3138
run: tox
39+
40+
- name: SonarCloud Scan
41+
if: matrix.python-version == '3.12' # Only run once, on the latest Python version
42+
uses: SonarSource/sonarcloud-github-action@master
43+
env:
44+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
45+
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

.gitignore

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,12 @@ cython_debug/
167167
# option (not recommended) you can uncomment the following to ignore the entire idea folder.
168168
#.idea/
169169

170+
# IDE settings
171+
.vscode/
172+
.idea/
173+
*.sublime-workspace
174+
*.sublime-project
175+
170176
# PyPI configuration file
171177
.pypirc
172178

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# ADR 002: Config Path and Storage Design
2+
3+
## Status
4+
Accepted
5+
6+
## Context
7+
When implementing the Helm Values Manager, we identified several key design considerations:
8+
9+
1. Users need to define configuration structure (paths, descriptions, requirements) independently of setting values
10+
2. As a CLI plugin, each command is executed independently, requiring consistent file operations
11+
3. The initial design included a PlainText backend which added unnecessary complexity
12+
4. Path lookups and validations needed optimization for better performance
13+
14+
## Decision
15+
16+
### 1. Separate Config Definition from Values
17+
- Allow users to define configuration paths and metadata without setting values
18+
- Support metadata like description, required flag, and sensitivity flag
19+
- Enable validation of structure before any values are set
20+
21+
### 2. Standardized Command Pattern
22+
- Implement a BaseCommand class for consistent file operations
23+
- Include file locking to prevent concurrent modifications
24+
- Add automatic backup before writes
25+
- Handle common error scenarios uniformly
26+
27+
```python
28+
class BaseCommand:
29+
def execute(self):
30+
try:
31+
config = self.load_config()
32+
result = self.run(config)
33+
self.save_config(config)
34+
return result
35+
except Exception as e:
36+
# Handle errors, cleanup
37+
raise
38+
```
39+
40+
### 3. Simplified Storage Design
41+
- Remove PlainTextBackend
42+
- Store local values directly in HelmValuesConfig
43+
- Use specialized backends (AWS, Azure) only for remote storage
44+
- Maintain clear separation between local and remote storage
45+
46+
### 4. Optimized Path Management
47+
- Implement a path map for O(1) lookups
48+
- Validate path uniqueness during definition
49+
- Store local values with efficient key structure
50+
- Separate path definition from value storage
51+
52+
```python
53+
class HelmValuesConfig:
54+
def __init__(self):
55+
self._path_map = {} # Fast path lookups
56+
self._local_values = {} # Local value storage
57+
```
58+
59+
## Consequences
60+
61+
### Positive
62+
1. **Better User Experience**
63+
- Clear separation between structure definition and value setting
64+
- Improved validation capabilities
65+
- More intuitive configuration management
66+
67+
2. **Improved Performance**
68+
- O(1) path lookups
69+
- Reduced complexity in local storage
70+
- More efficient validation
71+
72+
3. **Enhanced Maintainability**
73+
- Consistent command pattern
74+
- Clear separation of concerns
75+
- Simplified storage architecture
76+
77+
4. **Better Safety**
78+
- Built-in file locking
79+
- Automatic backups
80+
- Strict path validation
81+
82+
### Negative
83+
1. **Additional Complexity**
84+
- New base command pattern to learn
85+
- More sophisticated path management
86+
87+
## Implementation Notes
88+
89+
1. **Command Implementation**
90+
```python
91+
class SetValueCommand(BaseCommand):
92+
def run(self, config):
93+
config.set_value(self.path, self.env, self.value)
94+
```
95+
96+
2. **Path Definition**
97+
```python
98+
config.add_config_path(
99+
path="app.replicas",
100+
description="Number of application replicas",
101+
required=True
102+
)
103+
```
104+
105+
3. **Value Storage**
106+
```python
107+
# Local storage
108+
config.set_value("app.replicas", "dev", "3")
109+
110+
# Remote storage (AWS/Azure)
111+
config.set_value("app.secrets.key", "prod", "secret-value")
112+
```
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# ADR 003: Unified Path and Value Storage
2+
3+
## Status
4+
Accepted
5+
6+
## Context
7+
In our previous design (ADR 002), we had two separate dictionaries in `HelmValuesConfig`:
8+
1. `_path_map` for storing path metadata and validation
9+
2. `_local_values` for storing local values
10+
11+
This separation added unnecessary complexity and potential for inconsistency.
12+
13+
## Decision
14+
15+
### 1. Unified Storage Structure
16+
We will use a single dictionary with a nested structure:
17+
18+
```python
19+
_path_map = {
20+
"app.replicas": { # path
21+
"metadata": {
22+
"description": "Number of replicas",
23+
"required": True,
24+
"sensitive": False
25+
},
26+
"values": {
27+
"dev": "3", # local value
28+
"prod": ValueBackend # remote value
29+
}
30+
}
31+
}
32+
```
33+
34+
### 2. Value Resolution Strategy
35+
- Local values are stored directly in the values dict
36+
- Remote values are represented by their respective backend instances
37+
- The get_value method will automatically resolve the appropriate storage
38+
39+
```python
40+
def get_value(self, path: str, environment: str) -> str:
41+
if path not in self._path_map:
42+
raise PathNotFoundError(f"Path not defined: {path}")
43+
44+
env_values = self._path_map[path]["values"]
45+
if environment not in env_values:
46+
raise ValueError(f"No value set for {path} in {environment}")
47+
48+
value = env_values[environment]
49+
if isinstance(value, ValueBackend):
50+
return value.get_value(self._generate_key(path, environment))
51+
return value
52+
```
53+
54+
## Consequences
55+
56+
### Positive
57+
1. **Simplified Data Structure**
58+
- Single source of truth for paths and values
59+
- Clearer relationship between paths and their values
60+
- More intuitive code organization
61+
62+
2. **Better Performance**
63+
- Single lookup for both metadata and values
64+
- Reduced memory usage
65+
- Simpler caching if needed
66+
67+
3. **Improved Maintainability**
68+
- Less code to maintain
69+
- Fewer potential points of failure
70+
- Easier to understand and debug
71+
72+
### Negative
73+
1. **More Complex Value Resolution**
74+
- Need to check value type before resolving
75+
- Slightly more complex serialization/deserialization
76+
77+
## Implementation Notes
78+
79+
1. **Path Definition**
80+
```python
81+
config.add_config_path(
82+
path="app.replicas",
83+
description="Number of replicas",
84+
required=True
85+
)
86+
```
87+
88+
2. **Value Setting**
89+
```python
90+
# Setting local value
91+
config.set_value("app.replicas", "dev", "3")
92+
93+
# Setting remote value (automatically creates backend)
94+
config.set_value("app.secrets.key", "prod", "secret-value")
95+
```
96+
97+
3. **Serialization**
98+
```python
99+
def to_dict(self) -> dict:
100+
result = {}
101+
for path, data in self._path_map.items():
102+
result[path] = {
103+
"metadata": data["metadata"],
104+
"values": {
105+
env: value if not isinstance(value, ValueBackend) else None
106+
for env, value in data["values"].items()
107+
}
108+
}
109+
return result
110+
```

0 commit comments

Comments
 (0)