fix: Cast connparams to allow idempotence (#280)

[jegj]

SUMMARY

Fixes #280. postgresql_subscription module is not idempotent because the port is cast to int in the following chunk of code( 335):

                try:
                    self.attrs['conninfo'][tmp[0]] = int(tmp[1])
                except ValueError:
                    self.attrs['conninfo'][tmp[0]] = tmp[1]

On line 385 the if condition always return false because variable connparams is a dict of string

            if connparams != self.attrs['conninfo']:
                changed = self.__set_conn_params(convert_conn_params(connparams),
                                                 check_mode=check_mode)

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

postgresql_subscription

ADDITIONAL INFORMATION

[hunleyd]

thank you very much for the PR @jegj . Can you fix the failing tests and include a changelog fragment?

[Andersson007]

@jegj thanks for the fix!
i'm not sure if the patch is safe
Look (this is from our CI):

01:31    "invocation": {
01:31             "connparams": {
01:31                 "dbname": "acme_db",
01:31                 "host": "127.0.0.1",
01:31                 "password": "alsdjfKJKDf1#",
01:31                 "port": 5433,
01:31                 "user": "logical_replication"
01:31             },

The port can be passed as int. How does you playbook look, do you pass it as "5433" probably?
Then yeah it's a bug (which appear when we pass it as string) but if we merge the PR, it'll break things for users who pass the port as int.
In this case we should fix it by doing the same for connparams as we do for self.attrs['conninfo'] (with that try-except).
I suggest adding a function which will do it for connparams.
What do you think?
p.s. I'm not 100% sure my logic is correct

[jegj]

@Andersson007 I pass the port as int. I tried with string, same problem. But you are right, that is a better solution. I will add that block code

[Andersson007]

@Andersson007 I pass the port as int. I tried with string, same problem. But you are right, that is a better solution. I will add that block code

@jegj strange, i should check it, maybe it's converted to string internally somehow but ansible output shows as it's int..

[jegj]

@Andersson007 I have added the function but still getting errors on CI. I will take a deep look and see what's the problem

[Andersson007]

@jegj just a fyi, you can run the integration tests locally before pushing, see https://docs.ansible.com/ansible/devel/community/create_pr_quick_start.html. It'll save a bit of time as it's faster than waiting for results here

[Andersson007]

@jegj could you also please add a changelog fragment as @hunleyd mentioned?

[jegj]

@Andersson007 thanks for the info. Yes! I will add the changelog fragment shortly. Thanks all for the help, first time collaborating on an ansible collection. I should have read the documentation better

[patchback]

Backport to stable-1: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-1/cf4eee27bd91c24f675d024d4db6bf032ce245cd/pr-285

Backported as #289

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[Andersson007]

@jegj thanks very much for the contribution! We'll be happy to see more by you and help whenever needed!

[jegj]

Happy to help!