Skip to content

Commit 6ac1f9e

Browse files
ambledclaude
andcommitted
Extract forced actions from executor.py, fix tests (v0.6.0)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3d9ac5e commit 6ac1f9e

9 files changed

Lines changed: 1119 additions & 1063 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## [Unreleased]
44

5+
## [0.6.0] - 2026-02-16
6+
7+
### Changed
8+
- Extracted forced actions from `executor.py` into `forced_actions.py` module
9+
- Fixed 4 broken tests
10+
511
## [0.5.8] - 2026-02-15
612

713
### Changed

docs/refactoring-plan.md

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
# Weave Node Manager — Refactoring Plan
2+
3+
## Project Stats (as of 2026-02-15)
4+
- 10,485 lines source (28 files), 6,681 lines tests (15 files)
5+
- Python 3.12.3+, SQLAlchemy, platforming Linux (systemd/setsid) and macOS (launchd)
6+
7+
## Priority 1: Extract forced actions from `executor.py` (1,657 lines)
8+
9+
**Problem**: `_force_*` methods (lines 748-1657, ~900 lines) share nearly identical structure with heavy duplication.
10+
11+
**Shared pattern across `_force_remove_node`, `_force_upgrade_node`, `_force_stop_node`, `_force_start_node`**:
12+
1. Parse service names + validate via `_parse_node_name` / `_get_node_by_name`
13+
2. If service_names provided: loop with delay, process each node
14+
3. Else: count validation + batch processing by age (youngest/oldest)
15+
4. Result aggregation (counts of added/removed/upgraded/stopped)
16+
5. Identical delay logic between operations
17+
18+
**Approach**: Extract to `forced_actions.py` module. Consider a generic `_force_batch_operation()` that accepts: node query function, operation function, delay, and result key. The 4 methods collapse into configuration of this generic.
19+
20+
**Also**: `_execute_add_node` (149 lines) has node construction/ID allocation logic that could become a factory.
21+
22+
---
23+
24+
## Priority 2: Defer `config.py` side effects (1,340 lines)
25+
26+
**Problem**: Lines 1117-1341 execute at import time: database creation, migrations, machine init, filesystem setup. This couples everything to import order and makes testing harder.
27+
28+
**Approach**: Move module-level execution into an explicit `initialize()` function called from `__main__.py`.
29+
30+
---
31+
32+
## Priority 3: Data-driven `merge_config_changes()` (216 lines)
33+
34+
**Problem**: 40+ repetitive if-blocks, each following a nearly identical pattern.
35+
36+
### Field Categories
37+
38+
| Category | Cast/Transform | Truthiness Check | Fields |
39+
|----------|---------------|-----------------|--------|
40+
| **int** | `int()` | truthy | `node_cap`, `cpu_less_than`, `cpu_remove`, `mem_less_than`, `mem_remove`, `hd_less_than`, `hd_remove`, `delay_start`, `delay_restart`, `delay_upgrade`, `delay_remove`, `survey_delay`, `max_concurrent_upgrades`, `max_concurrent_starts`, `max_concurrent_removals`, `max_concurrent_operations`, `hdio_read_less_than`, `hdio_read_remove`, `hdio_write_less_than`, `hdio_write_remove`, `netio_read_less_than`, `netio_read_remove`, `netio_write_less_than`, `netio_write_remove`, `crisis_bytes`, `highest_node_id_used` |
41+
| **int_nullable** | `int()` | `is not None` | `action_delay` (0 is valid) |
42+
| **float** | `float()` | truthy | `max_load_average_allowed`, `desired_load_average` |
43+
| **str** | none | truthy | `node_storage`, `donate_address`, `environment`, `start_args`, `process_manager` |
44+
| **str_path** | none | truthy | `antnode_path`, `antctl_path`, `antctl_version` |
45+
| **port** | `normalize_port_start()` | truthy | `port_start`, `metrics_port_start`, `rpc_port_start` |
46+
47+
### Special cases (keep as explicit code)
48+
49+
1. **`rewards_address`** — calls `validate_rewards_address()`, exits on failure
50+
2. **`no_upnp`, `antctl_debug`**`bool` fields that check `sys.argv` directly because argparse `store_true` default of `False` is indistinguishable from "not provided"
51+
3. **`disable_config` block** (lines 806-820) — inverts boolean flags when `--force_action disable_config` is used; entirely separate logic
52+
53+
### Proposed implementation
54+
55+
```python
56+
# Field descriptors: (field_name, cast_type)
57+
# cast_type: "int", "int_nullable", "float", "str", "port"
58+
_MERGE_FIELDS = [
59+
("node_cap", "int"),
60+
("cpu_less_than", "int"),
61+
("cpu_remove", "int"),
62+
# ... all fields from table above ...
63+
("rpc_port_start", "port"),
64+
]
65+
66+
def merge_config_changes(options, machine_config):
67+
cfg = {}
68+
69+
# Data-driven: handle all standard fields
70+
for field, cast_type in _MERGE_FIELDS:
71+
opt_val = getattr(options, field, None)
72+
db_val = getattr(machine_config, field)
73+
74+
if cast_type == "int_nullable":
75+
if opt_val is None:
76+
continue
77+
opt_val = int(opt_val)
78+
elif cast_type == "int":
79+
if not opt_val:
80+
continue
81+
opt_val = int(opt_val)
82+
elif cast_type == "float":
83+
if not opt_val:
84+
continue
85+
opt_val = float(opt_val)
86+
elif cast_type == "port":
87+
if not opt_val:
88+
continue
89+
opt_val = normalize_port_start(opt_val)
90+
else: # str, str_path
91+
if not opt_val:
92+
continue
93+
94+
if opt_val != db_val:
95+
cfg[field] = opt_val
96+
97+
# Special: rewards_address (validation + exit on failure)
98+
if options.rewards_address and options.rewards_address != machine_config.rewards_address:
99+
is_valid, error_msg = validate_rewards_address(
100+
options.rewards_address, machine_config.donate_address
101+
)
102+
if not is_valid:
103+
logging.error(f"Invalid rewards_address: {error_msg}")
104+
sys.exit(1)
105+
cfg["rewards_address"] = options.rewards_address
106+
107+
# Special: bool flags checked via sys.argv (store_true default is indistinguishable)
108+
if "--no_upnp" in sys.argv or "--no-upnp" in sys.argv or os.getenv("NO_UPNP"):
109+
if bool(options.no_upnp) != bool(machine_config.no_upnp):
110+
cfg["no_upnp"] = bool(options.no_upnp)
111+
112+
if "--antctl_debug" in sys.argv or "--antctl-debug" in sys.argv or os.getenv("ANTCTL_DEBUG"):
113+
if bool(options.antctl_debug) != bool(machine_config.antctl_debug):
114+
cfg["antctl_debug"] = bool(options.antctl_debug)
115+
116+
# Special: disable_config inversion
117+
if hasattr(options, "force_action") and options.force_action == "disable_config":
118+
if "--antctl_debug" in sys.argv or "--antctl-debug" in sys.argv:
119+
if machine_config.antctl_debug != False:
120+
cfg["antctl_debug"] = False
121+
logging.info("disable_config: Setting antctl_debug to False")
122+
if "--no_upnp" in sys.argv or "--no-upnp" in sys.argv:
123+
if machine_config.no_upnp != False:
124+
cfg["no_upnp"] = False
125+
logging.info("disable_config: Setting no_upnp to False (enabling UPnP)")
126+
127+
return cfg
128+
```
129+
130+
**Result**: ~216 lines → ~60 lines. The 3 special cases remain explicit and readable. Standard fields are fully described by the `_MERGE_FIELDS` list — adding a new int config field is one line.
131+
132+
---
133+
134+
## Priority 4: Split `main()` into phases (284 lines)
135+
136+
**Problem**: `main()` in `__main__.py` handles version checks, lock files, migration, node discovery/import, port configuration, metrics, reporting, and forced actions.
137+
138+
**Approach**: Extract into named functions for each phase:
139+
- `check_prerequisites()` — version, lock file
140+
- `handle_migration()` — DB migration logic
141+
- `handle_initialization()` — node discovery, import, port config
142+
- `run_decision_cycle()` — metrics, decision engine, execution
143+
- `generate_reports()` — reporting phase
144+
145+
---
146+
147+
## Files reference (largest first)
148+
- `executor.py` — 1,657 lines (Priority 1)
149+
- `config.py` — 1,340 lines (Priority 2, 3)
150+
- `models.py` — 658 lines (fine)
151+
- `process_managers/antctl_zen_manager.py` — 676 lines (fine, platform-specific)
152+
- `process_managers/launchd_manager.py` — 649 lines (fine)
153+
- `process_managers/antctl_manager.py` — 602 lines (fine)
154+
- `process_managers/systemd_manager.py` — 593 lines (fine)
155+
- `utils.py` — 542 lines (fine)
156+
- `reports.py` — 521 lines (fine)
157+
- `decision_engine.py` — 518 lines (fine, well-organized)
158+
- `__main__.py` — 436 lines (Priority 4)

src/wnm/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
"""A service to manage a cluster of decentralized Autonomi nodes"""
22

3-
__version__ = "0.5.8"
3+
__version__ = "0.6.0"

0 commit comments

Comments
 (0)