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

Fixed integration tests #462

Merged
merged 1 commit into from
May 9, 2023
Merged

Fixed integration tests #462

merged 1 commit into from
May 9, 2023

Conversation

aleksvagachev
Copy link
Collaborator

SUMMARY

Due to a change in the receipt of connection parameters in postgresql_privs ( #451 ), fixed the integration tests.

@Andersson007
Copy link
Collaborator

Andersson007 commented May 5, 2023

  1. Looks like breaking changes were introduced by Fix connect_params being ignored on postgresql_privs #451 to the module's interface. Can users' playbook break the same way after users upgrade Ansible?
  2. Also interesting why we didn't get the errors in the original PR...
  3. The collection was released yesterday, should a quick fix be submitted to make the stuff working as it did?

cc @JohnAtOlo

@Andersson007
Copy link
Collaborator

Andersson007 commented May 5, 2023

@aleksvagachev as #463 seems solving the issue (thanks for spotting the cause!) I think you could cancel the changes in tests here and focus on tidying up the function like removing that param_dict and i think maybe those checks can be also replaced with functions from module_utils.postgres or removed

@Andersson007
Copy link
Collaborator

Andersson007 commented May 5, 2023

@aleksvagachev @hunleyd thanks! UPDATE: ah, i mixed up folks, sorry, @aleksvagachev anyway #462 (comment) is still relevant

Copy link
Contributor

@jchancojr jchancojr left a comment

Choose a reason for hiding this comment

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

lgtm

@Andersson007 Andersson007 merged commit 8dbc629 into ansible-collections:main May 9, 2023
@Andersson007
Copy link
Collaborator

cool, let's merge it, thanks everyone:) the function can still be improved more as an idea for further contributions.

@Andersson007
Copy link
Collaborator

@aleksvagachev once you're back from PTO, could you please backport this PR manually?

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.

4 participants