Skip to content

fix: update pgbouncer.get_auth (INFRA-1530) #1554

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

Merged
merged 7 commits into from
Apr 22, 2025

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented Apr 17, 2025

What kind of change does this PR introduce?

  • Updates the severity level of from warning to debug: INFRA-1530
  • Updates pgbouncer.get_auth definition

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

@kangmingtay kangmingtay force-pushed the km/update-pgbouncer-get-auth branch from 03f2d7d to a4be80b Compare April 17, 2025 19:22
@kangmingtay kangmingtay changed the title fix: update pgbouncer.get_auth fix: update pgbouncer.get_auth (INFRA-1530) Apr 17, 2025
Copy link
Member

@steve-chavez steve-chavez left a comment

Choose a reason for hiding this comment

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

@samrose @kangmingtay We need pgbouncer tests to ensure a regression doesn't happen.

They should be similar to the ones auth.out has. Namely:

  • tests that cover the pgbouncer schema: tables with owners, functions with owners
  • tests that cover the privileges roles have on the pgbouncer db objects

Just noted there's a test for the get_auth function:

pgbouncer | get_auth

But it doesn't cover the owner, that needs to be added.

@steve-chavez
Copy link
Member

How urgent is this? I might be able to help with the tests here if there's time.

@kangmingtay kangmingtay force-pushed the km/update-pgbouncer-get-auth branch from f85a38e to 25d23b7 Compare April 21, 2025 17:31
Copy link
Member

@steve-chavez steve-chavez left a comment

Choose a reason for hiding this comment

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

Great work! 💯

Those testinfra CI jobs are flaky, I suggest just rerunning them manually and see if they pass.

@kangmingtay kangmingtay merged commit 404d104 into develop Apr 22, 2025
14 checks passed
@kangmingtay kangmingtay deleted the km/update-pgbouncer-get-auth branch April 22, 2025 06:14
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.

3 participants