terraform-provider-postgresql icon indicating copy to clipboard operation
terraform-provider-postgresql copied to clipboard

feat: Add resource for subscriptions

Open nicarl opened this issue 3 years ago • 5 comments

This PR adds the new resource postgresql_publication for managing PostgreSQL publications.

Example:

resource "postgresql_subscription" "subscription" {
  name          = "subscription"
  conninfo      = "host=localhost port=5432 dbname=mydb user=postgres password=secret"
  publications  = ["publication"]
}

Key decisions:

  • I decided to let the user pass conninfo as one string instead of adding fields for host, port, etc.
  • There are additional options when setting up a subscription (https://www.postgresql.org/docs/current/sql-createsubscription.html), I did not add support for those yet to keep this as a small as possbile
  • I could not find a reliable way to update individual parameters - destroy and recreate is therefore preferred
  • For the testing one need to set the create_slot to false as the publisher is on the same database cluster. Otherwise the CREATE statement will hang. This is a known issue (https://www.postgresql.org/docs/current/sql-createsubscription.html)
  • The tests get flaky when not waiting a few seconds after the test. This seems to be related to the replication slots. I choose a pragmatic solution here and added a cool down period of 5 seconds

fix: https://github.com/cyrilgdn/terraform-provider-postgresql/issues/238

nicarl avatar Aug 28 '22 18:08 nicarl

cc @chromko maybe you can have a look at this as you added the publication resource?

nicarl avatar Aug 28 '22 18:08 nicarl

Hey @cyrilgdn I ran the tests successfully on my local machine, I would be curious if they pass in the CI

nicarl avatar Aug 30 '22 16:08 nicarl

@nicarl Thanks for you work on that :+1:

I would be curious if they pass in the CI

Just triggered them!

I'll try to review as soon as possible (hopefully this week-end)

cyrilgdn avatar Aug 31 '22 21:08 cyrilgdn

@nicarl Thanks for you work on that 👍

I would be curious if they pass in the CI

Just triggered them!

I'll try to review as soon as possible (hopefully this week-end)

Hey, did you have time to look at this?

nicarl avatar Sep 06 '22 15:09 nicarl

Hey @cyrilgdn the tests are passing, did you have any time to look at the PR?

nicarl avatar Oct 18 '22 07:10 nicarl

Sorry for the absence of response, I was a bit off the project for a while due to lack of time.

I'll review it so we can include in the next release, could you just merge master to resolve the conflict (you can revert test.yml to the master version)

cyrilgdn avatar Nov 20 '22 13:11 cyrilgdn

This has been released in v1.18.0

cyrilgdn avatar Nov 27 '22 17:11 cyrilgdn

Hey, sorry I did not find time earlier. Thank you for fixing the last details and your work for this provider! :bow:

nicarl avatar Nov 28 '22 19:11 nicarl