Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New module postgresql_sequence #55027

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@tcraxs
Copy link
Contributor

commented Apr 9, 2019

SUMMARY

Add or remove PostgreSQL sequence from remote hosts.

ISSUE TYPE
  • New Module Pull Request
MAIN OPTIONS
  • sequence: The name (optionally schema-qualified) of the sequence.
  • state: absent | present
  • data_type: Specifies the data type of the sequence. Valid types are smallint, integer, and bigint. bigint is the default. The data type determines the default minimum and maximum values of the sequence. Supported from PostgreSQL 10.
  • increment: Increment specifies which value is added to the current sequence value to create a new value.
  • minvalue: Minvalue determines the minimum value a sequence can generate.
  • maxvalue: Maxvalue determines the maximum value for the sequence.
  • start: Start allows the sequence to begin anywhere.
  • cache: Cache specifies how many sequence numbers are to be preallocated and stored in memory for faster access.
  • cycle: The cycle option allows the sequence to wrap around when the maxvalue or minvalue has been reached by an ascending or descending sequence respectively.
  • cascade: Automatically drop objects that depend on the sequence, and in turn all objects that depend on those objects.
  • restrict: Refuse to drop the sequence if any objects depend on it. This is the default.
  • rename: The new name for the sequence.
  • schema: The schema in the new sequence will be created.
  • newschema: The new schema for the sequence.
  • session_role: Switch to session_role after connecting.
    ...
RETURN
state:
  description: Sequence state at the end of execution.
  returned: always
  type: str
  sample: 'present'
sequence:
  description: Sequence name.
  returned: always
  type: str
  sample: 'foobar'
queries:
    description: List of queries that was tried to be executed.
    returned: always
    type: str
    sample: [ "CREATE SEQUENCE \"foo\"" ]
schema:
    description: Name of the schema of the sequence
    returned: always
    type: str
    sample: 'foo'
data_type:
    description:
    returned: always
    samlpe: 'bigint'
increment:
    description: The value of increment of the sequence. A positive value will make
                 an ascending sequence, a negative one a descending sequence.
    returned: always
    samlpe: '-1'
minvalue:
    description: The value of minvalue of the sequence.
    returned: always
    samlpe: '1'
maxvalue:
    description: The value of maxvalue of the sequence.
    returned: always
    samlpe: '9223372036854775807'
start:
    description: The value of start of the sequence.
    returned: always
    samlpe: '12'
cycle:
    description: Shows if the sequence cycle or not.
    returned: always
    samlpe: 'NO'
owner:
    description: Shows the current owner of the sequence.
    returned: always
    samlpe: 'postgres'
newname:
    description: Shows the new sequence name after rename.
    returned: on success
    samlpe: 'barfoo'
newschema:
    description: Shows the new schema of the sequence after schema change.
    returned: on success
    samlpe: 'foobar'
OUTPUT EXAMPLE

Task:

- name: postgresql_sequence - create an descending sequence called foobar_desc, starting at 101 and which cycle between 1 to 1000
  postgresql_sequence:
    name: foobar_desc
    increment: -1
    start: 101
    minvalue: 1
    maxvalue: 1000
    cycle: yes

Output:

changed: [testhost] => {
    "changed": true,
    "cycle": "YES",
    "data_type": "bigint",
    "increment": "-1",
    "invocation": {
        "module_args": {
            "ca_cert": null,
            "cache": null,
            "cascade": null,
            "cycle": true,
            "data_type": null,
            "db": "ansible_db",
            "increment": -1,
            "login_host": "",
            "login_password": "",
            "login_unix_socket": "",
            "login_user": "postgres",
            "maxvalue": 1000,
            "minvalue": 1,
            "name": "foobar_desc",
            "newschema": null,
            "owner": null,
            "port": 5432,
            "rename_to": null,
            "restrict": null,
            "schema": null,
            "sequence": "foobar_desc",
            "session_role": null,
            "ssl_mode": "prefer",
            "start": 101,
            "state": "present"
        }
    },
    "maxvalue": "1000",
    "minvalue": "1",
    "owner": "postgres",
    "queries": [
        "CREATE SEQUENCE \"foobar_desc\" INCREMENT BY -1 MINVALUE 1 MAXVALUE 1000 START WITH 101 CYCLE"
    ],
    "schema": "public",
    "sequence": "foobar_desc",
    "start": "101",
    "state": "present"
}
EXAMPLES
- name: Create an ascending sequence called foobar
  postgresql_sequence:
    name: foobar

