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

The hook provided by setInfoHook is called too late #39

Closed
guvra opened this issue Feb 28, 2024 · 4 comments
Closed

The hook provided by setInfoHook is called too late #39

guvra opened this issue Feb 28, 2024 · 4 comments

Comments

@guvra
Copy link

guvra commented Feb 28, 2024

Hi,

I'm working on a feature for gdpr-dump that displays the dump progress with a progress bar:
Smile-SA/gdpr-dump#113

The goal is to be able to know which tables take a long time to dump when creating a dump file for a very big database.

I'm using the function setInfoHook to make the progress bar advance.
However, it's not working properly, because the hook is triggered after a table was dumped.
Instead of displaying the name of the table that is being dumped, it displays the name of the previous table.

For this feature to work properly, I either need the hook to be moved before a table is dumped, or another hook to be added.
What do you think would be the best solution?
I can provide a PR.

@staabm
Copy link

staabm commented Feb 28, 2024

I agree that having a event before dumping the table would make more sense for DX. when the goal of the hook is to allow printing progress I think we should move it before the dump.

in case the hook is used in the wild to do other stuff as well, it might make sense to add another hook, so we don't break BC.

@staabm staabm mentioned this issue Feb 28, 2024
@staabm
Copy link

staabm commented Feb 28, 2024

in case we implent the suggestion in https://github.com/druidfi/mysqldump-php/pull/38/files#r1505546205 the current event could stay after the dump and it should work without a BC break

@back-2-95
Copy link
Member

@guvra you good with this one now that 1.1.0 was released?

@guvra
Copy link
Author

guvra commented Mar 4, 2024

@back-2-95 Yes, it's working fine, thanks for the new release 👍

@guvra guvra closed this as completed Mar 4, 2024
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