Fixing empty user/password issue #972
Conversation
[2] pry(main)> pw = ''
=> ""
[3] pry(main)> if pw then :true else :false end
=> :true
[4] pry(main)> indeed. @ajardan how will this impact the other backup methods / templates? from what i gather, the easiest would perhaps even be to not give it any default at all, since a backupuser / backuppassword is always required. or? this is the erlang school of thought
of course in our case it doesn't crash. the puppet compiler catches your empty |
|
@igalic for xtrabackup it is not required, since this doesn't need a MySQL connection. But it is required for mysqldump, so the other 2 methods are fine. And yes, I agree, would be better to keep them all undef, and require/fail wherever it is necessary to perform the backup. |
|
could you please update your commit message to explain this. |
| $backupuser = '', | ||
| $backuppassword = '', | ||
| $backupuser = undef, | ||
| $backuppassword = undef, |
There was a problem hiding this comment.
It seems like this might be a good chance to start using types and label these as Optional[String].
There was a problem hiding this comment.
here it would be okay
but not in the other backup classes.
innobackupex command (which is used by xtrabackup) does not require a username and a password, so those could be undefined. Since in the mysql::backup::xtrabackup those were defined as empty strings, and in the template we only check if the variable is defined, it results in a script that has the --user and --pasword set, but no actual credentials, which breaks it. Other backup providers are not affected, since a username and password are required.
ajardan
force-pushed
the
fix_empty_userpass_xtrabackup_template
branch
from
2541814
to
9d8d30f
Compare
|
@ajardan Data types are available in Puppet 3 with the future parser. |
…mplate Fixing empty user/password issue
With empty strings the template has the --user and --password options, but those are empty.