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

Fix socket handling in mysql resource #1971

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Conversation

chris-rock
Copy link
Contributor

Signed-off-by: Christoph Hartmann chris@lollyrock.com

Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

LGTM but needs a quick change to make Rubocop happy. I'll come back to approve once that's done. :)

@@ -44,9 +44,9 @@ def query(q, db = '')
cmd = inspec.command(command)
out = cmd.stdout + "\n" + cmd.stderr
if out =~ /Can't connect to .* MySQL server/ or
out.downcase =~ /^error/
out.downcase =~ /^error/
Copy link
Contributor

Choose a reason for hiding this comment

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

That space is needed to appease Rubocop since it's a multiline conditional. We should also take this opportunity to change the or to || in the line above it.

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>
@chris-rock
Copy link
Contributor Author

Rubocop issue fixed! Thank you @adamleff

@arlimus
Copy link
Contributor

arlimus commented Jun 27, 2017

Kudos for the PR and review guys! :)

@arlimus arlimus merged commit 50e762e into master Jun 27, 2017
@arlimus arlimus deleted the chris-rock/mysql-fixes branch June 27, 2017 10:26
@adamleff adamleff added the Type: Bug Feature not working as expected label Jun 27, 2017
@adamleff adamleff changed the title fix mysql resource Fix socket handling in mysql resource Jun 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants