-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
storage: Disable "Apply" in Teardown dialogs after error #19579
storage: Disable "Apply" in Teardown dialogs after error #19579
Conversation
A dialog with teardown actions might fail in the middle of all of it. (For example it might succeed to unmount one filesystem and then fail to unmount a second). With the current code, just hitting "Apply" again will run the exact same teardown actions again, which will likely fail. (For example, we would try to unmount the first filesystem again.) We could try to make the teardown more tolerant, but the best thing to do is to recompute all the actions and present them to the user again. This PR does the recomputation by forcing the user to open the dialog again. We could also recompute the teardown immediately when the error happens and update the dialog to reflect it. But that has the disadvantage that we change the list of actions that the dialog displays unexpectedly. |
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.
Unit tests fail:
test/verify/check-storage-used:170:9: F841 Local variable sleep_pid
is assigned to but never used
We want the user to cancel and start over.
193b725
to
872713b
Compare
@@ -138,6 +138,7 @@ export class VDODetails extends React.Component { | |||
Teardown: TeardownMessage(usage), | |||
Action: { | |||
Title: _("Stop"), | |||
disable_on_error: usage.Teardown, |
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.
This added line is not executed by any test. Details
@@ -199,6 +200,7 @@ | |||
Action: { | |||
Title: _("Delete"), | |||
Danger: _("Deleting erases all data on a VDO device."), | |||
disable_on_error: usage.Teardown, |
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.
This added line is not executed by any test. Details
We want the user to cancel and start over.