mirror of
https://github.com/basnijholt/compose-farm.git
synced 2026-02-03 14:13:26 +00:00
Drop CF_LOCAL_HOST; limit web-stack inference to containers (#163)
* config: Add local_host and web_stack options Allow configuring local_host and web_stack in compose-farm.yaml instead of requiring environment variables. This makes it easier to deploy the web UI with just a config file mount. - local_host: specifies which host is "local" for Glances connectivity - web_stack: identifies the web UI stack for self-update detection Environment variables (CF_LOCAL_HOST, CF_WEB_STACK) still work as fallback for backwards compatibility. Closes #152 * docs: Clarify glances_stack is used by CLI and web UI * config: Env vars override config, add docs - Change precedence: environment variables now override config values (follows 12-factor app pattern) - Document all CF_* environment variables in configuration.md - Update example-config.yaml to mention env var overrides * config: Consolidate env vars, prefer config options - Update docker-compose.yml to comment out CF_WEB_STACK and CF_LOCAL_HOST (now prefer setting in compose-farm.yaml) - Update init-env to comment out CF_LOCAL_HOST (can be set in config) - Update docker-deployment.md with new "Config option" column - Simplify troubleshooting to prefer config over env vars * config: Generate CF_LOCAL_HOST with config alternative note Instead of commenting out CF_LOCAL_HOST, generate it normally but add a note in the comment that it can also be set as 'local_host' in config. * config: Extend local_host to all web UI operations When running the web UI in a Docker container, is_local() can't detect which host the container is on due to different network namespaces. Previously local_host/CF_LOCAL_HOST only affected Glances connectivity. Now it also affects: - Container exec/shell (runs locally instead of via SSH) - File editing (uses local filesystem instead of SSH) Added is_local_host() helper that checks CF_LOCAL_HOST/config.local_host first, then falls back to is_local() detection. * refactor: DRY get_web_stack helper, add tests - Move get_web_stack to deps.py to avoid duplication in streaming.py and actions.py - Add tests for config.local_host and config.web_stack parsing - Add tests for is_local_host, get_web_stack, and get_local_host helpers - Tests verify env var precedence over config values * glances: rely on CF_WEB_STACK for container mode Restore docker-compose env defaults and document local_host scope. * web: ignore local_host outside container Document container-only behavior and adjust tests. * web: infer local host from web_stack Drop local_host config option and update docs/tests. * Remove CF_LOCAL_HOST override * refactor: move web_stack helpers to Config class - Add get_web_stack() and get_local_host_from_web_stack() as Config methods - Remove duplicate _get_local_host_from_web_stack() from glances.py and deps.py - Update deps.py get_web_stack() to delegate to Config method - Add comprehensive tests for the new Config methods * config: remove web_stack config option The web_stack config option was redundant since: - In Docker, CF_WEB_STACK env var is always set - Outside Docker, the container-specific behavior is disabled anyway Simplify by only using the CF_WEB_STACK environment variable. * refactor: remove get_web_stack wrapper from deps Callers now use config.get_web_stack() directly instead of going through a pointless wrapper function. * prompts: add rule to identify pointless wrapper functions
This commit is contained in:
@@ -6,6 +6,7 @@ Review the pull request for:
|
||||
- **Organization**: Is everything in the right place?
|
||||
- **Consistency**: Is it in the same style as other parts of the codebase?
|
||||
- **Simplicity**: Is it not over-engineered? Remember KISS and YAGNI. No dead code paths and NO defensive programming.
|
||||
- **No pointless wrappers**: Identify functions/methods that just call another function and return its result. Callers should call the underlying function directly instead of going through unnecessary indirection.
|
||||
- **User experience**: Does it provide a good user experience?
|
||||
- **PR**: Is the PR description and title clear and informative?
|
||||
- **Tests**: Are there tests, and do they cover the changes adequately? Are they testing something meaningful or are they just trivial?
|
||||
|
||||
Reference in New Issue
Block a user