feat: support bulk backup backing images #836
Conversation
Signed-off-by: Yi-Ya Chen <[email protected]>
WalkthroughThis pull request introduces modifications to the backing image management system, focusing on enhancing the bulk backup functionality. The changes span multiple files, including the model, routes, and styling components. The primary updates involve refactoring the Changes
Sequence DiagramsequenceDiagram
participant User
participant BulkActions
participant BackupModal
participant BackupService
User->>BulkActions: Select backing images
User->>BulkActions: Trigger Backup
BulkActions->>BackupModal: Open Backup Modal
User->>BackupModal: Select Backup Target
User->>BackupModal: Confirm Backup
BackupModal->>BackupService: Execute Backup
BackupService-->>BulkActions: Backup Completed
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 1
🧹 Nitpick comments (4)
src/models/backingImage.js (1)
215-215: Add error handling and consider concurrency.While passing both the URL and the backing image object to execAction helps provide context, note the following potential improvements:
• Handle potential errors or failures returned from the call to execAction, e.g., by assessing resp.status or using try/catch for robust error handling.
• If the payload array can be large, running each backup sequentially may be slow. Introducing concurrency (e.g., using Promise.all or dispatching multiple sagas) could improve performance and user experience.src/routes/backingImage/BackingImageBulkActions.js (2)
145-179: Form validation could be enhanced.
Currently, the user can proceed without choosing a valid target if the default is missing. Consider marking the 'Backup Target' field as required for a smoother UX.
193-193: Legacy Form.create() usage
Form.create() indicates usage of an older Ant Design pattern. Confirm this is intentional and supported, or consider the newer hooks-based form if upgrading.src/routes/backingImage/index.js (1)
128-128: Single retrieval of backup targets is clean.
Storing backupTargets in a variable improves readability and avoids repeated calls to getBackupTargets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/models/backingImage.js(1 hunks)src/routes/backingImage/BackingImageBulkActions.js(1 hunks)src/routes/backingImage/BackingImageBulkActions.less(1 hunks)src/routes/backingImage/index.js(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/backingImage/BackingImageBulkActions.less
🔇 Additional comments (8)
src/routes/backingImage/BackingImageBulkActions.js (6)
1-3: Imports look consistent and necessary.
These imports are all used, and there are no obvious redundancies.
7-15: Good use of default values for props.
Destructuring the props with default values (backupTargets = [], form = {}) helps avoid runtime errors if these props are undefined.
33-54: Deletion confirmation is well-structured.
The code clearly displays a list of selected items before confirming deletion, improving user awareness. No changes suggested.
83-85: Exposing backup modal via state is straightforward.
Toggling the backup modal using a boolean state is simple and effective.
114-117: Disable condition for 'Download' action is appropriate.
It excludes rows that are not ready or use the v2 data engine, avoiding partial/inconsistent downloads.
183-190: PropTypes are thorough.
Marking each prop as required clarifies usage and prevents runtime issues.
src/routes/backingImage/index.js (2)
183-183: Passing backupTargets to modals and bulk actions.
This maintains consistency across components and reduces duplication.
Also applies to: 331-331
349-349: Minor structural update.
No issues identified at line 349; the change appears to be a closing brace or an updated line break.
|
All UI changes are good. But I can't backup any backing image in cifs backup target. 
POST |
|
@mergify backport v1.8.x |
✅ Backports have been created
|
What this PR does / why we need it
Issue
[UI][FEATURE] Multiple backup stores support #8647
Test Result
Backing ImagepageBackupBackupAdditional documentation or context
Test with
LONGHORN_MANAGER_IP=http://134.209.101.112:30001/ npm run devSummary by CodeRabbit
New Features
Bug Fixes
Style
Documentation