Skip to content

refactor(dashboard-api): extract pure settings helpers into settings.py#1059

Open
boffin-dmytro wants to merge 3 commits intoLight-Heart-Labs:mainfrom
boffin-dmytro:refactor/settings-module
Open

refactor(dashboard-api): extract pure settings helpers into settings.py#1059
boffin-dmytro wants to merge 3 commits intoLight-Heart-Labs:mainfrom
boffin-dmytro:refactor/settings-module

Conversation

@boffin-dmytro
Copy link
Copy Markdown
Contributor

Summary

  • main.py was ~1478 lines (~3× the 500-line guideline). This PR extracts 9 env-related constants and 14 pure leaf functions into a new settings.py module, reducing main.py to ~1170 lines (~21% reduction).
  • Functions that call _resolve_install_root, _resolve_runtime_env_path, or _resolve_template_path remain in main.py so that monkeypatches in test_settings_env.py continue to intercept them correctly (Python resolves names at the module where they are defined, not the caller's module).
  • Zero behavioural change — only the file boundary moves.

What moved to settings.py

9 constants: _ENV_ASSIGNMENT_RE, _ENV_COMMENTED_ASSIGNMENT_RE, _SENSITIVE_ENV_KEY_RE, _SETTINGS_APPLY_ALLOWED_SERVICES, _LLAMA_APPLY_KEYS, _OPEN_WEBUI_APPLY_KEYS, _TOKEN_SPY_APPLY_KEYS, _PRIVACY_SHIELD_APPLY_KEYS, _MANUAL_RESTART_KEYS

14 functions: _strip_env_quotes, _parse_env_text, _read_env_map_from_path, _normalize_bool, _humanize_env_key, _is_secret_field, _slugify, _build_env_fields, _validate_env_values, _serialize_form_values, _match_apply_service, _build_apply_summary, _compute_env_apply_plan, _check_host_agent_available

Test plan

  • tests/test_settings_env.py — all 16 tests pass unchanged
  • python3 -c "import settings" — imports cleanly
  • No new tests added (pure refactor — no logic changed)

@Lightheartdevs
Copy link
Copy Markdown
Collaborator

Follow-up from today's merge pass: I updated this branch onto current main and reran the checks. Most checks went green, but Lint Python with Ruff still fails, so I'm leaving this unmerged.

Current Ruff failures are all fixable cleanup items:

  • main.py imports several helpers/constants from settings.py that are not used after the extraction (_SENSITIVE_ENV_KEY_RE, apply-key sets, _strip_env_quotes, _normalize_bool, _humanize_env_key, _is_secret_field, _match_apply_service, _build_apply_summary, etc.).
  • settings.py imports json but does not use it.

Once those imports are trimmed and Ruff is green, this should be ready for a normal refactor review again.

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified as a pure refactor: extracts _ENV_*_RE/_SETTINGS_*_KEYS/_MANUAL_RESTART_KEYS constants and ~14 helpers (_strip_env_quotes, _parse_env_text, _compute_env_apply_plan, etc.) from main.py into a new settings.py module. Import contract preserved. Net deletion in main.py matches additions in settings.py. Ship after rebase + API tests.

Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refactor is a reasonable cleanup, but it is not mergeable while the Python lint job is red.

I reproduced the CI failure locally with the same selector:

python -m ruff check dream-server/ --select E,F,W --ignore E501,E701,E731,E741,E402

Ruff reports 13 fixable F401 errors: unused settings imports in extensions/services/dashboard-api/main.py plus an unused json import in the new settings.py. Please prune those imports (or run Ruff with --fix and review the result), then rerun the API/lint checks. Since this is meant to be a behavior-preserving extraction, keeping CI green is the bar before merge.

@boffin-dmytro
Copy link
Copy Markdown
Contributor Author

Fixed — removed the 12 unused imports from main.py (helpers/constants now resolved within settings.py's own namespace, not needed in the caller) and the stray import json from settings.py. Ran ruff check dream-server/ --select E,F,W --ignore E501,E701,E731,E741,E402 locally — all checks pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants