feat: port web-based configuration editor from main to develop#319
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds an opt-in embedded web server and browser-based configuration editor (enabled via Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Web as WebServer
participant Backend as web_config
participant File as Config File
participant Proc as Application Process
Browser->>Web: GET /config (HTML)
Browser->>Web: GET /api/config
Web->>Backend: config_to_json(config_path)
Backend->>File: read INI
File-->>Backend: INI contents
Backend-->>Web: JSON payload
Web-->>Browser: 200 OK (config JSON)
Browser->>Web: POST /api/config (sections, order)
Web->>Backend: write_config_from_dict(config_path, sections, order)
Backend->>File: atomic write (with fallback)
File-->>Backend: success
Backend-->>Web: 200 OK
Browser->>Web: POST /api/restart
Web-->>Browser: 200 OK
Web->>Proc: schedule delayed SIGUSR1 (threading.Timer -> os.kill)
Note right of Proc: Supervisor expected to restart process
loop Poll health until ready
Browser->>Web: GET /health
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/astrameter/health_service.py (1)
50-58: Consider security implications: endpoints expose config (including secrets) without authentication.When
WEB_CONFIG_ENABLED=True, anyone with network access to port 52500 can:
- Read all config values including
PASSWORD,ACCESSTOKEN, and other secrets viaGET /api/config- Modify the configuration via
POST /api/config- Trigger a service restart via
POST /api/restartSince this is opt-in and likely intended for trusted local networks, this may be acceptable. Consider adding a note in the documentation warning users about the security implications, or binding to
127.0.0.1by default when web config is enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/astrameter/health_service.py` around lines 50 - 58, The web config routes added when enable_web_config is true expose secrets; update the health_service to restrict access by either binding the web UI to localhost by default and/or requiring a configurable token; specifically, when registering routes (where enable_web_config is checked and app.router.add_* calls occur) ensure the server listen/bind defaults to "127.0.0.1" unless an explicit host override is provided, or add a short auth check inside the handlers (_handle_api_config_get, _handle_api_config_post, _handle_api_restart, _handle_config_ui) that validates a secret token from an env var (e.g., WEB_CONFIG_TOKEN) before returning config or performing changes, returning 401 on failure; update documentation to note the security implications and how to override the default host or set the token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/health_service.py`:
- Around line 166-175: The handler _handle_api_restart currently uses
threading.Timer(..., lambda: os._exit(0)) which forcefully terminates the
process and bypasses cleanup; replace this with a graceful shutdown trigger:
instead of os._exit, schedule a shutdown/signaling mechanism (e.g., call a
shared shutdown coroutine or set an asyncio.Event) that the main loop listens
for and then runs graceful cleanup of MQTT, powermeters and other services
before exiting; update _handle_api_restart to log the restart request,
set/dispatch the shutdown signal (or post a SIGTERM to the process) via the
existing async shutdown path, and remove the direct os._exit call so all finally
blocks and cleanup handlers execute.
In `@src/astrameter/web_config.py`:
- Around line 564-586: In collectConfig(), currently any .section-card whose
.section-name-input is empty is silently skipped; change this so you validate
section names before building the payload: iterate over all .section-card
elements (same loop in collectConfig), collect any cards with empty trimmed
section-name-input values (and optionally duplicate names using seenSections),
and if any are found return an error or throw an exception (or call a UI
alert/confirm) to block the save and surface which sections are unnamed; ensure
you still reference the same identifiers (collectConfig, .section-card,
.section-name-input, seenSections, order) so the check runs before result/ order
are returned.
---
Nitpick comments:
In `@src/astrameter/health_service.py`:
- Around line 50-58: The web config routes added when enable_web_config is true
expose secrets; update the health_service to restrict access by either binding
the web UI to localhost by default and/or requiring a configurable token;
specifically, when registering routes (where enable_web_config is checked and
app.router.add_* calls occur) ensure the server listen/bind defaults to
"127.0.0.1" unless an explicit host override is provided, or add a short auth
check inside the handlers (_handle_api_config_get, _handle_api_config_post,
_handle_api_restart, _handle_config_ui) that validates a secret token from an
env var (e.g., WEB_CONFIG_TOKEN) before returning config or performing changes,
returning 401 on failure; update documentation to note the security implications
and how to override the default host or set the token.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18818849-ab25-4a85-87e4-8566a97baeca
📒 Files selected for processing (5)
CHANGELOG.mdconfig.ini.examplesrc/astrameter/health_service.pysrc/astrameter/main.pysrc/astrameter/web_config.py
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/health_service.py`:
- Around line 35-38: HealthCheckService currently defaults bind_address to
"0.0.0.0" and when enable_web_config is true it exposes unauthenticated config
editor endpoints (including /api/restart) on that listener; change the logic
that attaches the config editor routes (the code path gated by enable_web_config
in HealthCheckService) so that routes are only mounted when either (a) the
bind_address is loopback (127.0.0.1 or ::1) or (b) an explicit auth/CSRF
protection mechanism is provided; enforce this by checking the bind_address
and/or a new required auth_config parameter before calling the function that
registers the config editor endpoints (referenced as the enable_web_config
gating and the /api/restart handler), and if neither condition is met, refuse to
attach the routes and log/raise an error instructing the operator to use
loopback or provide auth.
- Around line 139-145: The handlers that catch exceptions and currently return
raw str(e) in the response should be changed to log the full exception
server-side (use logger.exception(...) or logger.error with traceback) and avoid
leaking exception text to clients; map client-side errors (e.g., ValueError,
json.JSONDecodeError) to a 400 response with a generic message like
{"error":"Invalid request"} and map all other unexpected exceptions to a 500
response with {"error":"Internal server error"} instead of str(e); update both
exception blocks (the one shown and the similar one around lines 167-173) to
implement this pattern so they log full details but return safe, generic JSON
error bodies and appropriate status codes.
In `@src/astrameter/web_config.py`:
- Around line 583-588: The code that builds pairs by iterating
card.querySelectorAll('.key-value-body tr') silently overwrites duplicate keys
via pairs[key] = val; update the logic in that block to detect duplicates
instead: when extracting key from tr.querySelector('.key-input').value.trim(),
check if key already exists in the pairs map and if so mark the card/row as
invalid and surface a validation error (same UX as duplicate section-name
checks) and prevent form submission; otherwise set pairs[key] = val. Ensure you
reference the key-input/val-input rows and reuse the existing duplicate-section
validation mechanism to display the error and block the request.
- Around line 784-813: The temp file is left behind when
shutil.copyfile(tmp_path, config_path) succeeds because transferred is set True
and the finally cleanup is skipped; after a successful copyfile you should
remove tmp_path to avoid leaking temps. Modify the block that calls
shutil.copyfile in the try (around shutil.copyfile(tmp_path, config_path)) so
that on success you call contextlib.suppress(OSError): os.unlink(tmp_path) (or
equivalent safe unlink) and then set transferred = True; keep the existing
retryable-exception handling for the copyfile failure unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67002d66-dd87-4520-881c-9a1475595a81
📒 Files selected for processing (2)
src/astrameter/health_service.pysrc/astrameter/web_config.py
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/health_service.py`:
- Around line 160-164: The handler currently assumes request.json() returns a
dict and that "sections" is a mapping and "order" is a list; validate the parsed
payload before using .get() by checking that data is a dict, sections is a dict
(or mapping) and order is a list (or sequence) and if any check fails return a
400 error instead of proceeding to call write_config_from_dict(self.config_path,
sections, order); use isinstance checks on the result of await request.json(),
on data.get("sections") and data.get("order") (or coerce/normalize them) to
avoid AttributeError/list-item issues and locate the logic around
request.json(), sections, order, and write_config_from_dict to implement this
validation and error response.
In `@src/astrameter/web_config.py`:
- Around line 531-565: When a section is renamed, update the client-side state
instead of leaving the old name: in the rename handler (the event attached to
the '.section-name-input' inside renderSection), read oldName from the card's
dataset and newName from the input, validate newName (non-empty and not already
in sectionOrder), then set currentConfig[newName] = currentConfig[oldName] (or
{} if missing), delete currentConfig[oldName], replace the entry in sectionOrder
(find index of oldName and set it to newName), and update the DOM card by
setting card.dataset.section = newName (and any visible labels) so
removeSection() and addSection() see the updated name; ensure addSection() still
checks sectionOrder for duplicates so new names won't collide.
- Around line 824-935: Reject unsafe names/values before emitting the INI: in
write_config_from_dict validate every section name, key, and value (iterate over
write_order and sections) and raise ValueError if a section contains '\n', '\r',
or ']' or if a key contains '\n' or '\r' (and optionally '=') or if a value
contains '\n' or '\r'; perform this single validation at the top of
write_config_from_dict so both the "file missing" branch and the
_update_section/new-section emission paths are protected (refer to symbols
write_config_from_dict, write_order, sections, keys/values, and
_update_section).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8b9321d9-5763-4d32-a5ae-ce6b1a6f1f74
📒 Files selected for processing (2)
src/astrameter/health_service.pysrc/astrameter/web_config.py
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/health_service.py`:
- Around line 202-205: The restart timer currently sends SIGTERM via
os.kill(os.getpid(), signal.SIGTERM) which doesn't trigger the in-process
restart path in main.py (which sets restart_requested=True only on SIGUSR1);
change the signal sent by the timer to signal.SIGUSR1 so the lambda called by
threading.Timer triggers the same SIGUSR1 handler used in src/astrameter/main.py
and allows the process to perform the in-process "Save & Restart" flow instead
of exiting for an external supervisor to restart it.
In `@src/astrameter/main.py`:
- Around line 578-589: The loop reloads cfg on restart but fails to recompute
config-derived runtime state, so device_types, device_ids and skip_test remain
stale; after reloading cfg (inside the KeyboardInterrupt restart branch and
anywhere restart_requested can trigger a reload) rebuild device_types,
device_ids and skip_test using the same logic you used before entering the loop
(the code that originally derives GENERAL.DEVICE_TYPE, DEVICE_IDS, and
SKIP_POWERMETER_TEST) so the subsequent call to asyncio.run(async_main(...))
sees the updated values and CT port / shellypro3em expansion checks reflect the
new config.
In `@src/astrameter/web_config.py`:
- Around line 162-175: The read-modify-write of the config must be protected by
_CONFIG_WRITE_LOCK for the entire transaction: move acquisition of
_CONFIG_WRITE_LOCK so it is obtained before checking os.path.exists(config_path)
and before reading any existing file contents (and keep it held until after
_atomic_write_lines(config_path, lines) completes); ensure functions like
_validate_config_payload, the computation of write_order, and building of lines
based on sections/order all occur while the lock is held (apply the same locking
change to the later block covering lines ~178-260) so concurrent requests cannot
both read the old file and clobber each other.
- Around line 129-133: In _validate_config_payload, add a check that the order
list contains no duplicate section names and raise ValueError if duplicates
exist; specifically, inside the _validate_config_payload(sections: dict, order:
list) function validate that len(order) == len(set(order)) (or use
collections.Counter to detect repeats) and raise a clear ValueError like
"'order' contains duplicate section names" so write_order cannot emit the same
section twice; keep the existing type checks and perform this duplicate check
after validating the type of order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aea2decd-8ca9-4b5f-808e-31345bd64373
📒 Files selected for processing (4)
src/astrameter/health_service.pysrc/astrameter/main.pysrc/astrameter/static/config_editor.htmlsrc/astrameter/web_config.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/astrameter/web_server.py (1)
161-186:⚠️ Potential issue | 🟠 MajorValidate the updated config before returning success.
Line 181 writes the live file, and Line 183 immediately returns
{"success": true}.write_config_from_dict()only guarantees INI structure; invalid values can still be persisted and then break the next restart.src/astrameter/web_config.pyalready addsvalidate_config(), so this handler should validate before committing or restore the previous file on failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/astrameter/web_server.py` around lines 161 - 186, The handler _handle_api_config_post currently calls write_config_from_dict(...) and immediately returns success; instead, after preparing sections and order call validate_config (from src/astrameter/web_config.py) to ensure the new config is valid before committing, or write to a temporary file and run validate_config against that temp file, and only atomically replace the live config_path if validation passes; if validation fails, restore the original file (or delete the temp) and return an error response with validation details. Ensure you reference and use write_config_from_dict, validate_config, and self.config_path in the flow so invalid values are never persisted as the live config.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/astrameter/main.py`:
- Around line 584-595: The restart path only reloads cfg and device lists but
skips the one-time pre-run setup (CLI THROTTLE_INTERVAL override and MARSTEK
auto-registration) done before the loop; move that full startup preparation into
the restart cycle by extracting the pre-run setup into a helper (e.g.,
prepare_startup(cfg, args) or similar) that applies the THROTTLE_INTERVAL
override and performs the MARSTEK auto-registration, then call that helper both
before the first asyncio.run(async_main(...)) and again after reloading cfg in
the except KeyboardInterrupt branch (before calling asyncio.run), ensuring
restart_requested is handled as before.
In `@src/astrameter/web_config.py`:
- Around line 264-293: The CT002 schema fields marked as "float" (e.g.,
DEDUPE_TIME_WINDOW, CONSUMER_TTL, MAX_SMOOTH_STEP, MIN_EFFICIENT_POWER,
EFFICIENCY_ROTATION_INTERVAL) are read with cfg.getint(...) in runtime (see
src/astrameter/main.py), causing mismatches; either change those keys in the
CT002 block to "type": "integer" to match getint(), or update the runtime calls
that read them (the cfg.getint(...) usages) to cfg.getfloat(...) so the parser
accepts decimals—pick one approach and apply it consistently for all listed keys
so UI metadata and runtime parsing align.
---
Duplicate comments:
In `@src/astrameter/web_server.py`:
- Around line 161-186: The handler _handle_api_config_post currently calls
write_config_from_dict(...) and immediately returns success; instead, after
preparing sections and order call validate_config (from
src/astrameter/web_config.py) to ensure the new config is valid before
committing, or write to a temporary file and run validate_config against that
temp file, and only atomically replace the live config_path if validation
passes; if validation fails, restore the original file (or delete the temp) and
return an error response with validation details. Ensure you reference and use
write_config_from_dict, validate_config, and self.config_path in the flow so
invalid values are never persisted as the live config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12f6dab9-b09d-4ea8-a4db-6f93b0275d6c
📒 Files selected for processing (6)
config.ini.exampleha_addon/run.shsrc/astrameter/main.pysrc/astrameter/static/config_editor.htmlsrc/astrameter/web_config.pysrc/astrameter/web_server.py
✅ Files skipped from review due to trivial changes (2)
- config.ini.example
- src/astrameter/static/config_editor.html
Comment on lines
+584
to
+595
| while True: | ||
| restart_requested = False | ||
| try: | ||
| asyncio.run(async_main(cfg, args, device_types, device_ids, skip_test)) | ||
| break # clean exit | ||
| except KeyboardInterrupt: | ||
| if not restart_requested: | ||
| break | ||
| logger.info("Restarting service…") | ||
| cfg = configparser.ConfigParser(dict_type=OrderedDict, interpolation=None) | ||
| cfg.read(args.config) | ||
| device_types, device_ids, skip_test = _resolve_device_config(cfg, args) |
There was a problem hiding this comment.
Move the full startup preparation into the restart cycle.
This branch only reloads cfg and recomputes device_types / device_ids / skip_test. The setup above the loop is still one-shot, so a web-triggered restart skips the CLI THROTTLE_INTERVAL override (Lines 509-513) and the MARSTEK auto-registration path (Lines 515-566). That makes “Save & Restart” behave differently from a real restart.
Please rerun that preparation after reloading the config, or wrap the whole pre-run setup in a helper used on every loop iteration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/astrameter/main.py` around lines 584 - 595, The restart path only reloads
cfg and device lists but skips the one-time pre-run setup (CLI THROTTLE_INTERVAL
override and MARSTEK auto-registration) done before the loop; move that full
startup preparation into the restart cycle by extracting the pre-run setup into
a helper (e.g., prepare_startup(cfg, args) or similar) that applies the
THROTTLE_INTERVAL override and performs the MARSTEK auto-registration, then call
that helper both before the first asyncio.run(async_main(...)) and again after
reloading cfg in the except KeyboardInterrupt branch (before calling
asyncio.run), ensuring restart_requested is handled as before.
Adapts the web_config feature from the main branch to work with the develop branch's async aiohttp architecture and src/astrameter layout. - Add src/astrameter/web_config.py with HTML editor and config read/write helpers - Extend HealthCheckService with /config, /api/config (GET/POST), /api/restart - Wire WEB_CONFIG_ENABLED setting in main.py (opt-in, default False) - Document WEB_CONFIG_ENABLED in config.ini.example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e threshold Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace os._exit(0) with SIGTERM in restart handler so asyncio cleanup (powermeters, MQTT insights, finally blocks) runs before process exits - Raise an error in collectConfig() when any section has an empty name, preventing silent data loss on save Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On overlayfs / bind-mount filesystems (e.g. Home Assistant add-ons), rename(2) returns EBUSY when the destination is a bind-mounted file. Handle errno.EBUSY by copying the temp file over the destination in-place instead of renaming it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Some container setups return EPERM instead of EBUSY from rename(2) when the destination is a bind-mounted file. Extend the fallback to handle both errno values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On some overlayfs setups the bind-mounted destination file cannot be opened for writing (EPERM) even though the directory is writable. Add a third strategy: unlink the destination first (which creates an overlayfs whiteout in the upper layer), then rename the temp file into place. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…table When all write strategies fail (rename, copyfile, unlink+rename), surface a human-readable error explaining the cause and how to fix it, instead of a raw [Errno 13] string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'Permission denied' is EACCES (13); EPERM is 'Operation not permitted' (1). All fallback checks were testing the wrong errno constant, so every permission error was being re-raised before the next strategy could run. Consolidate retryable errno values into a single tuple and check it consistently across all three strategies. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…trategies Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- In _atomic_write_lines, unlink the temp file after a successful shutil.copyfile so it is not left behind when rename is unavailable. - In collectConfig (JS), throw on duplicate keys within a section using the same error-surfacing path as duplicate section-name detection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…responses - Emit a WARNING log when WEB_CONFIG_ENABLED=true so the operator is reminded that unauthenticated read/write access is active - Use logger.exception() for unexpected errors and return generic 500 bodies instead of leaking internal error details to the client - Distinguish 400 (bad request) from 500 (server error) in the save handler - Add SVG favicon to the config editor page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Validate JSON payload shape (dict/list/dict) before calling .get() in the POST /api/config handler so malformed bodies return 400 not 500 - Add _validate_config_payload() to web_config.py that rejects section names, keys, or values containing \r, \n, or ] (section names), which could corrupt the emitted INI and have the API report success - Fix stale sectionOrder state in the JS editor: addSection() now reads live names from the DOM via getCurrentSectionNames() to detect duplicates after a rename; removeSection() uses the current input value instead of card.dataset.section for the confirm prompt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_write_lines Strategy 1 (shutil.copyfile) copies the temp file into place but leaves it on disk. The previous code tried to unlink it inline with contextlib.suppress, then set transferred=True; if the unlink was silently suppressed the finally guard (if not transferred) would skip cleanup and leak the file. Introduce a temp_consumed flag that is only set True when Strategy 2 (os.unlink + os.replace) physically renames the temp file away. The finally block now always cleans up the temp file unless it was consumed by a rename, covering both the Strategy 1 success path and all failure paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d restart loop Move the inline HTML template from web_config.py into src/astrameter/static/config_editor.html, loaded at import time via importlib.resources. Add a SIGUSR1-based restart loop in main() so the service re-reads config and restarts in-process (useful when no external supervisor is available). Also add validate_config() for trial-loading config files.
The hand-rolled line-level parser for comment-preserving config writes is replaced by the configupdater library, which handles comment/blank preservation, key updates, and section management natively. Adds tests for web_config.py covering read, write, validation, round-trip, and atomic-write helpers.
- Send SIGUSR1 instead of SIGTERM from the web UI restart handler so the process reloads config in-place instead of exiting. - Re-derive device_types, device_ids and skip_test after reloading cfg on restart so changed DEVICE_TYPE / DEVICE_IDS take effect. - Hold _CONFIG_WRITE_LOCK for the entire read-modify-write transaction to prevent concurrent requests from clobbering each other. - Reject duplicate section names in the order list.
…section-aware Rename health_service.py → web_server.py and HealthCheckService → WebServer to reflect its broader role (health endpoint, config editor, restart API). Rename ENABLE_HEALTH_CHECK config key to ENABLE_WEB_SERVER. Add configurable WEB_SERVER_PORT option (default 52500). Move key-type metadata from inline JS to Python (web_config.py) and serve via GET /api/key-types. Make key types section-based with prefix matching so each section gets appropriate input widgets and auto-suggestions. Add datalist-based key auto-completion filtered by section and excluding already-used keys.
…T002 schema types - Validate config against a temp copy before persisting in the POST /api/config handler; return the validation error on failure. - Extract _apply_cli_overrides() so --throttle-interval is re-applied after a web-UI-triggered restart (was lost on config reload). - Fix 13 CT002 key-type declarations from float to integer to match the getint() calls in main.py. - Add PID controller keys (PID_KP/KI/KD/OUTPUT_MAX/MODE) to GENERAL and _PM_COMMON schemas and config.ini.example.
tomquist
force-pushed
the
feature/web-config
branch
from
b82a354 to
1d5c8be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainbranch intodevelop, adapted to work with the async aiohttp architecture andsrc/astrameter/package layoutsrc/astrameter/web_config.py— HTML editor UI and config read/write helpers (atomic writes, comment preservation, duplicate-section validation)HealthCheckServicewith new aiohttp routes:GET /config(editor UI),GET /api/config,POST /api/config,POST /api/restartWEB_CONFIG_ENABLEDconfig option inmain.py(opt-in, defaultFalse)WEB_CONFIG_ENABLEDinconfig.ini.exampleandCHANGELOG.mdTest plan
uv run pytest)ruff formatandruff checkcleanmypy src/cleanWEB_CONFIG_ENABLED = Truein[GENERAL], start astrameter, openhttp://localhost:52500/config— editor loads, sections/keys editable, Save works, Save & Restart restarts the process🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Chore