From a8ebbecd53bbb28016cc21854bca2eb9a5ef1021 Mon Sep 17 00:00:00 2001 From: Andrey Klychkov Date: Tue, 11 Jun 2019 14:42:53 +0300 Subject: [PATCH] postgresql modules: use postgres.exec_sql instead of __exec_sql methods (#57674) * postgresql modules: use postgres.exec_sql instead of __exec_sql methods * postgresql modules: use exec_sql, added changelog fragment * Update changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml Co-Authored-By: Felix Fontein --- ...odules_use_exec_sql_instead_of_methods.yml | 2 + .../postgresql/postgresql_membership.py | 29 +++------- .../database/postgresql/postgresql_owner.py | 54 +++++------------ .../database/postgresql/postgresql_ping.py | 18 ++---- .../database/postgresql/postgresql_slot.py | 27 +++------ .../database/postgresql/postgresql_table.py | 43 +++++--------- .../postgresql/postgresql_tablespace.py | 58 ++++++------------- 7 files changed, 73 insertions(+), 158 deletions(-) create mode 100644 changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml diff --git a/changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml b/changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml new file mode 100644 index 00000000000..6026933fdc1 --- /dev/null +++ b/changelogs/fragments/57674-postgres_modules_use_exec_sql_instead_of_methods.yml @@ -0,0 +1,2 @@ +bugfixes: + - postgresql modules - use ``module_utils.postgres.exec_sql`` function instead of ``__exec_sql`` method (https://github.com/ansible/ansible/pull/57674) diff --git a/lib/ansible/modules/database/postgresql/postgresql_membership.py b/lib/ansible/modules/database/postgresql/postgresql_membership.py index 0d42ce0f49d..55c1b14468e 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_membership.py +++ b/lib/ansible/modules/database/postgresql/postgresql_membership.py @@ -144,7 +144,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.database import SQLParseError, pg_quote_identifier -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native @@ -173,7 +177,7 @@ class PgMembership(object): query = "GRANT %s TO %s" % ((pg_quote_identifier(group, 'role'), (pg_quote_identifier(role, 'role')))) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) if self.changed: self.granted[group].append(role) @@ -191,7 +195,7 @@ class PgMembership(object): query = "REVOKE %s FROM %s" % ((pg_quote_identifier(group, 'role'), (pg_quote_identifier(role, 'role')))) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) if self.changed: self.revoked[group].append(role) @@ -206,7 +210,7 @@ class PgMembership(object): "FROM pg_catalog.pg_roles r " "WHERE r.rolname = '%s'" % dst_role) - res = self.__exec_sql(query, add_to_executed=False) + res = exec_sql(self, query, add_to_executed=False) membership = [] if res: membership = res[0][0] @@ -252,22 +256,7 @@ class PgMembership(object): self.target_roles = [r for r in self.target_roles if r not in self.non_existent_roles] def __role_exists(self, role): - return self.__exec_sql("SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False) - - def __exec_sql(self, query, ddl=False, add_to_executed=True): - try: - self.cursor.execute(query) - - if add_to_executed: - self.executed_queries.append(query) - - if not ddl: - res = self.cursor.fetchall() - return res - return True - except Exception as e: - self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e))) - return False + return exec_sql(self, "SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False) # =========================================== diff --git a/lib/ansible/modules/database/postgresql/postgresql_owner.py b/lib/ansible/modules/database/postgresql/postgresql_owner.py index 388bbb0ae76..d837b49c978 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_owner.py +++ b/lib/ansible/modules/database/postgresql/postgresql_owner.py @@ -158,7 +158,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.database import SQLParseError, pg_quote_identifier -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native @@ -238,7 +242,7 @@ class PgOwnership(object): query.append('TO %s' % pg_quote_identifier(self.role, 'role')) query = ' '.join(query) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def set_owner(self, obj_type, obj_name): """Change owner of a database object. @@ -326,85 +330,59 @@ class PgOwnership(object): "WHERE matviewname = '%s' " "AND matviewowner = '%s'" % (self.obj_name, self.role)) - return self.__exec_sql(query, add_to_executed=False) + return exec_sql(self, query, add_to_executed=False) def __set_db_owner(self): """Set the database owner.""" query = "ALTER DATABASE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'database'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_func_owner(self): """Set the function owner.""" query = "ALTER FUNCTION %s OWNER TO %s" % (self.obj_name, pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_seq_owner(self): """Set the sequence owner.""" query = "ALTER SEQUENCE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_schema_owner(self): """Set the schema owner.""" query = "ALTER SCHEMA %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'schema'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_table_owner(self): """Set the table owner.""" query = "ALTER TABLE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_tablespace_owner(self): """Set the tablespace owner.""" query = "ALTER TABLESPACE %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'database'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_view_owner(self): """Set the view owner.""" query = "ALTER VIEW %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __set_mat_view_owner(self): """Set the materialized view owner.""" query = "ALTER MATERIALIZED VIEW %s OWNER TO %s" % (pg_quote_identifier(self.obj_name, 'table'), pg_quote_identifier(self.role, 'role')) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __role_exists(self, role): """Return True if role exists, otherwise return Fasle.""" - return self.__exec_sql("SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False) - - def __exec_sql(self, query, ddl=False, add_to_executed=True): - """Execute SQL. - - Return a query result if possible or True/False if ddl=True arg was passed. - It's necessary for statements that don't return any result (like DDL queries). - - Arguments: - query (str): SQL query to execute. - ddl (bool): Must return True or False instead of rows - (typical for DDL queries) (default False). - add_to_executed (bool): Append the query to self.executed_queries attr. - """ - try: - self.cursor.execute(query) - - if add_to_executed: - self.executed_queries.append(query) - - if not ddl: - res = self.cursor.fetchall() - return res - return True - except Exception as e: - self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e))) - return False + return exec_sql(self, "SELECT 1 FROM pg_roles WHERE rolname = '%s'" % role, add_to_executed=False) # =========================================== diff --git a/lib/ansible/modules/database/postgresql/postgresql_ping.py b/lib/ansible/modules/database/postgresql/postgresql_ping.py index 8ce9983c13f..a3ac5aa7d3c 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_ping.py +++ b/lib/ansible/modules/database/postgresql/postgresql_ping.py @@ -80,7 +80,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible.module_utils.database import SQLParseError -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native from ansible.module_utils.six import iteritems @@ -103,7 +107,7 @@ class PgPing(object): def get_pg_version(self): query = "SELECT version()" - raw = self.__exec_sql(query)[0][0] + raw = exec_sql(self, query, add_to_executed=False)[0][0] if raw: self.is_available = True raw = raw.split()[1].split('.') @@ -112,16 +116,6 @@ class PgPing(object): minor=int(raw[1]), ) - def __exec_sql(self, query): - try: - self.cursor.execute(query) - res = self.cursor.fetchall() - if res: - return res - except Exception as e: - self.module.fail_json("Unable to execute '%s': %s" % (query, to_native(e))) - - return False # =========================================== # Module execution. diff --git a/lib/ansible/modules/database/postgresql/postgresql_slot.py b/lib/ansible/modules/database/postgresql/postgresql_slot.py index c0099b6d821..a9c35969868 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_slot.py +++ b/lib/ansible/modules/database/postgresql/postgresql_slot.py @@ -150,7 +150,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.database import SQLParseError -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native @@ -192,37 +196,22 @@ class PgSlot(object): elif kind == 'logical': query = "SELECT pg_create_logical_replication_slot('%s', '%s')" % (self.name, output_plugin) - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def drop(self): if not self.exists: return False query = "SELECT pg_drop_replication_slot('%s')" % self.name - self.changed = self.__exec_sql(query, ddl=True) + self.changed = exec_sql(self, query, ddl=True) def __slot_exists(self): query = "SELECT slot_type FROM pg_replication_slots WHERE slot_name = '%s'" % self.name - res = self.__exec_sql(query, add_to_executed=False) + res = exec_sql(self, query, add_to_executed=False) if res: self.exists = True self.kind = res[0][0] - def __exec_sql(self, query, ddl=False, add_to_executed=True): - try: - self.cursor.execute(query) - - if add_to_executed: - self.executed_queries.append(query) - - if not ddl: - res = self.cursor.fetchall() - return res - return True - except Exception as e: - self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e))) - return False - # =========================================== # Module execution. diff --git a/lib/ansible/modules/database/postgresql/postgresql_table.py b/lib/ansible/modules/database/postgresql/postgresql_table.py index 797c18f5a93..c06f40142c8 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_table.py +++ b/lib/ansible/modules/database/postgresql/postgresql_table.py @@ -237,7 +237,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.database import SQLParseError, pg_quote_identifier -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native @@ -278,7 +282,7 @@ class Table(object): "INNER JOIN pg_namespace AS n ON c.relnamespace = n.oid " "WHERE t.tablename = '%s' " "AND n.nspname = '%s'" % (tblname, schema)) - res = self.__exec_sql(query) + res = exec_sql(self, query, add_to_executed=False) if res: self.exists = True self.info = dict( @@ -354,8 +358,7 @@ class Table(object): if tblspace: query += " TABLESPACE %s" % pg_quote_identifier(tblspace, 'database') - if self.__exec_sql(query, ddl=True): - self.executed_queries.append(query) + if exec_sql(self, query, ddl=True): changed = True if owner: @@ -402,8 +405,7 @@ class Table(object): if tblspace: query += " TABLESPACE %s" % pg_quote_identifier(tblspace, 'database') - if self.__exec_sql(query, ddl=True): - self.executed_queries.append(query) + if exec_sql(self, query, ddl=True): changed = True if owner: @@ -413,20 +415,17 @@ class Table(object): def truncate(self): query = "TRUNCATE TABLE %s" % pg_quote_identifier(self.name, 'table') - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def rename(self, newname): query = "ALTER TABLE %s RENAME TO %s" % (pg_quote_identifier(self.name, 'table'), pg_quote_identifier(newname, 'table')) - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def set_owner(self, username): query = "ALTER TABLE %s OWNER TO %s" % (pg_quote_identifier(self.name, 'table'), pg_quote_identifier(username, 'role')) - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def drop(self, cascade=False): if not self.exists: @@ -435,30 +434,16 @@ class Table(object): query = "DROP TABLE %s" % pg_quote_identifier(self.name, 'table') if cascade: query += " CASCADE" - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def set_tblspace(self, tblspace): query = "ALTER TABLE %s SET TABLESPACE %s" % (pg_quote_identifier(self.name, 'table'), pg_quote_identifier(tblspace, 'database')) - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def set_stor_params(self, params): query = "ALTER TABLE %s SET (%s)" % (pg_quote_identifier(self.name, 'table'), params) - self.executed_queries.append(query) - return self.__exec_sql(query, ddl=True) - - def __exec_sql(self, query, ddl=False): - try: - self.cursor.execute(query) - if not ddl: - res = self.cursor.fetchall() - return res - return True - except Exception as e: - self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e))) - return False + return exec_sql(self, query, ddl=True) # =========================================== diff --git a/lib/ansible/modules/database/postgresql/postgresql_tablespace.py b/lib/ansible/modules/database/postgresql/postgresql_tablespace.py index ef401881aa0..ff093ef0c4d 100644 --- a/lib/ansible/modules/database/postgresql/postgresql_tablespace.py +++ b/lib/ansible/modules/database/postgresql/postgresql_tablespace.py @@ -173,7 +173,11 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.database import SQLParseError, pg_quote_identifier -from ansible.module_utils.postgres import connect_to_db, postgres_common_argument_spec +from ansible.module_utils.postgres import ( + connect_to_db, + exec_sql, + postgres_common_argument_spec, +) from ansible.module_utils._text import to_native @@ -216,14 +220,14 @@ class PgTablespace(object): """Get tablespace information.""" # Check that spcoptions exists: - opt = self.__exec_sql("SELECT 1 FROM information_schema.columns " - "WHERE table_name = 'pg_tablespace' " - "AND column_name = 'spcoptions'", add_to_executed=False) + opt = exec_sql(self, "SELECT 1 FROM information_schema.columns " + "WHERE table_name = 'pg_tablespace' " + "AND column_name = 'spcoptions'", add_to_executed=False) # For 9.1 version and earlier: - location = self.__exec_sql("SELECT 1 FROM information_schema.columns " - "WHERE table_name = 'pg_tablespace' " - "AND column_name = 'spclocation'", add_to_executed=False) + location = exec_sql(self, "SELECT 1 FROM information_schema.columns " + "WHERE table_name = 'pg_tablespace' " + "AND column_name = 'spclocation'", add_to_executed=False) if location: location = 'spclocation' else: @@ -243,7 +247,7 @@ class PgTablespace(object): "ON t.spcowner = r.oid " "WHERE t.spcname = '%s'" % (location, self.name)) - res = self.__exec_sql(query, add_to_executed=False) + res = exec_sql(self, query, add_to_executed=False) if not res: self.exists = False @@ -273,7 +277,7 @@ class PgTablespace(object): """ query = ("CREATE TABLESPACE %s LOCATION '%s'" % (pg_quote_identifier(self.name, 'database'), location)) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def drop(self): """Drop tablespace. @@ -281,7 +285,7 @@ class PgTablespace(object): Return True if success, otherwise, return False. """ - return self.__exec_sql("DROP TABLESPACE %s" % pg_quote_identifier(self.name, 'database'), ddl=True) + return exec_sql(self, "DROP TABLESPACE %s" % pg_quote_identifier(self.name, 'database'), ddl=True) def set_owner(self, new_owner): """Set tablespace owner. @@ -296,7 +300,7 @@ class PgTablespace(object): return False query = "ALTER TABLESPACE %s OWNER TO %s" % (pg_quote_identifier(self.name, 'database'), new_owner) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def rename(self, newname): """Rename tablespace. @@ -309,7 +313,7 @@ class PgTablespace(object): query = "ALTER TABLESPACE %s RENAME TO %s" % (pg_quote_identifier(self.name, 'database'), newname) self.new_name = newname - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def set_settings(self, new_settings): """Set tablespace settings (options). @@ -349,7 +353,7 @@ class PgTablespace(object): """ query = "ALTER TABLESPACE %s RESET (%s)" % (pg_quote_identifier(self.name, 'database'), setting) - return self.__exec_sql(query, ddl=True) + return exec_sql(self, query, ddl=True) def __set_setting(self, setting): """Set tablespace setting. @@ -361,33 +365,7 @@ class PgTablespace(object): """ query = "ALTER TABLESPACE %s SET (%s)" % (pg_quote_identifier(self.name, 'database'), setting) - return self.__exec_sql(query, ddl=True) - - def __exec_sql(self, query, ddl=False, add_to_executed=True): - """Execute SQL query. - - Return a query result if possible or True/False if ddl=True arg was passed. - It's necessary for statements that don't return any result (like DDL queries). - - args: - query (str) -- SQL query to execute - ddl (bool) -- must return True or False instead of rows (typical for DDL queries) - add_to_executed (bool) -- append the query to self.executed_queries list - """ - - try: - self.cursor.execute(query) - - if add_to_executed: - self.executed_queries.append(query) - - if not ddl: - res = self.cursor.fetchall() - return res - return True - except Exception as e: - self.module.fail_json(msg="Cannot execute SQL '%s': %s" % (query, to_native(e))) - return False + return exec_sql(self, query, ddl=True) # ===========================================