Skip to content

(MODULES-3401) Fix for mysql version retrieval #869

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

Merged
merged 1 commit into from
Jul 22, 2016

Conversation

HelenCampbell
Copy link
Contributor

No description provided.

@HelenCampbell HelenCampbell changed the title Fix for mysql version retrieval (MODULES-3401) Fix for mysql version retrieval Jul 21, 2016
@HelenCampbell
Copy link
Contributor Author

Fixes issues around the retrieval of mysql version string, though further issues still remain these will be addressed in a separate issue.

@DavidS DavidS merged commit 8482efc into puppetlabs:master Jul 22, 2016
@HelenCampbell
Copy link
Contributor Author

Hey @jtopper! When does this code run on a host that neither has mysqld or the fact set? Without that information, the provider cannot generate the correct SQL and interactions, and will only fail later when trying to talk - wrongly - to the server, which is why this fix was implemented. Does that help clarify for you?

@jtopper
Copy link
Contributor

jtopper commented Sep 29, 2016

@HelenCampbell We manage Amazon RDS MySQL instances from hosts that don't have mysqld installed. In a number of our use cases, the default queries that the module builds are sufficient, so we haven't introduced facts (though obviously we will now be doing so, on account of this change :)

Ultimately I think you're right that this is an unusual case (this module really isn't great at managing remote databases), but I think the flow here of "look up the fact, then call mysqld -V" is a bit weird when the fact itself falls back to calling mysqld -V. Perhaps it should throw a more obvious error, with instructions on adding an appropriate version fact?

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.

5 participants