mysqldump additional-options behavior discrepancy

[Andrew Garner]

The mysqldump plugin expects the 'additional-options' setting to provide a comma separated list. However the configspec currently has a comment that notes this should be provided "exactly as on the command line". This causes multiple options to be misinterpretted (and mysqldump will fail) and is confusing. The comment/documentation at least needs to be clear on the format required here and possibly the implementation updated.

Example:

additional-options = --no-data, --routines # OK
additional-options = --no-data --routines # misinterpreted as mysqldump "--no-data --routines"

[m00dawg (tim-moocowproductions)]

Online docs (1.0 and 1.1) are also a bit vague:

"additional-options = <mysqldump arguments>

Can optionally specify additional options directly to mysqldump if there is no native Holland option for it."

That said, I wonder if it would be better to use the latter syntax (if possible) as it feels more like what you would put on the command-line? Otherwise, yes, this needs updating and clarification.

[Andrew Garner (muzazzi)]

Fixing additional-options is difficult now, since it has been a comma separated list for a long time before this bug was opened. In holland 1.1+ option aliases in config files are supported so we can potentially fix this there while maintaining compatibility by providing a separate option name.

[Andrew Garner (muzazzi)]

Just revisiting this as I was thinking about this previously. I think the best way forward here is to treat this as a comma-separated list of commandlines, where we parse each comma-separated string as a commandline.

So you can have messy configs like:

additional-options = --foo --bar, --baz

Which would be equivalent to:

additional-options = --foo --bar --baz

If a comma is required in an option, then quoting is needed:

additional-options = "--foo --bar --baz=,"

But this is the case already as configobj's list parsing is fairly simple.

In 1.1+ we can catch this error in the validator and raise a deprecation warning and eventually deprecate the comma separated approach entirely.

Still holding this sort of change off for a 1.1+ release.