Skip to content
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

fix: remove bulk instance migration feature due to server loading concerns #869

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

mas-who
Copy link
Collaborator

@mas-who mas-who commented Aug 29, 2024

Done

  • From the discussion with lxd team, there are concerns about potentially overloading the server if bulk migration is performed on a large number of instances. For now, we will remove the feature and wait for the team to add background task scheduling capability so we can do bulk actions in the UI without issues.

Fixes [list issues/bugs if needed]

QA

  1. Run the LXD-UI:
  2. Perform the following QA steps:
    • For the UI connected to a lxd cluster, check that the Migrate button is no longer visible when selecting multiple instances on the instance list page.

@webteam-app
Copy link

Copy link
Contributor

@Kxiru Kxiru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big oof. Looks good to me though. After all, it's not affecting anything else that is there. I'll allow David to give a final review.

Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan. If we remove it, we should also clean up the isBulkMigration from the MigrateInstanceModal and migrateInstanceBulk from the api/instances.tsx file.

@mas-who mas-who force-pushed the remove-bulk-instance-migration branch 2 times, most recently from 8271003 to 25398b0 Compare August 29, 2024 17:25
@mas-who
Copy link
Collaborator Author

mas-who commented Aug 29, 2024

I am not a fan. If we remove it, we should also clean up the isBulkMigration from the MigrateInstanceModal and migrateInstanceBulk from the api/instances.tsx file.

Yeah I'm also not a huge fan of this change, but I think overloading the server is a valid point. If you don't want to merge this one, maybe when I get back we can look into processing batches of requests instead. In the interim, I've done some more cleaning up, removed all bulk migration related logic and also adjusted the modal component to process a single instance only.

@mas-who mas-who force-pushed the remove-bulk-instance-migration branch from 25398b0 to f39b31e Compare August 29, 2024 17:31
@mas-who mas-who force-pushed the remove-bulk-instance-migration branch from f39b31e to dcb933c Compare September 9, 2024 07:23
Copy link
Collaborator

@edlerd edlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mas-who mas-who merged commit 94a7510 into canonical:main Sep 9, 2024
12 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 9, 2024
Kxiru pushed a commit to Kxiru/lxd-ui that referenced this pull request Sep 9, 2024
…cerns (canonical#869)

- From the discussion with lxd team, there are concerns about
potentially overloading the server if bulk migration is performed on a
large number of instances. For now, we will remove the feature and wait
for the team to add background task scheduling capability so we can do
bulk actions in the UI without issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants