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

Allow specifying an account for admin update-email #7355

Open
Tracked by #7351
mcpherrinm opened this issue Mar 5, 2024 · 6 comments
Open
Tracked by #7351

Allow specifying an account for admin update-email #7355

mcpherrinm opened this issue Mar 5, 2024 · 6 comments

Comments

@mcpherrinm
Copy link
Contributor

mcpherrinm commented Mar 5, 2024

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

@mcpherrinm mcpherrinm changed the title 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. Allow specifying an account for admin update-email Mar 5, 2024
@aarongable
Copy link
Contributor

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.

@mcpherrinm
Copy link
Contributor Author

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

@jsha
Copy link
Contributor

jsha commented Mar 7, 2024

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]"

@mcpherrinm
Copy link
Contributor Author

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)

@jsha
Copy link
Contributor

jsha commented Mar 7, 2024

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.

@mcpherrinm
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants