Conversation
agners
left a comment
There was a problem hiding this comment.
Looks good, looking forward to the new v2 API 😎
|
|
||
| CORE_FRONTEND: Final = re.compile( | ||
| r"^(?:" + _CORE_FRONTEND_PATHS + r")$" | ||
| _V2_FRONTEND_PATHS: Final = ( |
There was a problem hiding this comment.
Hm, I think we can drop frontend from v2 from the getgo.
There was a problem hiding this comment.
Hm. So here's the actual paths in here:
_V2_FRONTEND_PATHS: Final = (
r"|/app/.*\.(?:js|gz|json|map|woff2)"
r"|/v2/store/apps/" + RE_SLUG + r"/(logo|icon)"
)/app is the app store panel. And yea I think you're right about that, that's gone now.
But what about the icons and logos? Are you sure frontend doesn't ask Supervisor for those in the new UI? Where else would they come from?
| r"|/apps(?:/" + RE_SLUG + r"/(?!security).+|/reload)?" | ||
| r"|/audio/.+" | ||
| r"|/auth/cache" | ||
| r"|/available_updates" |
There was a problem hiding this comment.
.. not sure if we should deprecate this one as well. From what I can see the Core isn't using it? 🤔
There was a problem hiding this comment.
Those are in the Manager role though, they aren't core only. The manager role is one that addons can declare they use so we'd need to check addons for usage of that.
However we can drop the |/reload bit. I just checked, that's there because there's a legacy route of /addons/reload that really maps to /store/reload. That path won't exist in v2. The others I expect to exist for now so I think we leave it as is unless we have a reason to deny apps access to these paths.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a feature-flagged, versioned /v2 Supervisor API that consistently uses apps terminology while keeping the existing v1 endpoints backward-compatible by delegating to shared handlers and translating field names where needed.
Changes:
- Added a
/v2aiohttp sub-app (gated byFeatureFlag.SUPERVISOR_V2_API) and registered v2 routes for apps, backups, and store without duplicating handler logic. - Split API/backup terminology constants (
ATTR_ADDONSvsATTR_APPS) and updated v1/v2 response/request schemas to use the appropriate keys. - Expanded the test suite with fixtures and assertions to validate v1/v2 path behavior and key naming.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Updates backup test data to use the legacy addons key via ATTR_ADDONS. |
| tests/api/test_store.py | Refactors store endpoint tests to run against both v1 and v2 roots; adds v2 key assertions. |
| tests/api/test_backups.py | Refactors some tests to be v1/v2-prefix aware; adds v2-specific backup API tests. |
| tests/api/test_addons.py | Refactors app management tests to run against both v1 /addons and v2 /v2/apps; adds v2 list-key test. |
| tests/api/conftest.py | Adds api_client_v2 and shared (client, prefix/root) fixtures for dual-version testing. |
| supervisor/const.py | Introduces ATTR_ADDONS = "addons" and changes ATTR_APPS to "apps" for v2 payloads. |
| supervisor/backups/validate.py | Ensures backup validation schema continues to use the legacy addons key. |
| supervisor/backups/backup.py | Keeps backup internal data keyed by legacy addons while exposing it via the apps property. |
| supervisor/api/supervisor.py | Updates Supervisor info payload to keep returning legacy addons key. |
| supervisor/api/store.py | Adds shared store data helper and v1/v2 variants that differ only by top-level key (addons vs apps). |
| supervisor/api/middleware/security.py | Splits security regex patterns per API version (_V1_PATTERNS / _V2_PATTERNS) and selects based on path. |
| supervisor/api/backups.py | Splits v1/v2 schemas and endpoints for backups; adds key-translation helpers and shared business-logic helpers. |
| supervisor/api/addons.py | Adds shared list/info builders and v1/v2 list endpoints differing by response key. |
| supervisor/api/init.py | Mounts a /v2 sub-app when feature flag is enabled; registers v1 wrappers and v2 canonical routes. |
| pyproject.toml | Updates pylint configuration for re.Pattern members used in the new dataclass-based pattern set. |
Introduce a v2 API sub-app mounted at /v2 that uses 'apps' terminology throughout, while keeping v1 fully backward-compatible. Key changes: - Add ATTR_ADDONS = 'addons' constant alongside ATTR_APPS = 'apps' so backup file data (which must remain 'addons' for backward compat) and v2 API responses can use distinct constants - Add FeatureFlag.SUPERVISOR_V2_API to gate v2 route registration - Mount aiohttp sub-app at /v2 in RestAPI.load() when flag is enabled - Add _AppSecurityPatterns frozen dataclass and _V1_PATTERNS/_V2_PATTERNS with strict per-version regex sets (no cross-version matching) - Add _register_v2_apps, _register_v2_backups, _register_v2_store route registration methods - Add v1 thin wrapper methods (*_v1) for all affected endpoints so business logic lives in the canonical v2 methods - Extract _info_data() helper in APIApps so v1 closure can bypass @api_process and still catch APIAppNotInstalled for store routing - Add _rename_apps_to_addons_in_backups(), _process_location_in_body(), _all_store_apps_info() shared helpers to eliminate duplication - Add api_client_v2, api_client_with_prefix, app_api_client_with_root, store_app_api_client_with_root parameterized test fixtures - Add test_v2_api_disabled_without_feature_flag - Parameterize backup, addons, and store tests to cover both v1 and v2 paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
re.Pattern methods (match, search, etc.) are C extension methods. Pylint cannot detect them via static analysis when re.Pattern is used as a type annotation in a dataclass field, producing false E1101 no-member errors. Add generated-members to inform pylint these members exist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f9fbcf3 to
824a25a
Compare
Proposed change
Introduce a versioned
/v2API for the Supervisor that uses apps terminology consistently, while keeping the v1 API fully backward-compatible. Builds on the internal code rename from PR #6717.The rebranding of addons → apps requires breaking API changes (path and field renames). Rather than making these in-place and breaking existing clients, a versioned v2 API allows both old and new clients to work during a transition period.
Design
/v2— clean URL prefix with isolated routing and no code duplicationFeatureFlag.SUPERVISOR_V2_APImust be enabled; v2 routes are registered atload()time only when the flag is setapps↔addons)_V1_PATTERNSand_V2_PATTERNSmatch only their respective paths; no cross-version fallback_rename_apps_to_addons_in_backups(),_process_location_in_body(),_all_store_apps_info(),_info_data()eliminate duplicationPath Mapping
/addons/{slug}/.../v2/apps/{slug}/.../store/addons/{slug}/.../v2/store/apps/{slug}/.../backups/...(addonsfield)/v2/backups/...(appsfield)/store(addonsfield)/v2/store(appsfield)Constant Split
ATTR_APPS = "apps"— v2 API responses/requestsATTR_ADDONS = "addons"— v1 API responses and backup file format (must never change — old backups would be unreadable)Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed: