change deprecated parameter pw and db #116

ziegenberg · 2021-03-12T14:38:51Z

SUMMARY

Parameter db and passed are deprecated in PyMySQL since v1.0.0 and are kept as an alias for database and password. Warnings will be activated around 2022 (for details see PyMySQL/PyMySQL#939).

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

module_utils

ADDITIONAL INFORMATION

codecov · 2021-03-12T14:51:46Z

Codecov Report

Merging #116 (095e75c) into main (baea97d) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #116   +/-   ##
=======================================
  Coverage   75.75%   75.75%           
=======================================
  Files          12       12           
  Lines        1712     1712           
  Branches      438      438           
=======================================
  Hits         1297     1297           
  Misses        269      269           
  Partials      146      146

Impacted Files	Coverage Δ
plugins/module_utils/mysql.py	`83.13% <100.00%> (ø)`

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 baea97d...095e75c. Read the comment docs.

Andersson007 · 2021-03-12T15:28:07Z

@ziegenberg hi, thanks for the PR!
Please add a changelog fragment with minor_changes: see https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment (please follow the format), for example:

minor_changes:
- mysql module utils - change deprecated connection parameters ``passwd`` and ``db`` to ``password`` and ``database`` (https://github.com/ansible-collections/community.mysql/pull/116).

https://dev.mysql.com/doc/connector-python/en/connector-python-connectargs.html also doesn't recommend to use db and passwd

@bmalynovytch @Jorge-Rodriguez what do you think? As far as i know we test against both pymysql and mysqldb and the tests seem to be green.
If you're ok, feel free to to push the big green button

ziegenberg · 2021-03-12T15:45:23Z

@Andersson007 changelog fragment added.

Andersson007 · 2021-03-12T15:52:39Z

@ziegenberg thanks! BTW don't worry about codecov

Andersson007 · 2021-03-12T15:53:06Z

Let's wait for the folks opinion

Jorge-Rodriguez · 2021-03-15T11:53:09Z

@ziegenberg I'm worried about the backwards compatibility of this change. Do you have the version numbers when these options were deprecated?

ziegenberg · 2021-03-15T15:12:17Z

It was introduced into PyMySQL with this PR on Jun 6, 2014: PyMySQL/PyMySQL#247
If I saw this correctly, it's available at least since version 0.6.3.

Jorge-Rodriguez · 2021-03-15T15:43:26Z

@ziegenberg I'm afraid this change breaks the compatibility with MySQLdb connector that still uses the parameter names deprecated in pymysql, as seen here

But then, the tests pass, so it must be an issue with MySQLdb's documentation, and since we're testing all the connector versions that we support, I suppose it's all good.

Andersson007 · 2021-03-16T06:19:29Z

@ziegenberg thanks for the PR!
@Jorge-Rodriguez thanks for reviewing and merging!

ziegenberg · 2021-03-16T13:02:20Z

@Jorge-Rodriguez in MySQLdb they also have password and passwd as an alias, so I guess updating the docu, as you said, seems the right approach.

ziegenberg · 2021-03-16T13:03:05Z

Ah, they already have updated the docs in their main branch. See this commit PyMySQL/mysqlclient@fa25358

tschoonj · 2021-04-22T10:07:52Z

I am seeing the following error due to this change:

failed: [vm-ip] (item=rfi-stfc-guacamole-vm.novalocal) => {"ansible_loop_var": "item", "changed": false, "item": "rfi-stfc-guacamole-vm.novalocal", "msg": "unable to connect to database, check login_user and login_password are correct or /root/.my.cnf has the credentials. Exception message: 'database' is an invalid keyword argument for this function"}

The remote is using MySQL-python-1.2.5-1.el7.x86_64.

tschoonj · 2021-04-22T10:37:52Z

Switching to PyMySQL fixed things for me, but looks like backwards compatibility was broken with MySQLdb.

ziegenberg · 2021-04-22T10:40:58Z

I'm not sure if MySQL-python-1.2.5 is a supported version of this collection? It was released in 2014.

Jorge-Rodriguez · 2021-04-22T10:51:55Z

As of now, only MySQLdb version 2 is supported.

tschoonj · 2021-04-22T10:58:33Z

Ok that's fine, but it might be wise to update the docs accordingly.

Jorge-Rodriguez · 2021-04-22T11:18:22Z

You're very correct about that. And it's actually planned, along with the list of supported server versions.

awegrzyn · 2021-04-22T14:54:01Z

Hi,
I've spent a bit of time today to debug an issue with our deployment as we've been getting Exception message: 'database' is an invalid keyword argument for this function".
Would it be possible to deprecate MySQL-python-1.2.5 more gracefully?
Docs still says: Requires the PyMySQL (Python 2.7 and Python 3.X) or MySQL-python, and on C7 MySQL-python brings 1.2.5...
Thanks.

Jorge-Rodriguez · 2021-04-22T16:03:04Z

and on C7 MySQL-python brings 1.2.5...

Is that CentOS 7?

Andersson007 · 2021-04-23T04:58:41Z

#151

maybe we could just quickly revert this and release? The change doesn't bring a lot in comparison with the consequences

Andersson007 · 2021-04-23T05:11:15Z

or 2) quick patch like "if this error occurs, use db" ?

Jorge-Rodriguez · 2021-04-23T07:31:16Z

Let's revert and consider a more seamless approach to reimplement it

Jorge-Rodriguez · 2021-04-23T07:38:30Z

@ziegenberg could you check what was the version of MySQLdb that introduced the new database and password keywords?

ziegenberg · 2021-04-23T07:53:22Z

A quick check brought up that it might be v1.3.8 in 2016. I'll check thoroughly when I'm back at my desk. Currently only on a mobile.

Andersson007 · 2021-04-23T11:10:49Z

Reverted with #153. will try to release today.
I think the best approach is to check driver versions and behave correspondingly.
See the surrounding code in the mysql_connect function - there's an example of the check.

Thanks everyone for collaborating!

Andersson007 · 2021-04-23T14:49:35Z

Released:) Who wants to handle it gracefully now, welcome:)