- name: Create an ascending sequence called foobar, starting at 101
  postgresql_sequence:
    name: foobar
    start: 101

- name: Create an descending sequence called foobar, starting at 101
  postgresql_sequence:
    name: foobar
    increment: -1
    start: 101

- name: Create an ascending sequence called foobar, which cycle between 1 to 10
  postgresql_sequence:
    name: foobar
    cycle: yes
    min: 1
    max: 10

- name: Rename an existing sequence named foo to bar
  postgresql_sequence:
    name: foo
    rename_to: bar

- name: Change the schema of an existing sequence to foobar
  postgresql_sequence:
    name: foobar
    newschema: foobar

- name: Drop a sequence called foobar
  postgresql_sequence:
    name: foobar
    state: absent

@tcraxs tcraxs marked this pull request as ready for review Apr 9, 2019

@tcraxs

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

+label postgresql
+label database

@Andersson007

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

Looks solid, good job! I need time to look at it in detail. Hope it will soon.
Just a couple of remarks for a while.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

The test ansible-test sanity --test pep8 [explain] failed with 21 errors:

lib/ansible/modules/database/postgresql/postgresql_sequence.py:320:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/database/postgresql/postgresql_sequence.py:371:21: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:413:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:414:17: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:415:17: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:421:33: E128 continuation line under-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:449:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:453:33: E128 continuation line under-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:465:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:469:33: E128 continuation line under-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:478:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:482:33: E128 continuation line under-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:491:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:495:33: E128 continuation line under-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:504:5: E303 too many blank lines (2)
lib/ansible/modules/database/postgresql/postgresql_sequence.py:531:79: E261 at least two spaces before inline comment
lib/ansible/modules/database/postgresql/postgresql_sequence.py:560:43: E261 at least two spaces before inline comment
lib/ansible/modules/database/postgresql/postgresql_sequence.py:582:31: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:670:41: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:671:41: E127 continuation line over-indented for visual indent
lib/ansible/modules/database/postgresql/postgresql_sequence.py:747:1: E305 expected 2 blank lines after class or function definition, found 1

click here for bot help

@Andersson007

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

use for the local sanity checks

source ./hacking/env-setup

ansible-test sanity --python 2.7 --base-branch devel lib/ansible/modules/database/postgresql/postgresql_sequence.py

@ansibot ansibot removed the needs_triage label Apr 9, 2019

@Andersson007

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

@tcraxs , try to close and reopen it
maybe it's a temporary problem that doesn't depend on your module

@tcraxs tcraxs closed this Apr 9, 2019

@tcraxs tcraxs reopened this Apr 9, 2019

@tcraxs

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

ready_for_review

@tcraxs

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

!needs_revision

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

@Andersson007 @Dorn- @andytom @antoinell @archf @b6d @dschep @jbscalia @jensdepuydt @kostiantyn-nemchenko @kustodian @matburt @nerzhul @sebasmannem @wrouesnel

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py
rename_to:
description:
- The new name for the I(sequence).
- Works only for existing sequences.

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 10, 2019

Contributor

does it lock depending objects? If yes, we should point this out

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 10, 2019

Contributor

maybe also better to link to "alter sequence" documentation

This comment has been minimized.

Copy link
@tcraxs

tcraxs Apr 12, 2019

Author Contributor

does it lock depending objects? If yes, we should point this out

good question, I don't know ATM

Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py Outdated
Show resolved Hide resolved lib/ansible/modules/database/postgresql/postgresql_sequence.py
@Andersson007

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2019

@tcraxs , thank you for the PR! According to my requested changes, the code probably will be changed, so i'll look at the code logic after the fixes. Thank you!

