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

Add support for partitions #391

Closed

Conversation

SiL3x
Copy link

@SiL3x SiL3x commented Jan 10, 2023

SUMMARY

Add the option to create partitioned tables as described in #372

Fixes #372

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

postgresql_table

ADDITIONAL INFORMATION

Copy link
Collaborator

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SiL3x thanks for the PR!

Could you please:

  • Add an example to the EXAMPLES block
  • Add a changelog fragment
  • Specify in the doc that they are mutually required.
  • Specify in the AnsibleModule() invocation that they are required_together

Thanks:)

Comment on lines +130 to +132
### Comment: This block caused the tests to crash

#- name: Generate locales (Debian)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Comment: This block caused the tests to crash
#- name: Generate locales (Debian)
### Comment: This block caused the tests to crash
#- name: Generate locales (Debian)

Do you run them with --docker ?, e.g:

ansible-test integration <target_name> --docker ubuntu2004 -vvv > ~/test.log

i run the tests today and they were green

Copy link
Author

@SiL3x SiL3x Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran them exactly like that and they crashed with:

ERROR! couldn't resolve module/action 'locale_gen'. This often indicates a misspelling, missing collection, or incorrect module path.

The error appears to be in '/root/ansible_collections/community/postgresql/tests/output/.tmp/integration/postgresql_table-l3c1vxf2-ÅÑŚÌβŁÈ/tests/integration/targets/setup_postgresql_db/tasks/main.yml': line 132, column 3, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:


- name: Generate locales (Debian)
  ^ here

Also apparently nothing breaks when the block is out commented.

test.log

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SiL3x i've just checked out your branch, uncommented this block and run the tests locally:

$ ansible-test integration postgresql_table --docker ubuntu2004 -vvv > ~/test.log
WARNING: A Python 3.8 interpreter was found at "/usr/bin/python3.8", yet a matching pip was not found.
[WARNING]: running playbook inside collection community.postgresql
[WARNING]: Collection community.general does not support Ansible version 2.10.6
[WARNING]: Updating cache and auto-installing missing dependency: python3-apt
[WARNING]: Consider using the get_url or uri module rather than running 'wget'.
If you need to use command because get_url or uri is insufficient you can add
'warn: false' to this command task or set 'command_warnings=False' in
ansible.cfg to get rid of this message.
[WARNING]: Module remote_tmp /var/lib/postgresql/.ansible/tmp did not exist and
was created with a mode of 0700, this may cause issues when running as another
user. To avoid this, create the remote_tmp dir with the correct permissions
manually
[WARNING]: The value "True" (type bool) was converted to "'True'" (type
string). If this does not look like what you expect, quote the entire value to
ensure it does not change.
WARNING: Reviewing previous 1 warning(s):
WARNING: A Python 3.8 interpreter was found at "/usr/bin/python3.8", yet a matching pip was not found.

They pass.
Could you please uncomment the block and push? Let's see what our CI will say.

plugins/modules/postgresql_table.py Outdated Show resolved Hide resolved
plugins/modules/postgresql_table.py Outdated Show resolved Hide resolved
@Andersson007
Copy link
Collaborator

Andersson007 commented Jan 10, 2023

also please add yourself to the authors: doc field (if you're ok with it of course)
let us know please when the PR is ready for the second round of review

@Andersson007
Copy link
Collaborator

Fixes #372

This ^ statement will trigger GH to close the issue automatically, when the PR gets merged

Comment on lines +1 to +2
[flake8]
max-line-length = 160
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[flake8]
max-line-length = 160
[flake8]
max-line-length = 160

I think just moving a part of long line to another line is good.
ansible-test sanity intentionally sets some limitations for readability

@Andersson007
Copy link
Collaborator

@SiL3x hello, any updates on the PR?

@Andersson007
Copy link
Collaborator

closing to keep the tracker clean, thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for partitions to postgresql_table
2 participants