-
-
Notifications
You must be signed in to change notification settings - Fork 586
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
Allow specifying an account for admin update-email
#7355
Comments
admin update-email
The issue here is that, unfortunately, the point of update-email is to remove (or change) that email from all accounts on which it exists. So it needs to do a table-scan to identify the account(s) to update. |
If you do the dry-run to identify all the accounts, specifying those same accounts on the "actual" execution should avoid needing to make two expensive table scans, and makes it clear ahead of time what rows will be affected |
Seems like that introduces the potential for copy-paste errors, plus makes the process generally more tedious for the operator. Maybe what we need is a confirmation prompt? E.g. "found these accounts, clear them? [y/N]" |
I think a confirmation prompt is a good alternative, especially if we have a -confirm=yes flag to confirm (which feels like a similar idea to -dry-run flag but not really the same? I’m not sure how to resolve that) |
It seems like the process is always to run a dry-run first, at least for clearing emails. Is that correct? If so, unconditionally prompting (rather than adding a flag to control confirmation prompting) seems like the right choice. |
The main concern I have is that we may want to script the tool, including in tests, so having an option to confirm without user interaction is useful. Perhaps the proper thing for that is to have a batch mode built into the tool, though. |
update-email does a full table scan, but it could take an account to update and be efficient.
The dry-run should print accounts found for use in a non-dry-run
The text was updated successfully, but these errors were encountered: