search

ansible-collections / community.postgresql

Manage PostgreSQL with Ansible
https://galaxy.ansible.com/ui/repo/published/community/postgresql/
Other
111 stars 90 forks source link

fix(postgresql_info): use a server version check that handles beta and rc releases #733

Closed hunleyd closed 2 months ago

hunleyd commented 2 months ago
SUMMARY

Convert postgresql_info to using current_setting('server_version_num') as suggested by @keithf4 . This makes the version check work regardless if this is an actual release (17.0), a beta (17beta3), or an rc (17rc1) since the value for all is 170000.

Fixes #728

ISSUE TYPE
COMPONENT NAME

postgresql_info

ADDITIONAL INFORMATION
hunleyd commented 2 months ago

@Andersson007 this works and passes the tests. however, it's now PG10+. If you're cool with that, feel free to merge. If not, we can probably rework the logic to continue to support PG < 10 but it didn't seem worth it since the PG community only supports 12-17 as of the PG release at the end of this month

if you do merge this, we would want to make a release very soon since PG 17 is coming out any day now.

Andersson007 commented 2 months ago

@hunleyd i think this brings a breaking change. E.g. raw will change, correct? How about continuing using query = "SELECT version()" at least for raw? i.e. we'll execute both the queries to preserve full backwards compatibility? On supporting 10: let's not worry about it. On the other hand, it could be handled with just try: except: but proceed with it as you prefer, feel free to skip

cameronmurdoch commented 2 months ago

If support for < PG10 is getting dropped, (which I think it a good idea :slightly_smiling_face: ), can't all references to patch be removed as it will never get used?

Andersson007 commented 2 months ago

If support for < PG10 is getting dropped, (which I think it a good idea 🙂 ), can't all references to patch be removed as it will never get used?

@cameronmurdoch thanks for the question. we use SemVer and removing any ret values would a breaking change, so to do such things we must:

  1. Add a warning when something deprecated is used (not applicable in this case)
  2. Announce an upcoming breaking change in a minor release, update the docs
  3. Plan it for the next (the one after the next if not enough time) major release

Anything that can break backwards compatibility should go through this process ^. Anyway, this can be done in future if the community supports it. Feel free to file an issue

hunleyd commented 2 months ago

I think I handled all the things and this should be workign again for PG < 10 and raw should be as it was but the actual logic uses the new values so beta/rc builds are handled. Please give it a twice-over @Andersson007 @cameronmurdoch

If you're happy with it @Andersson007 feel free to merge. I'm not gonna be around much longer today or much this weekend (my eldest is flying into town)

Andersson007 commented 2 months ago

@hunleyd thanks a lot for fixing the module! @cameronmurdoch thanks for reviewing! I'm gonna release the collection right now

Andersson007 commented 2 months ago

The community.postgresql collection version 3.6.0 has been released! Among other changes, it contains fixes for some modules to work with PostgreSQL 17 beta. Thanks to everyone involved!