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

Better handling of extension versions #163

Merged
merged 21 commits into from
Nov 24, 2021

Conversation

keithf4
Copy link
Contributor

@keithf4 keithf4 commented Nov 15, 2021

SUMMARY

This PR is a bugfix and enhancement for better handling PostgreSQL extensions. The issues surrounding the current handling of extension versions are outlined in the related Issue #129.

Tried to keep it so that there is no change to the current interface or options that the user has to worry about with this update. Only change that the user may see is that a version of "latest" will now always ensure the latest version of an extension is installed whenever the play is run. It did not always do that in all cases before.

The module no longer cares about the actual "version number" of any extension since an extension version is a completely arbitrary string value. What determines which version is "greater" or "less than" another is simply the update paths that the extension author provides, so all version comparison operations have been removed.

For extension updates, this changes the handling of them to simply check if there is a valid update path between the currently installed version and the given target version. This allows supporting of "downgrades" as well if the extension author provides such a method.

Giving a value of "latest" for the extension version works pretty much the same as before, but now it will always attempt to perform an extension update if that value is given. This better supports major version upgrades of PostgreSQL to also include updates to extensions as well if a newer version of the extension is available at that time. For example, when upgrading to PG 13, the version of pg_stat_statements changed to 1.8 to add new columns. If the user had the version of "latest" in their ansible inventory, it would upgrade pg_stat_statements as well as PG if making use of this module.

I'm still not quite familiar with how to get the testing environment for ansible modules set up yet, so I haven't provided updated tests for these changes. I'm attempting to get it figured out, but if someone else could assist with that, that would be great.

I've so far tested these changes with ansible 2.9 and 2.11 myself either by updating the python module manually or by using a custom collections path to the playbook.

Fixes: #129

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

postgresql_ext

@tcraxs
Copy link
Contributor

tcraxs commented Nov 17, 2021

@keithf4 One of the extensions checks failed: see here

It looks like that the assertion is not equal to your new error message (here) ... and this is your change which triggers that: line 423

You have to change this line

@keithf4
Copy link
Contributor Author

keithf4 commented Nov 17, 2021

@keithf4 One of the extensions checks failed: see here

It looks like that the assertion is not equal to your new error message (here) ... and this is your change which triggers that: line 423

You have to change this line

Yes, I did catch that. I have that fixed and am working on some additional python doc changes for the functions and will have that pushed up shortly. Thank you!

@keithf4
Copy link
Contributor Author

keithf4 commented Nov 17, 2021

I believe I have this PR complete now if anyone else is able to review.

At the time I made it I didn't realize the testing would be done automatically, so took advantage of that to get things fixed and all tests to pass. I believe the existing tests cover all new function usage, so I don't think there's any need for additional tests at this time.

Only lingering concern I had was I had attempted to use the psycopg2.sql library to write "safe" sql statements that involve variable identity objects (tables, schemas, etc). However these were failing on CentOS7/U20 since it seems they have psycopg 2.5 by default and that library wasn't added until 2.7. So something to keep in mind for future updates as older OS support is dropped.

https://www.psycopg.org/docs/sql.html

Copy link
Contributor

@tcraxs tcraxs left a comment

Choose a reason for hiding this comment

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

@keithf4 Thanks for updating this. LGTM

@keithf4
Copy link
Contributor Author

keithf4 commented Nov 19, 2021

Reviewing code again, realized I'd created a code path that would never be hit. Fixed!

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.

@keithf4 thanks for the great improvement! @tcraxs @hunleyd thanks for reviewing!
There are a couple of minor things from me.

plugins/modules/postgresql_ext.py Outdated Show resolved Hide resolved
plugins/modules/postgresql_ext.py Outdated Show resolved Hide resolved
plugins/modules/postgresql_ext.py Outdated Show resolved Hide resolved
plugins/modules/postgresql_ext.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
@Andersson007 Andersson007 merged commit 8eaf1af into ansible-collections:main Nov 24, 2021
@Andersson007
Copy link
Collaborator

@keithf4 thanks for the great improvement! Would be cool to see more contributions from you!
@tcraxs @hunleyd thanks for reviewing!

I'll try to release the collection today

@Andersson007 Andersson007 mentioned this pull request Nov 24, 2021
@Andersson007
Copy link
Collaborator

Done, 1.6.0. Available via ansible-galaxy now and will be included in the Anbsible package with the next Ansible's patch / minor release.

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.

Better handling of PostgreSQL extension versioning
4 participants