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

exportdb: Implemented --dry-run on some options which change data. #1682

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

oPromessa
Copy link
Contributor

exportdb: Implemented --dry-run on some options which change data: create, check, repair, vacuum, save-config, delete-uuid, delete-file, upgrade, sql

Used "[Dryrun]" prefix in some output messages when using --dry-run. Don't know if it is aligned with the message output philosophy. ;)

migrate_photos_library added to sub_commands (to run only one of sub_commands)

…eate, check, repair, vacuum, save-config, delete-uuid, delete-file, upgrade, sql

Used "[Dryrun]" prefix in some output messages when using --dry-run. Don't know if it is aligned with the message output philosophy. ;)

migrate_photos_library added to sub_commands (to run only one of sub_commands)
@RhetTbull
Copy link
Owner

Thanks for this. Will merge as soon as I have time.

@RhetTbull
Copy link
Owner

Hi @oPromessa I find --dry-run useful for cases where data is changed and we can provide the user a preview of what will be changed. With the exportdb commands, there's now way to provide a review so it's not clear to me that adding a --dry-run command here is that useful. Is there a particular use case you had in mind or an issue that bit you?

@oPromessa
Copy link
Contributor Author

Is there a particular use case you had in mind or an issue that bit you?

Understand your point.

I had a stubborn file which was being exported and added "(1)" each time it ran. As there were 'no longer valid' entries on the exportdb -- it's a known situation for which one uses exportdb delete.

I went ahead to run exportdb delete and used dry-run to see the impact beforehand, and to my surprise it deleted immediately without following dry-run. Not a of deal as it actually solved the issue 😃

So I went ahead and applied dry-run where I thought it'd made sense.

Maybe consider only options that change database, but then one would have to also notify the user dry-run doesn't really apply to all exportdb operations and complicate the options handling.

Im fine with whichever approach you consider best.

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

Successfully merging this pull request may close these issues.

2 participants