Andersson007 · 2021-04-23T14:51:32Z

Available on galaxy https://galaxy.ansible.com/community/mysql. How to install from galaxy, refer to https://galaxy.ansible.com/community/mysql, or wait for the next Ansible package release.

Andersson007 · 2021-05-14T09:40:15Z

@ziegenberg hi, would you like to continue working on this?

ziegenberg · 2021-05-14T10:14:27Z

I can give it another try. But first, I want to say sorry for the disruption my contribution has caused.

Before implementing this, I guess we need to sort out the supported versions of the connectors. And fix the CI accordingly. I think this should go into a separate PR beforehand.

Andersson007 · 2021-05-14T10:27:47Z

I can give it another try. But first, I want to say sorry for the disruption my contribution has caused.

Don't worry about it much, this happens to all of us from time to time (for example, I broke and am breaking stuff a lot but it gives valuable experience). It's normal. Now we know that we should be careful about driver versions even if they seem to be absolutely obsolete.

Before implementing this, I guess we need to sort out the supported versions of the connectors. And fix the CI accordingly. I think this should go into a separate PR beforehand.

Sounds very sensible.
I think the related files are stored in .github/workflows/ and tests/integration/targets/setup_mysql/. If i missed something @Jorge-Rodriguez can correct me.

@ziegenberg feel free to create related issues / PRs and ask if there are any questions on your mind.

Thanks!

Jorge-Rodriguez · 2021-05-14T10:50:22Z

@ziegenberg I'm working on a PR tackling CI versions, you can keep an eye on #163

ziegenberg · 2021-06-01T20:46:30Z

I've created #176 to track the discussion of changing deprecated parameter pw and db.

ziegenberg added 2 commits March 12, 2021 15:00

change deprecated parameter pw to password

30247e7

change deprecated parameter db to database

b3a9937

ziegenberg changed the title ~~Fix deprecations~~ change deprecated parameter pw and db Mar 12, 2021

add changelog fragment

095e75c

Jorge-Rodriguez merged commit 2254b29 into ansible-collections:main Mar 16, 2021

ziegenberg deleted the fix-deprecations branch March 16, 2021 10:26

Andersson007 mentioned this pull request Apr 23, 2021

mysql_user module failing with "'database' is an invalid keyword argument for this function" #151

Closed

Andersson007 mentioned this pull request Apr 23, 2021

