community.mysql icon indicating copy to clipboard operation
community.mysql copied to clipboard

Update the list of supported connectors

Open ziegenberg opened this issue 3 years ago • 14 comments

SUMMARY

Update the list of supported connectors (especially the links) in README.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME
  • README.md
  • module_utils
ADDITIONAL INFORMATION

ziegenberg avatar Jun 01 '21 21:06 ziegenberg

For a first try, I've gone with the current minimum version introduced in #163. So this would be:

  • PyMySQL >= v0.7.10
  • MySQLdb >= v1.2.5

ziegenberg avatar Jun 01 '21 21:06 ziegenberg

Codecov Report

Merging #178 (d510eeb) into main (71b2742) will decrease coverage by 0.79%. The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   76.92%   76.12%   -0.80%     
==========================================
  Files          20       20              
  Lines        1794     1801       +7     
  Branches      439      444       +5     
==========================================
- Hits         1380     1371       -9     
- Misses        268      281      +13     
- Partials      146      149       +3     
Impacted Files Coverage Δ
plugins/module_utils/mysql.py 70.00% <25.00%> (-13.14%) :arrow_down:
plugins/modules/mysql_query.py 86.58% <0.00%> (-2.44%) :arrow_down:
plugins/modules/mysql_db.py 74.21% <0.00%> (-0.35%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 71b2742...d510eeb. Read the comment docs.

codecov[bot] avatar Jun 01 '21 21:06 codecov[bot]

@ziegenberg thanks for the PR!

"msg": ".... Exception message: module 'MySQLdb' has no attribute '__version__'"

Is there another way to get it?

Andersson007 avatar Jun 03 '21 05:06 Andersson007

BTW before pushing, you can run all the tests locally, see https://github.com/ansible/community-docs/blob/main/create_pr_quick_start_guide.rst . It's very simple. Needs only Docker installed.

Andersson007 avatar Jun 03 '21 05:06 Andersson007

and for this collection you should use just --docker option without specifying a distro. So it'll run in a default container. My command usually look like (you should be in ~/ansible_collection/community/mysql/, the path is important, mysql is the cloned repo with changes, see the guide above):

ansible-test integration test_mysql_user --docker -vvv > ~/test.log

Andersson007 avatar Jun 03 '21 05:06 Andersson007

@ziegenberg if you find something unclear while reading the guide https://github.com/ansible/community-docs/blob/main/create_pr_quick_start_guide.rst. I'd be really happy if you share it (feel free to create an issue under https://github.com/ansible/community-docs).

Andersson007 avatar Jun 03 '21 05:06 Andersson007

@ziegenberg thanks for the PR!

"msg": ".... Exception message: module 'MySQLdb' has no attribute '__version__'"

Is there another way to get it?

Yeah, I know. What a bummer. But I haven't had enough time to looked into it further. It was a bit late the other day and I had no energy left to setup testing. So I thought let's see what ci will make of it. Looks like it wasn't too fond of my code. :)

I'll have to investigate into this issue and do some local debugging.

ziegenberg avatar Jun 03 '21 06:06 ziegenberg

But I haven't had enough time to looked into it further. It was a bit late the other day and I had no energy left to setup testing.

There's no rush:) The main thing is not to burn out:)

Andersson007 avatar Jun 03 '21 07:06 Andersson007

Maybe it MySQL-Python should be dropped and also replaced by mysqlclient like in https://github.com/ansible-collections/community.proxysql/commit/df5ede0077c40da0f850410fd6a0f9fe14dade1f
plus rework error handling with from ansible.module_utils.basic import missing_required_lib

markuman avatar Jul 25 '21 09:07 markuman

@ziegenberg @bmalynovytch @Jorge-Rodriguez what do you think of ^ ?

Andersson007 avatar Jul 26 '21 11:07 Andersson007

@ziegenberg @bmalynovytch @Jorge-Rodriguez what do you think of ^ ?

I agree, but we shouldn't further to follow the usual procedure of warnings about the connector no longer being supported and so on

Jorge-Rodriguez avatar Jul 26 '21 11:07 Jorge-Rodriguez

@ziegenberg PR #163 has been superseded by #246

Jorge-Rodriguez avatar Nov 20 '21 08:11 Jorge-Rodriguez

Hi! Sorry for the long absence. I'll try to come up with the necessary changes in the next two weeks if that's ok with you.

ziegenberg avatar Dec 05 '21 22:12 ziegenberg

Hi! Sorry for the long absence. I'll try to come up with the necessary changes in the next two weeks if that's ok with you.

Welcome back! And thank you for working on this.

Jorge-Rodriguez avatar Dec 06 '21 06:12 Jorge-Rodriguez