mysql_user: refactor to reduce execute() calls

[steveteahan]

SUMMARY

This module does not currently log the SQL statements that it executes.
A change was proposed to add this functionality, but it would require
modifications in many sections of the code due to how many cursor.execute()
statements there currently are. This change simply consolidates the
number of execute() calls where it is trivial to do so.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

mysql_user

ADDITIONAL INFORMATION

Without this change, logging the queries in my change proposed in #75 would either look like:

# repeating the query text and prone to error on changes
log_query(*mogrify("CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password), tls_requires))
cursor.execute(*mogrify("CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password), tls_requires))

or

# avoids repeating the query, but turns 1 line into 3 everywhere where the query will be logged (7 times in `user_add` alone)
query_with_args = mogrify("CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password), tls_requires)
log_query(*query_with_args)
cursor.execute(*query_with_args)

Overall, even if I were to create a execute_and_log method, the proposed changes would still be an improvement in readability.

[codecov]

Codecov Report

Merging #76 (389eef0) into main (0690771) will increase coverage by 0.06%.
The diff coverage is 95.00%.

@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
+ Coverage   75.01%   75.07%   +0.06%     
==========================================
  Files          12       12              
  Lines        1641     1645       +4     
  Branches      423      423              
==========================================
+ Hits         1231     1235       +4     
  Misses        267      267              
  Partials      143      143              

Impacted Files	Coverage Δ
plugins/modules/mysql_user.py	78.57% <95.00%> (+0.18%)	⬆️

---

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 0690771...389eef0. Read the comment docs.

[steveteahan]

It doesn't look like the plugin parameters had any testing. The encrypted parameter test was dropped here: https://github.com/ansible/ansible/pull/25825/files

I'm assuming that I'll hit this issue in #75 even without refactoring due to the number of plugin-related branches. I'm not very familiar with other auth plugins, but I can take a look at what it'd take to fix the encrypted test and add a tests for plugins.

[Andersson007]

from my personal experience: we should cover all changed parts:), i.e. to keep CI green

[steveteahan]

I believe this should be unblocked as soon as #79 is merged. That PR adds test coverage that was missing for the majority of password and plugin authentication paths.

[Andersson007]

@steveteahan could you please rebase the branch and fix the conflicts

[Andersson007]

@steveteahan if the tests get green, will try to review the PR tomorrow (if nothing super urgent on my day work), thanks for working on this

[Andersson007]

@bmalynovytch what do you think? It's a kinda preparation for #75

[bmalynovytch]

@bmalynovytch what do you think? It's a kinda preparation for #75

Looks good to me, although I didn't test it.

[Andersson007]

@steveteahan thanks for the refactoring!
@bmalynovytch thanks for reviewing! Now i can be sure we will fix hypothetical consequences together;)
Merged