mysqldump-php icon indicating copy to clipboard operation
mysqldump-php copied to clipboard

Improve the library extensibility

Open KamilBaczkowski opened this issue 4 years ago • 3 comments

The library is great and works well, unfortunately it doesn't give much to work with when it comes to changing or extending the current behaviour. One of the examples is the variable $dumpSettingsDefault from the __construct method in the Mysqldump class; it actively blocks extensibility, by throwing an error when any non-Mysqldump-standard arguments were passed. I moved it to the class scope, and because of it, anyone wanting to extend the library with new args can just add their own arguments to that array, thus providing a safe to use interface that fails on unknown arguments, but is also extensible. In my own case, I wanted to disable dumping of the views. Unfortunately, since all the methods are protected, I couldn't achieve that (I wanted to specifically reduce the number of queries send to INFORMATION_SCHEMA).

I hope some meaningful discussion emerges and eventually this PR will be merged.

KamilBaczkowski avatar Aug 05 '20 11:08 KamilBaczkowski

Looks like the build failed because of travis, I'm restarting the check.

Meanwhile, for me its ok to change the visibility, after @KamilBaczkowski and what was said by @luc45 in #200.

Same comment here as in the patch to reuse a pdo connection. User can break things and blame mysqldump-php. But if user is extending this library, [s]he is advanced enough to fix his/her own problems.

Will this patch solve both (#200 & #202) issues? @KamilBaczkowski , anything you want to add?

ifsnop avatar Aug 10 '20 16:08 ifsnop

For it to also fix #200 it would have to move PDO default settings to the class instead of limiting it to the constructor. I can do that easily in this PR too.

KamilBaczkowski avatar Aug 10 '20 17:08 KamilBaczkowski

@ifsnop any update when this will be merged ?

krowinski avatar Apr 13 '21 20:04 krowinski

Merged.

ifsnop avatar Feb 09 '23 02:02 ifsnop