@tcraxs

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@tcraxs , thank you for the PR! According to my requested changes, the code probably will be changed, so i'll look at the code logic after the fixes. Thank you!

Thanks for your comments. I will make some changes and add more examples and tests :)

@tcraxs tcraxs force-pushed the tcraxs:postgresql_sequence branch from 92bc028 to db46ce3 Apr 12, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

@tcraxs this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

@tcraxs This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.
  • More than 50 commits.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

tbirkefeld added some commits Apr 8, 2019

add new examples and remove restrict option
* remove restrict option
* remove `required: false`
* add links to documentation
* change some minor parts in documentation
* add new integration tests for
** drop cascade
** change owner

@tcraxs tcraxs force-pushed the tcraxs:postgresql_sequence branch from 605455b to 06dad68 Apr 17, 2019

@tcraxs tcraxs closed this Apr 17, 2019

@tcraxs tcraxs reopened this Apr 17, 2019

requirements: [ psycopg2 ]
author:
- Tobias Birkefeld (@tcraxs)
'''

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

Please, remove lines 131-172 (and maybe something from "notes" too, see the content of the doc fragment file).

Suggested change
'''
extends_documentation_fragment: postgres
'''

See https://github.com/ansible/ansible/pull/55363/files.
It makes the documentation section more compact.
I didn't know about this before, gundalow told me to do this recently.


if res[0][0]:
self.exists = True
self.sequence_schema = res[0][1]

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

FYI, I did it in the same way by index number before.
However, I've known a way to do it better by Dict cursor. See #55434.
It's just FYI, not need to change anything now.


def get_info(self):
"""Getter to refresh and get sequence info"""
query = ("SELECT * FROM information_schema.sequences "

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

You could get this info and owner info by one SQL.

query = "CREATE SEQUENCE"

if schema:
query += " %s.%s" % (pg_quote_identifier(schema, 'schema'),

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

FYI: I found using query_fragments.append('query_part') and eventually query = ' '.join(query_fragments) more elegant and efficient. (IMO, not need to change, just think about it)

# Drop non-existing sequence
elif not sequence_obj.exists and state == 'absent':
# Nothing to do
module.fail_json(msg="Tries to drop nonexistent sequence '%s'" % sequence)

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

Maybe warn and param fail_on_seq=yes|no? No sequence, no problem
I slept very bad today but seems it's quite flexible ;)


####################
# Test: change owner of a sequence
- name: postgresql_sequence - change owner of an existing sequence from "{{ pg_user }}" to "{{ db_user1 }}"

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

check_mode

columns:
- f1 INT NOT NULL DEFAULT nextval('seq1')

- name: postgresql_sequence - drop with cascade a sequence called seq1

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

check_mode

- A positive value will make an ascending sequence, a negative one a
descending sequence. The default value is 1.
type: int
minvalue:

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

if I use min, max as in examples, I catch

    "invocation": {
        "module_args": {
            "cycle": true, 
            "data_type": "integer", 
            "db": "acme", 
            "login_db": "acme", 
            "max": 10, 
            "min": 2, 
            "name": "foobar2", 
            "sequence": "foobar2"
        }
    }, 
    "msg": "Unsupported parameters for (postgresql_sequence) module: max, min Supported parameters include: ca_cert, cache, cascade, cycle, data_type, db, increment, login_host, login_password, login_unix_socket, login_user, maxvalue, minvalue, newschema, owner, port, rename_to, schema, sequence, session_role, ssl_mode, start, state"
}

add aliases, please

- name: Drop a sequence called foobar with cascade
postgresql_sequence:
name: foobar
cascade: yes

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

mark that cascade: yes is ignored if state=present

- name: Drop a sequence called foobar
postgresql_sequence:
name: foobar
state: absent

This comment has been minimized.

Copy link
@Andersson007

Andersson007 Apr 18, 2019

Contributor

If I try to drop non existent sequence it return

"msg": "Tries to drop nonexistent sequence 'foobar2'"

but should return changed=false
Please, fix it and add tests for it

@Andersson007

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

and, please, use connect_to_db in your module #55514

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.