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

Dynamic user/password in .my.cnf #89

Merged
merged 4 commits into from
Feb 11, 2022
Merged

Conversation

mrpk1906
Copy link
Contributor

User and password in .my.cnf hardcoded to admin/admin
This pull request to allow set dynamic user/pass to proxysql admin

@markuman
Copy link
Member

@mrpk1906 Can you also add a bugfix changelog fragment?

@Andersson007
Copy link
Contributor

For convenience, there's a link to changelog doc https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment

@mrpk1906
Copy link
Contributor Author

Hi @markuman @Andersson007
I updated changelogs

Please check :)

Copy link
Contributor

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

In terms of changelog LGTM.
If this role didn't work with python-mysqldb and after this PR gets merged it will work, LGTM

@markuman
Copy link
Member

markuman commented Feb 11, 2022

If this role didn't work with python-mysqldb and after this PR gets merged it will work, LGTM

hmmm, maybe it works on focal (ubuntu 20.04) with that.
https://github.com/ansible-collections/community.proxysql/blob/main/roles/proxysql/meta/main.yml#L13

And maybe it breaks xenial (ubuntu 16.04)?
Imo xenial is EOL since April 2021.

I'll try to verify next days.

@markuman
Copy link
Member

Tested with bionic and focal ... the role runs in other errors that are not related to that PR.
I'll open a separat issue for it.

@mrpk1906 Thanks for your contribution.

@markuman markuman merged commit c527827 into ansible-collections:main Feb 11, 2022
@Andersson007
Copy link
Contributor

@mrpk1906 thanks for the contribution!
@markuman thanks for reviewing!

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

Successfully merging this pull request may close these issues.

3 participants