-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Conversation
Thanks @bmildren for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion. [This message brought to you by your friendly Ansibull-bot.] |
shipit Thanks |
🐑 it! |
I've produced a basic example below of how the modules can be used based on the "Mini HOW TO on ProxySQL Configuration" in the proxysql docs. The example supplies the default admin:admin creds, however a config file containing the username and password can also be used.
|
Nice Ben. Ship it |
shipit This is great - it will be handy. |
+1 |
Some comments.. they'll make better code but they don't strictly fix any errors:
from ansible.module_utils.pycompat24 import get_exception
[...]
except MySQLdb.Error:
e = get_exception() and one that may be more serious:
You can see it in action in code like this: https://github.com/ansible/ansible-modules-core/blob/devel/database/mysql/mysql_db.py#L116 |
needs_revision |
Thanks @bmildren for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
Thanks for the previous review! As suggested I've moved the module_utils imports up with the other imports, changed them to not be wildcard imports, and I've switched to using the get_exception function. With regards to sql injection, I've reviewed the sql calls, and the use of cursor.execute(query_string, query_data) syntax escapes the strings so would mitigate against this, the only places this syntax isn't used is in proxysql_manage_config, however here all the user inputs all come from predefined choices. Thanks! |
ready_for_review |
This reverts commit 9d51f82. proxysql is breaking docs build. Reverting until those are fixed
@bmildren This was breaking documentation building so we had to revert it. Take a look at 6e4a182 for an example of how to clean up the module docs. You can run ansible-doc MODULE_NAME on each of the proxy_sql modules to check that the docs are parsing okay. Once that's done resubmit and we can try again. (Feel free to ping me on irc abadger1999 in #ansible-devel on ic.freenode.net) if you need to get it looked at once you've resubmitted. |
ISSUE TYPE
COMPONENT NAME
database/proxysql/proxysql_backend_servers.py
database/proxysql/proxysql_global_variables.py
database/proxysql/proxysql_manage_config.py
database/proxysql/proxysql_mysql_users.py
database/proxysql/proxysql_query_rules.py
database/proxysql/proxysql_replication_hostgroups.py
database/proxysql/proxysql_scheduler.py
ANSIBLE VERSION
SUMMARY