mysql: revert changes made in PR 116 #153

Merged

mgedmin mentioned this pull request Jun 1, 2021

why login_unix_socket can't use for remote connection? #148

Closed

ziegenberg mentioned this pull request Jun 1, 2021

change deprecated parameter pw and db - 2nd try #176

Closed

ziegenberg mentioned this pull request Jun 1, 2021

change deprecated parameter pw and db #177

Merged

betanummeric mentioned this pull request May 16, 2023

fix connection arguments mysql driver compatability #551

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change deprecated parameter pw and db #116

change deprecated parameter pw and db #116

ziegenberg commented Mar 12, 2021

codecov bot commented Mar 12, 2021 •

edited

Loading

Andersson007 commented Mar 12, 2021 •

edited

Loading

ziegenberg commented Mar 12, 2021

Andersson007 commented Mar 12, 2021

Andersson007 commented Mar 12, 2021

Jorge-Rodriguez commented Mar 15, 2021

ziegenberg commented Mar 15, 2021

Jorge-Rodriguez commented Mar 15, 2021 •

edited

Loading

Andersson007 commented Mar 16, 2021

ziegenberg commented Mar 16, 2021

ziegenberg commented Mar 16, 2021

tschoonj commented Apr 22, 2021

tschoonj commented Apr 22, 2021

ziegenberg commented Apr 22, 2021

Jorge-Rodriguez commented Apr 22, 2021

tschoonj commented Apr 22, 2021

Jorge-Rodriguez commented Apr 22, 2021

awegrzyn commented Apr 22, 2021 •

edited

Loading

Jorge-Rodriguez commented Apr 22, 2021 •

edited

Loading

Andersson007 commented Apr 23, 2021 •

edited

Loading

Andersson007 commented Apr 23, 2021

Jorge-Rodriguez commented Apr 23, 2021

Jorge-Rodriguez commented Apr 23, 2021

ziegenberg commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented May 14, 2021

ziegenberg commented May 14, 2021

Andersson007 commented May 14, 2021

Jorge-Rodriguez commented May 14, 2021

ziegenberg commented Jun 1, 2021

change deprecated parameter pw and db #116

change deprecated parameter pw and db #116

Conversation

ziegenberg commented Mar 12, 2021

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

codecov bot commented Mar 12, 2021 • edited Loading

Codecov Report

Andersson007 commented Mar 12, 2021 • edited Loading

ziegenberg commented Mar 12, 2021

Andersson007 commented Mar 12, 2021

Andersson007 commented Mar 12, 2021

Jorge-Rodriguez commented Mar 15, 2021

ziegenberg commented Mar 15, 2021

Jorge-Rodriguez commented Mar 15, 2021 • edited Loading

Andersson007 commented Mar 16, 2021

ziegenberg commented Mar 16, 2021

ziegenberg commented Mar 16, 2021

tschoonj commented Apr 22, 2021

tschoonj commented Apr 22, 2021

ziegenberg commented Apr 22, 2021

Jorge-Rodriguez commented Apr 22, 2021

tschoonj commented Apr 22, 2021

Jorge-Rodriguez commented Apr 22, 2021

awegrzyn commented Apr 22, 2021 • edited Loading

Jorge-Rodriguez commented Apr 22, 2021 • edited Loading

Andersson007 commented Apr 23, 2021 • edited Loading

Andersson007 commented Apr 23, 2021

Jorge-Rodriguez commented Apr 23, 2021

Jorge-Rodriguez commented Apr 23, 2021

ziegenberg commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented Apr 23, 2021

Andersson007 commented May 14, 2021

ziegenberg commented May 14, 2021

Andersson007 commented May 14, 2021

Jorge-Rodriguez commented May 14, 2021

ziegenberg commented Jun 1, 2021

codecov bot commented Mar 12, 2021 •

edited

Loading

Andersson007 commented Mar 12, 2021 •

edited

Loading

Jorge-Rodriguez commented Mar 15, 2021 •

edited

Loading

awegrzyn commented Apr 22, 2021 •

edited

Loading

Jorge-Rodriguez commented Apr 22, 2021 •

edited

Loading

Andersson007 commented Apr 23, 2021 •

edited

Loading