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

Update postgresql_pg_hba.py #1124

Merged
merged 3 commits into from
Oct 21, 2020
Merged

Conversation

sebasmannem
Copy link
Contributor

Fixes #1108

SUMMARY

Fixes #1108

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

postgresql_pg_hba.py

ADDITIONAL INFORMATION

The module was accessing dict values using square brackets, and apparently in this case, the old rule did not have 'options' set, and new one has.
Using the .get method is safer (returns None if key not set).
With this change, if oldrule does not have options set, and rule does(or the other way around), a PgHbaRuleChanged is raised, which will replace the old rule for the new, which is as expected.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Please add a changelog fragment.

@Andersson007
Copy link
Contributor

@sebasmannem hi, thanks for the quick fix!
In addition to the changelog fragment @felixfontein mentioned, would be also nice to cover the case in tests, is this possible?

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review database module module new_contributor Help guide this first time contributor owner_pr PR created by owner/maintainer plugins plugin (any type) postgresql small_patch Hopefully easy to review labels Oct 20, 2020
@sebasmannem
Copy link
Contributor Author

I will add a changelog fragment and will look at a unit test.
But that needs a bit more time, so I need to see when I can pan that out.

@Andersson007
Copy link
Contributor

@sebasmannem can't we just add the options case to the existing CI tests?

@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/1124-pg_hba-dictkey bugfix:0:0: extension must be one of: .yml, .yaml

The test ansible-test sanity --test changelog [explain] failed with 1 error:

changelogs/fragments/1124-pg_hba-dictkey bugfix:0:0: extension must be one of: .yml, .yaml

click here for bot help

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Oct 20, 2020
@Andersson007
Copy link
Contributor

@sebasmannem please add .yml extension to the changelog fragment

@Andersson007
Copy link
Contributor

I think the pr is ok without tests but if it's easy to add the case to the existing tests, it would be good

@ansibullbot ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Oct 21, 2020
@sebasmannem
Copy link
Contributor Author

I think I have a unit test. I will push it in first, to see it fail.
After that I will push with change to see it succeed.

@ansibullbot ansibullbot added integration tests/integration tests tests and removed module module owner_pr PR created by owner/maintainer small_patch Hopefully easy to review labels Oct 21, 2020
@sebasmannem
Copy link
Contributor Author

Yes, unittest fails. Now lets re add feature which should fix the unit test

@sebasmannem
Copy link
Contributor Author

We are all good

Copy link
Contributor

@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.

LGTM

@Andersson007 Andersson007 merged commit 3a56699 into ansible-collections:main Oct 21, 2020
patchback bot pushed a commit that referenced this pull request Oct 21, 2020
* Update postgresql_pg_hba.py

Fixes #1108

* Create changelog fragment for pull 1124

* Adding a unit test for pg_hba

(cherry picked from commit 3a56699)
@Andersson007
Copy link
Contributor

@sebasmannem thanks for fixing this!
@felixfontein thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Oct 21, 2020
* Update postgresql_pg_hba.py

Fixes #1108

* Create changelog fragment for pull 1124

* Adding a unit test for pg_hba

(cherry picked from commit 3a56699)

Co-authored-by: Sebastiaan Mannem <[email protected]>
@sebasmannem sebasmannem deleted the patch-1 branch October 21, 2020 22:19
@sebasmannem
Copy link
Contributor Author

@Andersson007 , @felixfontein , many thanks to the both of you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review database integration tests/integration module module new_contributor Help guide this first time contributor plugins plugin (any type) postgresql tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module postgresql_pg_hba Throws KeyError When Setting Method Options
4 participants