Increase time between update checks #5413
Conversation
mdegat01
added
the
refactor
📝 WalkthroughWalkthroughThe changes involve a modification to the constant Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tasks
participant Updater
User->>Tasks: Schedule task
Tasks->>Updater: Set reload interval (27100 seconds)
Updater-->>Tasks: Acknowledge scheduling
Tasks-->>User: Task scheduled successfully
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
supervisor/misc/tasks.py (1)
36-36: LGTM! Consider documenting the rationale.The increase from 7200 to 27100 seconds (~2h to ~7.5h) aligns with the PR objective to reduce update check frequency and address rate limiting issues.
Consider adding a comment explaining why this specific interval was chosen and its relationship to rate limits. This will help future maintainers understand the rationale behind this value.
+# Increased from 2h to ~7.5h to prevent rate limiting issues with update checks RUN_RELOAD_UPDATER = 27100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
supervisor/misc/tasks.py(1 hunks)
🔇 Additional comments (1)
supervisor/misc/tasks.py (1)
36-36: Verify impact on critical updates.
While reducing update frequency helps with rate limits, we should ensure this doesn't delay critical updates too much. The code shows that supervisor updates are still handled immediately when detected:
_reload_updaterchecks for updates- If supervisor needs update, it calls
_update_supervisorimmediately - Other components wait for their next scheduled check
This implementation maintains a good balance between reducing API calls and ensuring critical updates are applied promptly.
Also applies to: 442-450
There was a problem hiding this comment.
This will pace our rollout similar to how things were before #5375. It will take slightly below 8h to complete a Supervisor rollout from Supervisor client perspective.
Before it took up to 2h for a Supervisor to learn about the update, then depending on timing of the Supervisor update task up to another 8h for the update to actually happen, so worst case 10h.
Note however that this will slowdown rollout of plug-ins a bit, since they themself test for updates roughly every 8h. Since the updater reload task is now 8h, it can take up to 16h for a plug-in to be rolled out. Which is fine, but something to keep in mind.
LGTM, thanks! 🙏
Proposed change
Increase time between update checks from 2 hours to ~8 hours (staggered with the other tasks as per our existing pattern). This is in response of our recent issues around updates hitting rate limits.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit