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

Create log-bin directory if it doesn't exist #596

Merged
merged 1 commit into from Nov 12, 2014
Merged

Create log-bin directory if it doesn't exist #596

merged 1 commit into from Nov 12, 2014

Conversation

NoodlesNZ
Copy link
Contributor

It seems the MySQL doesn't like it when you use a log-bin directory that doesn't exist. If you add this before the mysql package is installed then it will fail (if the mysql user doesn't exist). If you add the directory after you run the mysql::server class then mysql will fail to start the first time, it will only be resolved on a subsequent puppet run.

@igalic
Copy link
Contributor

igalic commented Nov 7, 2014

can you please squash the changes? we don't wanna have the two prs mixed into one

}
else {
$logbin = ''
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this could be expressed better with a selector

i would also make the default case undef, so that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling to come up with a selector block that replaces this. I'm trying to make sure it handles both log-bin and log_bin (afaik both are acceptable to MySQL).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using pick() from stdlib be acceptable?
E.g.

$logbin = pick($options['mysqld']['log-bin'], $options['mysqld']['log_bin'], '.')

Unfortunately pick() won't let me have the third option as undef or ''. It would mean checking for '.' instead of '' or undef

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally everything from stdlib is fine, since we already depend on it.

@NoodlesNZ
Copy link
Contributor Author

I have squashed commits as requested.

@NoodlesNZ
Copy link
Contributor Author

Ok, I have added pick() and used false as the fallback (if the option doesn't exist or is empty), this makes it easy to use in an if statement.

@igalic
Copy link
Contributor

igalic commented Nov 11, 2014

super! Can you please squash it down to one commit?

@NoodlesNZ
Copy link
Contributor Author

No problem, have done.

igalic added a commit that referenced this pull request Nov 12, 2014
Create log-bin directory if it doesn't exist
@igalic igalic merged commit 56e52fc into puppetlabs:master Nov 12, 2014
@igalic
Copy link
Contributor

igalic commented Nov 12, 2014

thank you @NoodlesNZ \o/

@NoodlesNZ NoodlesNZ deleted the logbindir branch January 29, 2015 02:39
@NoodlesNZ NoodlesNZ restored the logbindir branch January 29, 2015 03:19
@NoodlesNZ
Copy link
Contributor Author

Looking at this the dirname() function uses ruby's File.dirname(). If you don't specify a path (i.e. just mysql-bin) then dirname will return .

I haven't tested to see if this causes problems, but I'll do a check for . and stop puppet from trying to manage a directory for this.

@NoodlesNZ
Copy link
Contributor Author

Should be addressed in PR #654

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants