Fix backup remove for alternate locations #5515
Conversation
agners
force-pushed
the
fix-backup-remove-for-alternate-locations
branch
from
6b2d7b0 to
94a7005
Compare
|
Warning Rate limit exceeded@agners has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request introduces a minor refactoring in the Changes
Sequence DiagramsequenceDiagram
participant BackupManager
participant Backup
participant System
BackupManager->>Backup: get all_locations
BackupManager->>BackupManager: Filter locations
BackupManager->>BackupManager: Remove backup files
alt Removal Successful
BackupManager-->>System: Update resolution state
else Error Occurs
BackupManager-->>System: Log error and update resolution state
end
The sequence diagram illustrates the high-level flow of the backup removal process, highlighting the key interactions between the 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
🧹 Nitpick comments (1)
supervisor/backups/manager.py (1)
266-268: Consider handling any unexpectedNonefrom_get_location_name.Although using the walrus operator here is concise and clear, if
_get_location_name(location)ever returnsNoneunexpectedly (e.g., a misconfiguration or a removed mount), the list comprehension silently excludes that location without logging. If that is intentional, no change is needed; otherwise, consider logging or raising a warning for unrecognized locations to aid debugging.targets = ( [ - location_name - for location in locations - if (location_name := self._get_location_name(location)) in backup.all_locations + location_name + for location in locations + if (location_name := self._get_location_name(location)) is not None + and location_name in backup.all_locations ] if locations else list(backup.all_locations.keys()) )
Currently the API converts backup locations on network mounts to the Supervisor's Mount representation. However, the locations stored in the backup representations is a dictionary with the location string as key. Make sure to use the backup location string to validate the remove requests. This fixes removing backups from network storage mounts.
agners
force-pushed
the
fix-backup-remove-for-alternate-locations
branch
from
94a7005 to
b1878e8
Compare
Proposed change
Currently the API converts backup locations on network mounts to the Supervisor's Mount representation. However, the locations stored in the backup representations is a dictionary with the location string as key.
Make sure to use the backup location string to validate the remove requests. This fixes removing backups from network storage mounts.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit