Move extra params definition to backup manager config

[nafiesl]

This PR will be a fix for reported error after laravel 6 support. #118 (comment)

BackupManager\ShellProcessing\ShellProcessFailed
mysqldump: [ERROR] unknown variable 'column-statistics=0'

This fix works for laravel 6 on MySQL 5.7 or MariaDB 10.2.

Kindly please review this @ShawnMcCool, @mitchellvanw.

[robbielove]

This will break functionality for those who require this argument, the PR needs to allow arguments to be passed from a config file instead of hardcoded '' or 'column-statistics=0'

[nafiesl]

Noted @robbielove, I will try to make it optional via the config file.

[robbielove]

If it helps, I believe the parent package seems to handle this with the ability to pass these parameters, you might find this useful: backup-manager/backup-manager#138

[nafiesl]

@robbielove, I moved the extraParams to the backup-manager config file.

About your references here, backup-manager/backup-manager#138, I have tried to change the extraParams key and value on this config file:

vendor/backup-manager/backup-manager/config/database.php

But I don't see the change was affected in Laravel project.

So as far as I try, only the backup-manager config file that can be read by Laravel.

[robbielove]

I have reviewed these changes and realised that this is better suited to an environment variable as the requirement to fill extraParams is based on whether mysql needs --column-statistics=0 or not (e.g. this is required for maria db and maybe mysql 8?) therefore this will be based purely on the enviroment where the app is running.

I propose that you change src/GetDatabaseConfig.php to:
Line 41 'extraParams' => env('DB_PARAMS'),
and remove your changes to config/backup-manager.php

Sorry for suggesting this needs to go in a config file earlier. I have tested your code and also my changes and found both work.

I would also suggest making a note of this in the README.md to suggest that DB_PARAMS="--column-statistics=0" can be added to the .env file for environments that require it. (remembering this can be used for other parameters that I have not explored; like '--max-allowed-packet')

[nafiesl]

@robbielove, I do agree to have this on env variable, since our env on local, staging and production might be different. But we still need to have it in config. Because, if we use php artisan config:cache all of the direct env var calls like: env('xxxxx') will become null.

Docs reference: https://laravel.com/docs/6.x/configuration#configuration-caching

Wdyt @robbielove?

[robbielove]

I agree with that

[nafiesl]

Thanks a lot for your review @robbielove, we will wait for repo maintainer's review to see if it fit their need.