-
Notifications
You must be signed in to change notification settings - Fork 751
Add dedicated version update refresh for main components #5833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughA new POST API endpoint Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (RestAPI/APIRoot)
participant Updater
Client->>API: POST /reload_updates
API->>Updater: reload()
Updater-->>API: Reload complete
API-->>Client: Response
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
08b7819 to
6075cfc
Compare
Currently we have the /refresh_updates endpoint which updates the main component versions (Core, OS, Supervisor, Plug-ins) and the add-on store at the same time. This combined update causes more update information reloads than necessary. To allow fine grained update refresh control introduce a new endpoint /reload_updates which asks Supervisor to only update main component versions (learned through the version json files). The /store/reload endpoint already allows to update the add-on store separately.
6075cfc to
906c24f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/api/test_root.py (1)
86-99: Good addition of a test case for the new/reload_updatesendpoint.The test correctly verifies that the endpoint refreshes only the updater information by patching and asserting that
fetch_datais called exactly once. This aligns with the PR objective of creating a more fine-grained update refresh process.Consider enhancing the test to explicitly verify that
coresys.store.reloadis NOT called when accessing this endpoint, which would further emphasize the key differentiation from the existing/refresh_updatesendpoint:async def test_api_reload_updates( coresys: CoreSys, api_client: TestClient, ): """Test reload updates.""" coresys.updater.channel = UpdateChannel.STABLE + coresys.store.reload = AsyncMock() with ( patch("supervisor.updater.Updater.fetch_data") as fetch_data, ): resp = await api_client.post("/reload_updates") fetch_data.assert_called_once_with() + assert not coresys.store.reload.called assert resp.status == 200
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/api/test_root.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
- GitHub Check: Run tests Python 3.13.3
🔇 Additional comments (3)
tests/api/test_root.py (3)
4-5: Appropriate import additions.The new imports are correctly added to support the test case's requirements.
6-6: Properly imported TestClient for type hinting.The TestClient import allows for proper type annotation in the test function parameters.
9-9: Correctly imported UpdateChannel enum.The UpdateChannel enum is properly imported to set up the test environment.
115b7d6 to
92509b4
Compare
sairon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. But let's not forgot about the client library and documentation update (along with the deprecation notice probably).
Proposed change
Currently we have the /refresh_updates endpoint which updates the main component versions (Core, OS, Supervisor, Plug-ins) and the add-on store at the same time. This leads to much more updates then necessary.
To allow more fine grained update refresh control introduce a new endpoint /refresh_update which only refreshes the main component versions.
The /store/reload endpoint already allows to update the add-on store only.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit